[PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Roger Pau Monne]

The tools infrastructure used to build livepatches for Xen
(livepatch-build-tools) consumes some DWARF debug information present in
xen-syms to generate a livepatch (see livepatch-build script usage of readelf
-wi).

The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
on release builds, thus providing a default Kconfig selection that's not
suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
because it's missing the DWARF debug section.

Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/common/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 90268d92499a..f3135f85034d 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -420,6 +420,7 @@ config LIVEPATCH
 	default X86
 	depends on "$(XEN_HAS_BUILD_ID)" = "y"
 	select CC_SPLIT_SECTIONS
+	select DEBUG_INFO
 	help
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
-- 
2.46.0

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Andrew Cooper]

On 07/11/2024 8:49 am, Roger Pau Monne wrote:
> The tools infrastructure used to build livepatches for Xen
> (livepatch-build-tools) consumes some DWARF debug information present in
> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
> -wi).
>
> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
> on release builds, thus providing a default Kconfig selection that's not
> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
> because it's missing the DWARF debug section.
>
> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Oops, yes.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Fixes tag ?

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Roger Pau Monné]

On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
> > The tools infrastructure used to build livepatches for Xen
> > (livepatch-build-tools) consumes some DWARF debug information present in
> > xen-syms to generate a livepatch (see livepatch-build script usage of readelf
> > -wi).
> >
> > The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
> > on release builds, thus providing a default Kconfig selection that's not
> > suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
> > because it's missing the DWARF debug section.
> >
> > Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Oops, yes.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Fixes tag ?

Was borderline on adding one, but wasn't sure since it's strictly
livepatch-build-tools that requires the DWARF data, but custom made
livepatches (like the examples in tests) do not require such
information.

Possibly:

Fixes: 11ff40fa7bb5 ('xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op')

Which is the commit that originally introduced the CONFIG_XSPLICE option.

Thanks, Roger.

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Jan Beulich]

On 07.11.2024 10:40, Roger Pau Monné wrote:
> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>> The tools infrastructure used to build livepatches for Xen
>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>> -wi).
>>>
>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>> on release builds, thus providing a default Kconfig selection that's not
>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>> because it's missing the DWARF debug section.
>>>
>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Oops, yes.
>>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> Fixes tag ?
> 
> Was borderline on adding one, but wasn't sure since it's strictly
> livepatch-build-tools that requires the DWARF data, but custom made
> livepatches (like the examples in tests) do not require such
> information.

At which point: Is "select" really appropriate then? Wouldn't it be more
logical then to change DEBUG_INFO's default to take LIVEPATCH into account
(still permitting people to turn debug info off if they know they won't
need it)?

Jan

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Roger Pau Monné]

On Thu, Nov 07, 2024 at 10:48:21AM +0100, Jan Beulich wrote:
> On 07.11.2024 10:40, Roger Pau Monné wrote:
> > On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
> >> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
> >>> The tools infrastructure used to build livepatches for Xen
> >>> (livepatch-build-tools) consumes some DWARF debug information present in
> >>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
> >>> -wi).
> >>>
> >>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
> >>> on release builds, thus providing a default Kconfig selection that's not
> >>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
> >>> because it's missing the DWARF debug section.
> >>>
> >>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
> >>>
> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >>
> >> Oops, yes.
> >>
> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> >> Fixes tag ?
> > 
> > Was borderline on adding one, but wasn't sure since it's strictly
> > livepatch-build-tools that requires the DWARF data, but custom made
> > livepatches (like the examples in tests) do not require such
> > information.
> 
> At which point: Is "select" really appropriate then? Wouldn't it be more
> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
> (still permitting people to turn debug info off if they know they won't
> need it)?

At least right now the only way to build useful livepatches for Xen
(not dummy tests) is using livepatch-build-tools (that requires DWARF
sections).  My concern with using the Fixes tag was that I don't know
whether initially the DWARF info was needed or not, or maybe whether
people used a different tool rather than livepatch-build-tools.

I don't mind changing, but I also don't think an hypervisor built with
LIVEPATCH but without DEBUG_INFO is going to be useful to anyone given
the requirements of the tools we provide to build livepatches.

Thanks, Roger.

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Jan Beulich]

