| Message ID | 20241107084927.37748-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/Kconfig: livepatch-build-tools requires debug information | expand |
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 ?
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.
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
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.
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
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
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
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
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
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
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(+)