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

Re: [PATCH v3 2/2] livepatch: set -f{function,data}-sections compiler option


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 8 Mar 2022 17:41:57 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8FpsNgGd4pW12nUfLqrWEI0Fl+hLrQ10LohQqvbxOoM=; b=N14wETthtrijQEYENdXdEHMKQKdjV8OfnLg8FpkkEvMK2slnd8KYQdQ6lRbEggvC+w6CH4xZwggKOoYOAbDR1HSKjGvrOHNU/7NLo5CgypxA2QocciV6J4YOXAeeBaFEFVvs1zmIDjf4POWWHkjgDf6imvWI4oGe7Iqz4cJdpOQ2Ysiz0NpEKd22gXW2heB/p3oswXAGQW2GTEQmqEtssVxWKBQ00aGbtcyQgs2nHnqUrRx0s1ve3jVIFqLx/5kjTbqLb0q50fEpqzOXRFdzKh4sr+OFTbb1PRTmpRAOtwXy6JnVieFwRkY7s1ygwbyLN02spANhiBHysQzlSjuxEA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gCvj+h00Np6vppn6MihZ4d+XagjxKonUze5xwd5z2iNtDdUD4Y112nnjy78VXfzT57CRDEuUfd7HktTlJrmmEJ52Wiqp1RchOAVMfebWPZ79QqPLn0fXC0lGk19bWrBl3jbGXF9vQ3Pnp0x5Hd4KdLICTv4JmXqo6M5+sqPlGVIdNVUzfwiK69im2mr7P5B9SnEs4LSVNk7WeeoptGeiEobhCjMzxZuxbODcyvxUDxqwhO+HfVWijDoTDpM0MJNPM+A8bZCibypRrSeUqZjDI/TbkfMSnxV2gXO5N+ipUpkgQvAC6FK774EugohCJ4CTh/6921jKeBMUn1p7d1IM1w==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 08 Mar 2022 16:42:16 +0000
  • Ironport-data: A9a23:OECl6aiB5Gfm6q57Z6n6eQ8aX161lBAKZh0ujC45NGQN5FlHY01je htvXWCCM/mKNGTxeN4kYdu19x4AsZPTnIA1TAs+qihmFygb9cadCdqndUqhZCn6wu8v7a5EA 2fyTvGacajYm1eF/k/F3oDJ9CU6jefSLlbFILas1hpZHGeIcw98z0M78wIFqtQw24LhWFvd4 YmaT/D3YzdJ5RYlagr41IrbwP9flKyaVOQw5wFWiVhj5TcyplFNZH4tDfjZw0jQG+G4KtWSV efbpIxVy0uCl/sb5nFJpZ6gGqECaua60QFjERO6UYD66vRJjnRaPqrWqJPwwKqY4tmEt4kZ9 TlDiXC/YV4ZPIPBqfYgaDMbFCojIY1m5ubDIXfq5KR/z2WeG5ft6/BnDUVwNowE4OdnR2pJ8 JT0KhhUMErF3bjvhuvmFK883azPL+GyVG8bkmtnwjzDS+4vXLjIQrnQ5M8e1zA17ixLNaiDO JFDMWExBPjGSzAMJG9LLLRvp+fyhynta3pIo3HOgLVitgA/yyQuieOwYbI5YOeiT8hPglyRo G6A+m3jGwwbL/SW0z/D+XWp7sfxmif8VJMXBaeP3Pdgi12OxUQeEBQTE1C8pJGRmkO4Ht5SN UEQ0i4vtrQpslymSMHnWB+1q2LCuQQTM/JPF8Uq5QfLzbDbiy6bDGUZSj9KaPQ9qdQ7Azct0 zehnc7tBDFpmK2YTzSa7Lj8hSipJSEfIGsGZCkFZQgI+d/upMc0lB2nczp4OPfr1JuvQ2i2m m3U6nhl71kOsSIV//yL/37KsiCRnbKKSQVo+iX3QDuh7gwsMeZJeLeUwVTc6P9BKqOQQV+Ao GUIlqCi0QweMX2evHfTGbtQRdlF897AaWSB2gA3Q/HN4hzwoybLQGxG3N1pyK6F2O4gcCShX kLcsBg5CHR7bCrzNv8fj25c5q0XIUnc+TbNC6i8gjlmOMEZmOq7EMdGPxb4M4fFyhRErE3HE c3HGftA9F5DYUid8BK4Rv0GzZggzT0kyGXYSPjTlkr7j+TBNCPOEOdYazNii9zVCove+205F P4Fa6O3J+h3CrWiMkE7D6ZJRbz1EZTLLc+v8JEGHgJyCgFnBHsgG5fsLUAJIORYc1Buvr6Qp BmVAxYAoHKm3CGvAVjaOxhLNeK0Nb4i/C1TAMDZFQvxs5TVSd30t/l3mlpeVeRPydGPOtYoF 6lVIZvRWK8TItkFkhxEBaTAQEVZXE3DrSqFPja/YSh5eJhlRgfT/cTjcBep/y4LZhdbf+Nny 1F8/ms3maY+ejk=
  • Ironport-hdrordr: A9a23:5eu4Z6xhYPffGo4Bjj4NKrPwKL1zdoMgy1knxilNoHtuA6ulfq GV7ZAmPHrP4wr5N0tNpTntAsa9qBDnlaKdg7N+AV7KZmCP0gaVxepZjLfK8nnNHDD/6/4Y9Y oISdkaNDQoNykYsS8t2njbL+od
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Mar 08, 2022 at 04:13:55PM +0100, Jan Beulich wrote:
> On 08.03.2022 15:46, Roger Pau Monné wrote:
> > 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.pau@xxxxxxxxxx>
> >>
> >> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>
> >>> --- 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
> 
> Perhaps; I'm not really worried of .text.__x86_indirect_thunk_*,
> though. When adding .text.* that one can likely go away.
> 
> > FWIW, Linux seems fine to package .text.page_aligned together with the
> > rest of .text using the .text.[0-9a-zA-Z_]* catch-all.
> 
> There's no question this is functionally fine. The question is how
> many extra padding areas are inserted because of this.
> 
> >>> @@ -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?
> 
> Depends on the scope of the sorting that would result when enabled
> globally like this.

I'm not sure I'm following. Don't we generally want to sort by
alignment in order to avoid adding unnecessary padding as much as
possible?

For any wildcard sections we really don't care anymore how they are
sorted?

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.