On 07.11.2024 12:25, Roger Pau Monné wrote:
> On Thu, Nov 07, 2024 at 10:48:21AM +0100, Jan Beulich wrote:
>> On 07.11.2024 10:40, Roger Pau Monné wrote:
>>> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>>>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>>>> The tools infrastructure used to build livepatches for Xen
>>>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>>>> -wi).
>>>>>
>>>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>>>> on release builds, thus providing a default Kconfig selection that's not
>>>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>>>> because it's missing the DWARF debug section.
>>>>>
>>>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>>>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>
>>>> Oops, yes.
>>>>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> Fixes tag ?
>>>
>>> Was borderline on adding one, but wasn't sure since it's strictly
>>> livepatch-build-tools that requires the DWARF data, but custom made
>>> livepatches (like the examples in tests) do not require such
>>> information.
>>
>> At which point: Is "select" really appropriate then? Wouldn't it be more
>> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
>> (still permitting people to turn debug info off if they know they won't
>> need it)?
> 
> At least right now the only way to build useful livepatches for Xen
> (not dummy tests) is using livepatch-build-tools (that requires DWARF
> sections).  My concern with using the Fixes tag was that I don't know
> whether initially the DWARF info was needed or not, or maybe whether
> people used a different tool rather than livepatch-build-tools.
> 
> I don't mind changing, but I also don't think an hypervisor built with
> LIVEPATCH but without DEBUG_INFO is going to be useful to anyone given
> the requirements of the tools we provide to build livepatches.

Well, okay, especially Andrew looks to be pretty strong about this. I
don't want to stand in the way ...

Jan

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Andrew Cooper]

On 07/11/2024 9:48 am, Jan Beulich wrote:
> On 07.11.2024 10:40, Roger Pau Monné wrote:
>> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>>> The tools infrastructure used to build livepatches for Xen
>>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>>> -wi).
>>>>
>>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>>> on release builds, thus providing a default Kconfig selection that's not
>>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>>> because it's missing the DWARF debug section.
>>>>
>>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Oops, yes.
>>>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> Fixes tag ?
>> Was borderline on adding one, but wasn't sure since it's strictly
>> livepatch-build-tools that requires the DWARF data, but custom made
>> livepatches (like the examples in tests) do not require such
>> information.

Ok.  I guess it doesn't matter too much.

> At which point: Is "select" really appropriate then? Wouldn't it be more
> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
> (still permitting people to turn debug info off if they know they won't
> need it)?

I am very disinterested in letting people who think they can do
livepatching without debug symbols shoot themselves in the foot like that.

It's only debugging symbols.   If people *really* think they know
better, they can strip them themselves.

~Andrew

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Jan Beulich]

On 07.11.2024 12:30, Andrew Cooper wrote:
> On 07/11/2024 9:48 am, Jan Beulich wrote:
>> On 07.11.2024 10:40, Roger Pau Monné wrote:
>>> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>>>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>>>> The tools infrastructure used to build livepatches for Xen
>>>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>>>> -wi).
>>>>>
>>>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>>>> on release builds, thus providing a default Kconfig selection that's not
>>>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>>>> because it's missing the DWARF debug section.
>>>>>
>>>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>>>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> Oops, yes.
>>>>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> Fixes tag ?
>>> Was borderline on adding one, but wasn't sure since it's strictly
>>> livepatch-build-tools that requires the DWARF data, but custom made
>>> livepatches (like the examples in tests) do not require such
>>> information.
> 
> Ok.  I guess it doesn't matter too much.
> 
>> At which point: Is "select" really appropriate then? Wouldn't it be more
>> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
>> (still permitting people to turn debug info off if they know they won't
>> need it)?
> 
> I am very disinterested in letting people who think they can do
> livepatching without debug symbols shoot themselves in the foot like that.
> 
> It's only debugging symbols.   If people *really* think they know
> better, they can strip them themselves.

Besides my abstract concern, let me also add a concrete practical one. I'm
sure you've noticed that xen.efi is _much_ slower to link with debug info
than without, or than xen-syms. That's a consequence of how GNU ld (really:
libbfd) works internally. By not allowing DEBUG_INFO to stay off you're
forcing me to either wait longer for every single one of my post-commit
pre-push build tests, or to turn off LIVEPATCH there. The latter not really
being a good idea.

Nevertheless, as said in reply to Roger: Go ahead if you absolutely think
that's the only sensible way.

Jan

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Andrew Cooper]

