[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: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 8 Mar 2022 17:58:49 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=ymkM/QvlWViNXLz+BD43jTuKIk42fHJh3wM19X0MRSE=; b=Hh7PBpWgMoehCunNIdmVvrpeZtGqwRBq9ZHC/YwKw0D6qPp3uSIig3O+dcIW5YVtDUle9uyKYLbma4hGcSdpmoJb6VpawwPLltzU8/GxM64VtuXNGc3g+UE0PNNwE8q8dxfLNiplEequkwS1baJsx1OY5n6CY3LrM1nf9URWk+J4QkV2zNW96PZVA0C39D8ullCofguTfxguHzjDpYCyZLZ9K6tuUYB58CAhRtWoqU1gGKZ/3ngAxpegf3a5PwCFhnlsh2b7aHG4zHQcaUdU6A1bmphRQolI/b6f0LI4AigY1626qPJB0h/+G3wdHxh9oF4ae5VJCCkUe933gcS6wQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YW9yFGd8P8ISdFjfXC99k9vJJ/g5O8po0E4cxx6yBHICW4VMmMziuNvj6SFiL0kAU9kGSEhX/oNQLgFv2zyDDdRuoHwDoPW4gHYGJRA31orSw1jtFmIwlF3SH9A6U8bc9m8aLM9SNvsO1NwE7rPkHwMTP41ivX1ftRC+7kI1C8IKDi5QHHSV0T9EIK61+FL+ejjRBAOPXomnfAT7HFEyCW028Yw6VWo6TI+frKjmgkPmBWbzejUeBxIsNHNfXdycBip6Gi0RapyqZwKUEwYZNHm0Ci2UoQBL+iigxzgUxiUt3a9AcQUzbxjDWpEFJWG99gMkMeZFAtuNPx7bO/wdIg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.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:59:03 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 08.03.2022 17:41, Roger Pau Monné wrote:
> 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?

Sure. Question is whether sorting is limited to within any single
*(...) construct, or whether it could extend to adjacent ones. IOW
whether the command line option strictly is a replacement of adding
SORT_BY_ALIGNMENT to every one of these constructs.

Jan




 


Rackspace

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