[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] livepatch: set -f{function,data}-sections compiler option
On Mon, Mar 07, 2022 at 05:19:53PM +0000, Julien Grall wrote: > Hi Roger, > > On 07/03/2022 15:55, 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. > > > > Note that placement of the sections inside of .text is also slightly > > adjusted to be more similar to the position found in the default GNU > > ld linker script. This requires having a separate section for the > > header in order to place it at the begging of the output image, > > followed with the unlikely and related sections, and finally the main > > .text section. > > > > On Arm the .data.read_mostly needs to be moved ahead of the .data > > section like it's already done on x86, and the alignment 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. > > This sounds like a bug not related to this patch. Can this be split > separately? No, .data.read_mostly needs to be moved before .data so that the catch all .data.* introduced to .data to account for -fdata-sections doesn't end up also including .data.read_mostly. > > > > 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.pau@xxxxxxxxxx> > > --- > > Changes since v1: > > - Introduce CC_SPLIT_SECTIONS for selecting the compiler options. > > - Drop check for compiler options, all supported versions have them. > > - Re-arrange section placement in .text, to match the default linker > > script. > > - Introduce .text.header to contain the headers bits that must appear > > first in the final binary. > > --- > > Given that now the header is explicitly placed in .text.header, it's > > likely possible to simplify some of the ordering of the object files > > for the prelink.o generation, as arch/x86/boot/built_in.o no longer > > needs to be the first object file in the list. > > > > It also seems on Arm the schedulers and hypfs .data sections should be > > moved into read_mostly. > > --- > > Tested by gitlab in order to assert I didn't introduce any regression > > on Arm specially. > > --- > > xen/Makefile | 2 ++ > > xen/arch/arm/arm32/head.S | 1 + > > xen/arch/arm/arm64/head.S | 1 + > > xen/arch/arm/xen.lds.S | 49 +++++++++++++++++++++------------------ > > xen/arch/x86/boot/head.S | 2 +- > > xen/arch/x86/xen.lds.S | 22 +++++++++++------- > > xen/common/Kconfig | 4 ++++ > > 7 files changed, 49 insertions(+), 32 deletions(-) > > > > diff --git a/xen/Makefile b/xen/Makefile > > index 5c21492d6f..18a4f7e101 100644 > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -273,6 +273,8 @@ else > > CFLAGS += -fomit-frame-pointer > > endif > > +CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections > > + > > CFLAGS += -nostdinc -fno-builtin -fno-common > > CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith > > $(call cc-option-add,CFLAGS,CC,-Wvla) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > > index 7a906167ef..c837d3054c 100644 > > --- a/xen/arch/arm/arm32/head.S > > +++ b/xen/arch/arm/arm32/head.S > > @@ -120,6 +120,7 @@ > > #endif /* !CONFIG_EARLY_PRINTK */ > > + .section .text.header, "ax", %progbits > > .arm > > /* > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > > index 66d862fc81..e62c48ec1c 100644 > > --- a/xen/arch/arm/arm64/head.S > > +++ b/xen/arch/arm/arm64/head.S > > @@ -133,6 +133,7 @@ > > add \xb, \xb, x20 > > .endm > > + .section .text.header, "ax", %progbits > > /*.aarch64*/ > > /* > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > > index 08016948ab..836da880c3 100644 > > --- a/xen/arch/arm/xen.lds.S > > +++ b/xen/arch/arm/xen.lds.S > > @@ -30,9 +30,16 @@ SECTIONS > > _start = .; > > .text : { > > _stext = .; /* Text section */ > > + *(.text.header) > > With this change, the changes in arch/*/arch.mk to order head.o looks > unnecessary. Can we remove it at the same time? (The .text.header and the > update of arch.mk may want to be done together in a separate patch). I had a note below the commit message about this, as I didn't think it was strictly necessary to get this accepted. I will do a pre-patch then with those added. Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |