Re: [PATCH 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section

[Juergen Gross <jgross@xxxxxxxx>]

On 07.03.23 11:31, Jan Beulich wrote:
> On 07.03.2023 07:32, Juergen Gross wrote:
> > In order to support hypervisor analysis of crash dumps, xen-syms needs
> > to contain debug_info. It should be allowed to configure the hypervisor
> > to be built with CONFIG_DEBUG_INFO in non-debug builds without having
> > to enable EXPERT.
> In how far does this apply to xen.efi as well? (You can't really use
> xen-syms for crash debugging when the crash occurred with xen.efi in
> use.)
TBH I don't know what is needed for analysis of crash dumps with the xen.efi
binary.

> > Using a rather oldish gcc (7.5) it was verified that code generation
> > doesn't really differ between CONFIG_DEBUG_INFO on or off without
> > CONFIG_DEBUG being set (only observed differences were slightly
> > different symbol addresses, verified via "objdump -d"). The old gcc
> > version selection was based on the assumption, that newer gcc won't
> > regress in this regard.
> This is good to know, but I'm still curious about the mentioned
> differences in symbol addresses: If code generation didn't change, what
> caused addresses to differ? Is that merely because individual functions
> or objects are emitted in different order by the compiler? (If so I'd
> be inclined to infer that comparing generated code must have been
> quite a bit of effort, as first of all you would have had to undo that
> re-ordering.)
I did a simple diff of the two disassembly outputs and got only small
differences for %rip relative addresses (the differences were in the
range of +/- 32 bytes).

> The other thing to at least mention here is that with new enough
> binutils, when Dwarf debug info can be enabled for keeping in xen.efi,
> linking time of xen.efi increases quite a bit with DEBUG_INFO=y (which
> is a result of linking ELF objects into a non-ELF binary, when at
> least GNU ld optimizes only the ELF -> ELF case when processing the
> [massive amount of] relocations).
Okay, I can add this.

> > So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.
> Isn't the prior DEBUG dependency as relevant?
Not for the case "non-debug builds" the patch is addressing.

> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -11,6 +11,13 @@ config DEBUG
> >   
> >   	  You probably want to say 'N' here.
> >   
> > +config DEBUG_INFO
> > +       bool "Compile Xen with debug info"
> > +       default DEBUG
> > +       ---help---
> Nit: Even if just code movement, please use "help" in the moved
> instance.
Okay.

> > +         If you say Y here the resulting Xen will include debugging info
> > +         resulting in a larger binary image.
> > +
> >   if DEBUG || EXPERT
> The new placement isn't very helpful when considering some of the ways
> kconfig data is presented. At least for the non-graphical presentation
> it used to be the case that hierarchies were presented properly only
> when dependencies immediately followed their dependents (i.e. here:
> DEBUG is a dependent of everything inside the "if" above). Therefore I
> think rather than moving the block up you may better move it down past
> the "endif".
Fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature