[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 15:46:34 +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=kF/Y7x+EaR4NgtF/xj4yHlhreyRRqf4Q2AKZ4rRQWyI=; b=EuJgR6jKLTBGsq7bbwW8TCuusr8KRURwkeGfVLbWxOc+INhOLwIo8ve5aT/DoII/q2Ys6b6KOp/empc8JG9xQiZ2MEJs88lEOKsPzL59Ua684UYqaJhCGSHr91BH8fow92VLTvKCogHM3RDMBiCXY1DsjFCufJr/ihoQOu90VBu0w7bCLvlSO2xzypI4flSXmxDaUdL5JS1e11N8194iNTDE2T9Yk/13Ga8bw6AECp4sLJkICBaBEBgxIQfjplu3LxrPF7uuCwtRwUF5jM7iGwlk2Wao71Dh+GhGe554hq/RGYgnpU5ZtEgXbVWiSSE66diJ8wrre97zdOaLTe+5Lg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TaPOWhLGJm8y/Pll84bVL0H6A7vKaSZv8of+Bi7zgAJmp09z4mjAwv7ubzDj1Zgn1ZlJAJHEb8manb98VnUsusfyXGBBSOT7iB1Hi3oH+DvSvBI4WtinjANFnZv0csDLvudXMfH36ryli/JV5igGOylwJaXawwHXPXcDHax9IqEJACCEzkI8PbJLp02BrsgwY6LZehPNr7HbdtLrVOFRoeZNpqnKns6wCBmuRmZeGeK9e6jlUenCp6uh2neAiRthLlLMLrrFWaY+7N9OCQhgriOFYNJsKaOY+jP+LOq+CgzNkQ4ph3An8xH7ujoFqZHcn6k193gi5Vn1lJg4WXiu/w==
  • Authentication-results: esa1.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 14:46:53 +0000
  • Ironport-data: A9a23:o5MpCath3U5QDAGxmpj5I2glP+fnVKJeMUV32f8akzHdYApBsoF/q tZmKWHQOv6DMWHyKdB1btmw9EsB7Z/TzdBrSANp/i9jEy8U+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZhSAgk/nOHNIQMcacUsxLbVYMpBwJ1FQyw4bVvqYy2YLjW1jV6 YuoyyHiEATNNwBcYzp8B52r8HuDjNyq0N/PlgVjDRzjlAa2e0g9VPrzF4noR5fLatA88tqBb /TC1NmEElbxpH/BPD8HfoHTKSXmSpaKVeSHZ+E/t6KK2nCurQRquko32WZ1he66RFxlkvgoo Oihu6BcRi91JpfAp+4/CiMGDnpDO/BW/5zWGGCG5Jn7I03uKxMAwt1rBUAye4YZ5vx2ESdF8 vlwxDIlN07ZwbjsmfTiF7cq1p9LwMrDZevzvll6yj7UF7A+SI3rSKTW/95Imjw3g6iiGN6AO ZNAMWExNHwsZTVfK0UFNa4Vk9uJrXvRKRpIqQmnt6w4tj27IAtZj+G2bYu9lsaxbc9YhFqCr 2TKuWHwGAgHNce3wCCAtHmrg4fnoyT/X44DEayiwdRjilaT2287BQUfUB2wpvzRol6zXZdTJ lIZ/gIqrLMu7wq7Q9/lRRq6rXWY+BkGVLJ4DOkS+AyLjK3O7G6xBGIJUzpAY9wOr9ItSHoh0 Vrht9H0AT1itpWFRHTb8a2bxRupIjQcJ2IGYS4CTCMG7sPlrYV1iQjAJuuPC4bs0IezQ2uph WnX8m5u3N3/kPLnyY3n5WrLvDiO/6KXU1UI4FjHWUG01xtAMdvNi5OT1XDX6vNJLYC8R1aHv WQZl8X20N3iHa1hhwTWHrxTQejBC+KtdWSF3AUxR8VJGyGFpib7Fb289g2SM6uA3iwsXTbyK HHetgpKjHO4FCv7NPQnC25d5ilD8EQBKTgHfq2MBjatSsIoHONiwM2ITRTIt4wKuBJw+ZzTw b/BLa6R4Y8yUMyLNgaeSeYHyqMMzSsj327VTp2T5035jeTAOy7IEu5VbQDmggUFAEWs+l69H zF3bZfi9vmieLemPnm/HXA7dzjm0kTX9bip8pcKJ4Zv0yJtGX07Cu+5/F/SU9cNokihrc+Rp ivVchYBkDLX3CSbQS3XOiELQO6+Bv5X8CNkVRHAyH71ghDPl670t/xBH3b2FJF6nNFeIQlcE 6FVK5/eU64UFlwqOV01NPHAkWCrTzzy7SqmNCu5ejkvOZlmQg3C4Nj/eQXzsiIJC0KKWQEW+ dVMCiuzrUI/ejlf
  • Ironport-hdrordr: A9a23:Sn+waK3SDQoKxjG7Gu3nOgqjBVByeYIsimQD101hICG9Lfb2qy n+ppgmPEHP5Qr5OEtApTiBUJPwJk800aQFm7X5Wo3SITUO2VHYV72KiLGN/9SOIVydygcw79 YET0E6MqyNMbEYt7eK3ODbKadY/DDvysnB7o2/vhRQpENRGtldBm9Ce3im+yZNNW977PQCZf 6hDp0tnUveRZ1bVLXyOlA1G8z44/HbnpPvZhALQzYh9Qm1lDutrJr3CQKR0BsyWy5Ghe5Kyx mOryXJooGY992rwB7V0GHeq7xQhdva09NGQOiBkNIcJDnAghuhIK5hR7qBljYop/zH0idgrP D85zMbe+hj4XLYeW+45TPrxgnbyT4rr0TvzFeJ6EGT1vDRdXYfMY5slIhZehzW5w4Lp9dnyp 9G2Gqfqt5+EQ7AtD6V3amGazha0m6P5VYym+8aiHJSFaEEbqVKkIAZ9ERJVL8dASPB7pw9Gu UGNrCR2B9vSyLaU5nlhBgu/DT1NU5DXStuA3Jy9/B96gIm0kyQlCAjtY4idnRpzuNId3AL3Z WADk1SrsA8ciYnV9MMOA4/e7rENoXse2O7DIvAGyWvKEk4U0i93qIfpo9FoN2XRA==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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

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.



 


Rackspace

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