diff mbox series

[8/8] x86/EFI: don't have an overly large image size

Message ID 26ec4a83-cda9-5193-d797-357c05b26ab7@suse.com (mailing list archive)
State Superseded
Headers show
Series x86/EFI: build adjustments | expand

Commit Message

Jan Beulich April 1, 2021, 9:47 a.m. UTC
While without debug info the difference is benign (so far), since we pad
the image to 16Mb anyway, forcing the .reloc section to a 2Mb boundary
causes subsequent .debug_* sections to go farther beyond 16Mb than
needed. There's no reason to advance . for establishing __2M_rwdata_end,
as all data past _end is of no interest at runtime anymore anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This makes more explicit a possible latent problem with the ELF image:
It ends at _end, not __2M_rwdata_end (advancing . as was done here does
not have the effect of increasing the image size). Interestingly the
conversion xen-syms => xen rounds up the program header specified size
suitably, as per the comment "Do not use p_memsz: it does not include
BSS alignment padding" in mkelf32.c. I do think this would instead want
taking care of in the linker script. Commit 7a95e0a2c572 ("x86: properly
calculate xen ELF end of image address") clearly only hacked an existing
hack rather than addressing the root cause. Thoughts?

Comments

Roger Pau Monne April 21, 2021, 11:18 a.m. UTC | #1
On Thu, Apr 01, 2021 at 11:47:35AM +0200, Jan Beulich wrote:
> While without debug info the difference is benign (so far), since we pad
> the image to 16Mb anyway, forcing the .reloc section to a 2Mb boundary
> causes subsequent .debug_* sections to go farther beyond 16Mb than
> needed. There's no reason to advance . for establishing __2M_rwdata_end,
> as all data past _end is of no interest at runtime anymore anyway.

So you just expand the load size.

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> This makes more explicit a possible latent problem with the ELF image:
> It ends at _end, not __2M_rwdata_end (advancing . as was done here does
> not have the effect of increasing the image size). Interestingly the
> conversion xen-syms => xen rounds up the program header specified size
> suitably, as per the comment "Do not use p_memsz: it does not include
> BSS alignment padding" in mkelf32.c. I do think this would instead want
> taking care of in the linker script. Commit 7a95e0a2c572 ("x86: properly
> calculate xen ELF end of image address") clearly only hacked an existing
> hack rather than addressing the root cause. Thoughts?

We should likely define _end after __2M_rwdata_end to account for this
padding?

Thanks, Roger.
Jan Beulich April 21, 2021, 1:15 p.m. UTC | #2
On 21.04.2021 13:18, Roger Pau Monné wrote:
> On Thu, Apr 01, 2021 at 11:47:35AM +0200, Jan Beulich wrote:
>> While without debug info the difference is benign (so far), since we pad
>> the image to 16Mb anyway, forcing the .reloc section to a 2Mb boundary
>> causes subsequent .debug_* sections to go farther beyond 16Mb than
>> needed. There's no reason to advance . for establishing __2M_rwdata_end,
>> as all data past _end is of no interest at runtime anymore anyway.
> 
> So you just expand the load size.

Shrink. Or maybe I'm misunderstanding you.

>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>> ---
>> This makes more explicit a possible latent problem with the ELF image:
>> It ends at _end, not __2M_rwdata_end (advancing . as was done here does
>> not have the effect of increasing the image size). Interestingly the
>> conversion xen-syms => xen rounds up the program header specified size
>> suitably, as per the comment "Do not use p_memsz: it does not include
>> BSS alignment padding" in mkelf32.c. I do think this would instead want
>> taking care of in the linker script. Commit 7a95e0a2c572 ("x86: properly
>> calculate xen ELF end of image address") clearly only hacked an existing
>> hack rather than addressing the root cause. Thoughts?
> 
> We should likely define _end after __2M_rwdata_end to account for this
> padding?

I don't think this would help - we'd need to arrange for the image size
to cover that extra padding. Like advancing . doesn't grow the image
size, I also don't think placing _end later would do.

Jan
diff mbox series

Patch

--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -303,8 +303,7 @@  SECTIONS
   } PHDR(text)
   _end = . ;
 
-  . = ALIGN(SECTION_ALIGN);
-  __2M_rwdata_end = .;
+  __2M_rwdata_end = ALIGN(SECTION_ALIGN);
 
 #ifdef EFI
   .reloc ALIGN(4) : {