On Tue, Mar 08, 2022 at 03:09:17PM +0100, Jan Beulich wrote:
> On 08.03.2022 14:49, Roger Pau Monne wrote:
> > If livepatching support is enabled build the hypervisor with
> > -f{function,data}-sections compiler options, which is required by the
> > livepatching tools to detect changes and create livepatches.
> >
> > This shouldn't result in any functional change on the hypervisor
> > binary image, but does however require some changes in the linker
> > script in order to handle that each function and data item will now be
> > placed into its own section in object files. As a result add catch-all
> > for .text, .data and .bss in order to merge each individual item
> > section into the final image.
> >
> > The main difference will be that .text.startup will end up being part
> > of .text rather than .init, and thus won't be freed. .text.exit will
> > also be part of .text rather than dropped. Overall this could make the
> > image bigger, and package some .text code in a sub-optimal way.
> >
> > On Arm the .data.read_mostly needs to be moved ahead of the .data
> > section like it's already done on x86, so the .data.* catch-all
> > doesn't also include .data.read_mostly. The alignment of
> > .data.read_mostly also needs to be set to PAGE_SIZE so it doesn't end
> > up being placed at the tail of a read-only page from the previous
> > section. While there move the alignment of the .data section ahead of
> > the section declaration, like it's done for other sections.
> >
> > The benefit of having CONFIG_LIVEPATCH enable those compiler option
> > is that the livepatch build tools no longer need to fiddle with the
> > build system in order to enable them. Note the current livepatch tools
> > are broken after the recent build changes due to the way they
> > attempt to set -f{function,data}-sections.
> >
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -88,6 +88,9 @@ SECTIONS
> > *(.text.unlikely .text.*_unlikely .text.unlikely.*)
> >
> > *(.text)
> > +#ifdef CONFIG_CC_SPLIT_SECTIONS
> > + *(.text.*)
> > +#endif
> > *(.text.__x86_indirect_thunk_*)
> > *(.text.page_aligned)
>
> These last two now will not have any effect anymore when
> CC_SPLIT_SECTIONS=y. This may have undesirable effects on the
> overall size when there is more than one object with a
> .text.page_aligned contribution. In .data ...
Agreed. I wondered whether to move those ahead of the main text
section, so likely:
*(.text.unlikely .text.*_unlikely .text.unlikely.*)
*(.text.page_aligned)
*(.text.__x86_indirect_thunk_*)
*(.text)
#ifdef CONFIG_CC_SPLIT_SECTIONS
*(.text.*)
#endif
FWIW, Linux seems fine to package .text.page_aligned together with the
rest of .text using the .text.[0-9a-zA-Z_]* catch-all.
> > @@ -292,9 +295,7 @@ SECTIONS
> >
> > DECL_SECTION(.data) {
> > *(.data.page_aligned)
> > - *(.data)
> > - *(.data.rel)
> > - *(.data.rel.*)
> > + *(.data .data.*)
> > } PHDR(text)
>
> ... this continues to be named first. I wonder whether we wouldn't
> want to use SORT_BY_ALIGNMENT (if available) instead in both places.
We could use the command line option if available
(--sort-section=alignment) to sort all wildcard sections?
Thanks, Roger.