On 07/11/2024 11:57 am, Jan Beulich wrote:
> On 07.11.2024 12:30, Andrew Cooper wrote:
>> On 07/11/2024 9:48 am, Jan Beulich wrote:
>>> On 07.11.2024 10:40, Roger Pau Monné wrote:
>>>> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>>>>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>>>>> The tools infrastructure used to build livepatches for Xen
>>>>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>>>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>>>>> -wi).
>>>>>>
>>>>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>>>>> on release builds, thus providing a default Kconfig selection that's not
>>>>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>>>>> because it's missing the DWARF debug section.
>>>>>>
>>>>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>>>>
>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>> Oops, yes.
>>>>>
>>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>
>>>>> Fixes tag ?
>>>> Was borderline on adding one, but wasn't sure since it's strictly
>>>> livepatch-build-tools that requires the DWARF data, but custom made
>>>> livepatches (like the examples in tests) do not require such
>>>> information.
>> Ok.  I guess it doesn't matter too much.
>>
>>> At which point: Is "select" really appropriate then? Wouldn't it be more
>>> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
>>> (still permitting people to turn debug info off if they know they won't
>>> need it)?
>> I am very disinterested in letting people who think they can do
>> livepatching without debug symbols shoot themselves in the foot like that.
>>
>> It's only debugging symbols.   If people *really* think they know
>> better, they can strip them themselves.
> Besides my abstract concern, let me also add a concrete practical one. I'm
> sure you've noticed that xen.efi is _much_ slower to link with debug info
> than without, or than xen-syms. That's a consequence of how GNU ld (really:
> libbfd) works internally. By not allowing DEBUG_INFO to stay off you're
> forcing me to either wait longer for every single one of my post-commit
> pre-push build tests, or to turn off LIVEPATCH there. The latter not really
> being a good idea.
>
> Nevertheless, as said in reply to Roger: Go ahead if you absolutely think
> that's the only sensible way.

I had noticed that link was taking a long time.  I hadn't realised it
was this specifically.

But to put this another way, you're arguing to intentionally avoid
fixing a sharp corner, because there's a perf issue in Binutils.

I will note that it was you who forced the generation of xen.efi on
everyone, even those who didn't want it, without any knob to turn it off.

~Andrew

Re: [PATCH] xen/Kconfig: livepatch-build-tools requires debug information

[Jan Beulich]

On 07.11.2024 13:17, Andrew Cooper wrote:
> On 07/11/2024 11:57 am, Jan Beulich wrote:
>> On 07.11.2024 12:30, Andrew Cooper wrote:
>>> On 07/11/2024 9:48 am, Jan Beulich wrote:
>>>> On 07.11.2024 10:40, Roger Pau Monné wrote:
>>>>> On Thu, Nov 07, 2024 at 09:21:26AM +0000, Andrew Cooper wrote:
>>>>>> On 07/11/2024 8:49 am, Roger Pau Monne wrote:
>>>>>>> The tools infrastructure used to build livepatches for Xen
>>>>>>> (livepatch-build-tools) consumes some DWARF debug information present in
>>>>>>> xen-syms to generate a livepatch (see livepatch-build script usage of readelf
>>>>>>> -wi).
>>>>>>>
>>>>>>> The current Kconfig defaults however will enable LIVEPATCH without DEBUG_INFO
>>>>>>> on release builds, thus providing a default Kconfig selection that's not
>>>>>>> suitable for livepatch-build-tools even when LIVEPATCH support is enabled,
>>>>>>> because it's missing the DWARF debug section.
>>>>>>>
>>>>>>> Fix by forcing the selection of DEBUG_INFO from LIVEPATCH.
>>>>>>>
>>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>>> Oops, yes.
>>>>>>
>>>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>>
>>>>>> Fixes tag ?
>>>>> Was borderline on adding one, but wasn't sure since it's strictly
>>>>> livepatch-build-tools that requires the DWARF data, but custom made
>>>>> livepatches (like the examples in tests) do not require such
>>>>> information.
>>> Ok.  I guess it doesn't matter too much.
>>>
>>>> At which point: Is "select" really appropriate then? Wouldn't it be more
>>>> logical then to change DEBUG_INFO's default to take LIVEPATCH into account
>>>> (still permitting people to turn debug info off if they know they won't
>>>> need it)?
>>> I am very disinterested in letting people who think they can do
>>> livepatching without debug symbols shoot themselves in the foot like that.
>>>
>>> It's only debugging symbols.   If people *really* think they know
>>> better, they can strip them themselves.
>> Besides my abstract concern, let me also add a concrete practical one. I'm
>> sure you've noticed that xen.efi is _much_ slower to link with debug info
>> than without, or than xen-syms. That's a consequence of how GNU ld (really:
>> libbfd) works internally. By not allowing DEBUG_INFO to stay off you're
>> forcing me to either wait longer for every single one of my post-commit
>> pre-push build tests, or to turn off LIVEPATCH there. The latter not really
>> being a good idea.
>>
>> Nevertheless, as said in reply to Roger: Go ahead if you absolutely think
>> that's the only sensible way.
> 
> I had noticed that link was taking a long time.  I hadn't realised it
> was this specifically.
> 
> But to put this another way, you're arguing to intentionally avoid
> fixing a sharp corner, because there's a perf issue in Binutils.

I understand there is a sharp corner, yet recall I had suggested an
alternative. People changing defaults are responsible for what they're
doing, after all.

> I will note that it was you who forced the generation of xen.efi on
> everyone, even those who didn't want it, without any knob to turn it off.

True, but if there's desire to turn it off, a knob can always be added.
EFI support pre-dates the introduction of Kconfig by several years, iirc.

Jan