On Wed, Mar 02, 2022 at 05:09:10PM +0100, Jan Beulich wrote:
> On 02.03.2022 16:46, Roger Pau Monné wrote:
> > On Wed, Mar 02, 2022 at 03:41:21PM +0100, Jan Beulich wrote:
> >> On 02.03.2022 14:44, Roger Pau Monne wrote:
> >>> @@ -292,6 +295,9 @@ SECTIONS
> >>> *(.data)
> >>> *(.data.rel)
> >>> *(.data.rel.*)
> >>> +#ifdef CONFIG_LIVEPATCH
> >>> + *(.data.*)
> >>> +#endif
> >>> CONSTRUCTORS
> >>> } PHDR(text)
> >>>
> >>> @@ -308,6 +314,9 @@ SECTIONS
> >>> . = ALIGN(SMP_CACHE_BYTES);
> >>> __per_cpu_data_end = .;
> >>> *(.bss)
> >>> +#ifdef CONFIG_LIVEPATCH
> >>> + *(.bss.*)
> >>> +#endif
> >>
> >> ... are these two really in need of being conditional?
> >
> > Will drop if you agree. I didn't want to risk introducing unwanted
> > changes in the !CONFIG_LIVEPATCH case.
>
> The only "unwanted" change I can imagine here would be that we place a
> section which the linker would otherwise need to guess how to place,
> for being "orphan".
>
> >>> --- a/xen/common/Kconfig
> >>> +++ b/xen/common/Kconfig
> >>> @@ -353,7 +353,9 @@ config CRYPTO
> >>> config LIVEPATCH
> >>> bool "Live patching support"
> >>> default X86
> >>> - depends on "$(XEN_HAS_BUILD_ID)" = "y"
> >>> + depends on "$(XEN_HAS_BUILD_ID)" = "y" && \
> >>> + $(cc-option,-ffunction-sections) && \
> >>> + $(cc-option,-fdata-sections)
> >>
> >> Is this for certain Clang versions? Gcc has been supporting this in
> >> 4.1.x already (didn't check when it was introduced).
> >
> > I've checked clang and it seems to be prevent in at least Clang 5,
> > which is likely enough?
>
> Clang5 accepts the options fine here. But that wouldn't be enough,
> ./README says "Clang/LLVM 3.5 or later".
>
> > I've added the check just to be on the safe side.
>
> Well, yes, if you're unsure and the old version can't be checked,
> then perhaps indeed better to probe.
OK, so I've managed to probe clang 3.5.0 and it does support
-f{function,data}-sections so we can drop the check.
Thanks, Roger.