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.

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", resulting from
the different config.gz in the binary). The old gcc version selection
was based on the assumption, that newer gcc won't regress in this
regard.

So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.

It should be mentioned that there have been reports that the linking
of the xen.efi might take considerably longer with CONFIG_DEBUG_INFO
selected when using newer binutils.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- expanded commit message (Jan Beulich)
---
 xen/Kconfig.debug | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index fad3050d4f..e0d773d347 100644
--- 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
+	  If you say Y here the resulting Xen will include debugging info
+	  resulting in a larger binary image.
+
 if DEBUG || EXPERT
 
 config CRASH_DEBUG
@@ -28,13 +35,6 @@ config GDBSX
 	  If you want to enable support for debugging guests from dom0 via
 	  gdbsx then say Y.
 
-config DEBUG_INFO
-	bool "Compile Xen with debug info"
-	default y
-	---help---
-	  If you say Y here the resulting Xen will include debugging info
-	  resulting in a larger binary image.
-
 config FRAME_POINTER
 	bool "Compile Xen with frame pointers"
 	default DEBUG
-- 
2.35.3

On 03.04.2023 18:28, 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.
> 
> 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", resulting from
> the different config.gz in the binary). The old gcc version selection
> was based on the assumption, that newer gcc won't regress in this
> regard.
> 
> So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.
> 
> It should be mentioned that there have been reports that the linking
> of the xen.efi might take considerably longer with CONFIG_DEBUG_INFO
> selected when using newer binutils.

Thinking of it: Because of the need to deal with older binutils, we
already force --strip-debug as a linking option for xen.efi in
certain cases. Perhaps we could make another (x86-only) Kconfig
control which allows to force this mode even with recent binutils?
If so, would you be willing to include this right here, or should I
take care of this afterwards (or maybe even in parallel)?

> --- 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
> +	  If you say Y here the resulting Xen will include debugging info
> +	  resulting in a larger binary image.
> +
>  if DEBUG || EXPERT

Just to repeat my v1 comment (to which your response was "Fine with me"):

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".

Jan

On 05.04.23 15:14, Jan Beulich wrote:
> On 03.04.2023 18:28, 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.
>>
>> 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", resulting from
>> the different config.gz in the binary). The old gcc version selection
>> was based on the assumption, that newer gcc won't regress in this
>> regard.
>>
>> So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.
>>
>> It should be mentioned that there have been reports that the linking
>> of the xen.efi might take considerably longer with CONFIG_DEBUG_INFO
>> selected when using newer binutils.
> 
> Thinking of it: Because of the need to deal with older binutils, we
> already force --strip-debug as a linking option for xen.efi in
> certain cases. Perhaps we could make another (x86-only) Kconfig
> control which allows to force this mode even with recent binutils?
> If so, would you be willing to include this right here, or should I
> take care of this afterwards (or maybe even in parallel)?

I'm planning to do the EFI side in the next days, hoping that I can setup
the test system.

I'd include that additional config option in the probably needed patch(es)
then (crash isn't happy with xen.efi anyway, so I'll need some kind of
xen-syms.efi or whatever you want to call it).

> 
>> --- 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
>> +	  If you say Y here the resulting Xen will include debugging info
>> +	  resulting in a larger binary image.
>> +
>>   if DEBUG || EXPERT
> 
> Just to repeat my v1 comment (to which your response was "Fine with me"):
> 
> 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".

Oh, I seem to have missed that last paragraph when going through the thread.

Will correct it.


Juergen