Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] xen: some CONFIG_DEBUG_INFO changes
@ 2023-04-03 16:28 Juergen Gross
  2023-04-03 16:28 ` [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section Juergen Gross
  2023-04-03 16:28 ` [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text Juergen Gross
  0 siblings, 2 replies; 8+ messages in thread
From: Juergen Gross @ 2023-04-03 16:28 UTC (permalink / raw
  To: xen-devel
  Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu

Enabling crash dump analysis of the hypervisor requires the hypervisor
having been built with CONFIG_DEBUG_INFO enabled. Today this requires
either CONFIG_DEBUG or CONFIG_EXPERT set, which are both not security
supported.

This small series changes that in order to allow security supported
Xen builds with the capability to do crash dump analysis via the
"crash" tool.

Note that due to problems with test machines proper support for EFI
booted systems hasn't been verified, so this will likely need some more
work.

Changes in V2:
- comments addressed

Juergen Gross (2):
  xen: move CONFIG_DEBUG_INFO out of EXPERT section
  xen: update CONFIG_DEBUG_INFO help text

 xen/Kconfig.debug | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

-- 
2.35.3



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section
  2023-04-03 16:28 [PATCH v2 0/2] xen: some CONFIG_DEBUG_INFO changes Juergen Gross
@ 2023-04-03 16:28 ` Juergen Gross
  2023-04-05 13:14   ` Jan Beulich
  2023-04-03 16:28 ` [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text Juergen Gross
  1 sibling, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2023-04-03 16:28 UTC (permalink / raw
  To: xen-devel
  Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu

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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text
  2023-04-03 16:28 [PATCH v2 0/2] xen: some CONFIG_DEBUG_INFO changes Juergen Gross
  2023-04-03 16:28 ` [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section Juergen Gross
@ 2023-04-03 16:28 ` Juergen Gross
  2023-04-04  9:09   ` Jan Beulich
  1 sibling, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2023-04-03 16:28 UTC (permalink / raw
  To: xen-devel
  Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu

Update the help text of the CONFIG_DEBUG_INFO option to be a little
bit more specific.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- expand help text, especially mentioning INSTALL_EFI_STRIP
  (Jan Beulich)
---
 xen/Kconfig.debug | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index e0d773d347..e6e609f813 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -15,8 +15,14 @@ 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.
+	  Say Y here if you want to build Xen with debug information. This
+	  information is needed e.g. for doing crash dump analysis of the
+	  hypervisor via the "crash" tool.
+	  Saying Y will increase the size of the xen-syms and xen.efi
+	  binaries. In case the space on the EFI boot partition is rather
+	  limited, you may want to make use of the INSTALL_EFI_STRIP make
+	  variable when building the hypervisor, in order to strip xen.efi
+	  before installing it to the EFI partition.
 
 if DEBUG || EXPERT
 
-- 
2.35.3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text
  2023-04-03 16:28 ` [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text Juergen Gross
@ 2023-04-04  9:09   ` Jan Beulich
  2023-04-05 11:24     ` Juergen Gross
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2023-04-04  9:09 UTC (permalink / raw
  To: Juergen Gross
  Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
	Wei Liu, xen-devel

On 03.04.2023 18:28, Juergen Gross wrote:
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -15,8 +15,14 @@ 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.
> +	  Say Y here if you want to build Xen with debug information. This
> +	  information is needed e.g. for doing crash dump analysis of the
> +	  hypervisor via the "crash" tool.
> +	  Saying Y will increase the size of the xen-syms and xen.efi
> +	  binaries. In case the space on the EFI boot partition is rather
> +	  limited, you may want to make use of the INSTALL_EFI_STRIP make
> +	  variable when building the hypervisor, in order to strip xen.efi
> +	  before installing it to the EFI partition.

Hmm, INSTALL_EFI_STRIP is only a courtesy to developers wanting to install
xen.efi directly into the EFI partition. It wouldn't affect the normal
flow, and hence I think this wants expressing here such that both kinds of
people have at least a hint what they need to do. I.e. in the normal case
they'd need to adjust the way xen.efi is "propagated" from its installed
location onto the EFI partition, to do the desired stripping at that time.

Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text
  2023-04-04  9:09   ` Jan Beulich
@ 2023-04-05 11:24     ` Juergen Gross
  2023-04-05 13:04       ` Jan Beulich
  0 siblings, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2023-04-05 11:24 UTC (permalink / raw
  To: Jan Beulich
  Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
	Wei Liu, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1766 bytes --]

On 04.04.23 11:09, Jan Beulich wrote:
> On 03.04.2023 18:28, Juergen Gross wrote:
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -15,8 +15,14 @@ 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.
>> +	  Say Y here if you want to build Xen with debug information. This
>> +	  information is needed e.g. for doing crash dump analysis of the
>> +	  hypervisor via the "crash" tool.
>> +	  Saying Y will increase the size of the xen-syms and xen.efi
>> +	  binaries. In case the space on the EFI boot partition is rather
>> +	  limited, you may want to make use of the INSTALL_EFI_STRIP make
>> +	  variable when building the hypervisor, in order to strip xen.efi
>> +	  before installing it to the EFI partition.
> 
> Hmm, INSTALL_EFI_STRIP is only a courtesy to developers wanting to install
> xen.efi directly into the EFI partition. It wouldn't affect the normal
> flow, and hence I think this wants expressing here such that both kinds of
> people have at least a hint what they need to do. I.e. in the normal case
> they'd need to adjust the way xen.efi is "propagated" from its installed
> location onto the EFI partition, to do the desired stripping at that time.

Would you be fine with:

   In case the space on the EFI boot partition is rather
   limited, you may want to install a stripped variant of xen.efi in
   the EFI boot partition (look for "INSTALL_EFI_STRIP" in
   docs/misc/efi.pandoc for more information - when not using
   "make install-xen" for installing xen.efi, stripping needs to be
   done outside the Xen build environment).


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text
  2023-04-05 11:24     ` Juergen Gross
@ 2023-04-05 13:04       ` Jan Beulich
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2023-04-05 13:04 UTC (permalink / raw
  To: Juergen Gross
  Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
	Wei Liu, xen-devel

On 05.04.2023 13:24, Juergen Gross wrote:
> On 04.04.23 11:09, Jan Beulich wrote:
>> On 03.04.2023 18:28, Juergen Gross wrote:
>>> --- a/xen/Kconfig.debug
>>> +++ b/xen/Kconfig.debug
>>> @@ -15,8 +15,14 @@ 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.
>>> +	  Say Y here if you want to build Xen with debug information. This
>>> +	  information is needed e.g. for doing crash dump analysis of the
>>> +	  hypervisor via the "crash" tool.
>>> +	  Saying Y will increase the size of the xen-syms and xen.efi
>>> +	  binaries. In case the space on the EFI boot partition is rather
>>> +	  limited, you may want to make use of the INSTALL_EFI_STRIP make
>>> +	  variable when building the hypervisor, in order to strip xen.efi
>>> +	  before installing it to the EFI partition.
>>
>> Hmm, INSTALL_EFI_STRIP is only a courtesy to developers wanting to install
>> xen.efi directly into the EFI partition. It wouldn't affect the normal
>> flow, and hence I think this wants expressing here such that both kinds of
>> people have at least a hint what they need to do. I.e. in the normal case
>> they'd need to adjust the way xen.efi is "propagated" from its installed
>> location onto the EFI partition, to do the desired stripping at that time.
> 
> Would you be fine with:
> 
>    In case the space on the EFI boot partition is rather
>    limited, you may want to install a stripped variant of xen.efi in
>    the EFI boot partition (look for "INSTALL_EFI_STRIP" in
>    docs/misc/efi.pandoc for more information - when not using
>    "make install-xen" for installing xen.efi, stripping needs to be
>    done outside the Xen build environment).

SGTM, thanks.

Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section
  2023-04-03 16:28 ` [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section Juergen Gross
@ 2023-04-05 13:14   ` Jan Beulich
  2023-04-05 13:44     ` Juergen Gross
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2023-04-05 13:14 UTC (permalink / raw
  To: Juergen Gross
  Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
	Wei Liu, xen-devel

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section
  2023-04-05 13:14   ` Jan Beulich
@ 2023-04-05 13:44     ` Juergen Gross
  0 siblings, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2023-04-05 13:44 UTC (permalink / raw
  To: Jan Beulich
  Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
	Wei Liu, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2794 bytes --]

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


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-04-05 13:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-03 16:28 [PATCH v2 0/2] xen: some CONFIG_DEBUG_INFO changes Juergen Gross
2023-04-03 16:28 ` [PATCH v2 1/2] xen: move CONFIG_DEBUG_INFO out of EXPERT section Juergen Gross
2023-04-05 13:14   ` Jan Beulich
2023-04-05 13:44     ` Juergen Gross
2023-04-03 16:28 ` [PATCH v2 2/2] xen: update CONFIG_DEBUG_INFO help text Juergen Gross
2023-04-04  9:09   ` Jan Beulich
2023-04-05 11:24     ` Juergen Gross
2023-04-05 13:04       ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).