2024-02-04 12:20:15

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] x86: select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

It took me some time to understand the purpose of the tricky code at
the end of arch/x86/Kconfig.debug.

Without it, the following would be shown:

WARNING: unmet direct dependencies detected for FRAME_POINTER

because commit 81d387190039 ("x86/kconfig: Consolidate unwinders into
multiple choice selection") removed 'select ARCH_WANT_FRAME_POINTERS'.

The correct and more straightforward approach should have been to move
it where 'select FRAME_POINTER' is located.

Several architectures properly handle the conditional selection of
ARCH_WANT_FRAME_POINTERS. For example, 'config UNWINDER_FRAME_POINTER'
in arch/arm/Kconfig.debug.

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/x86/Kconfig.debug | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index c5d614d28a75..74777a97e394 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -248,6 +248,7 @@ config UNWINDER_ORC

config UNWINDER_FRAME_POINTER
bool "Frame pointer unwinder"
+ select ARCH_WANT_FRAME_POINTERS
select FRAME_POINTER
help
This option enables the frame pointer unwinder for unwinding kernel
@@ -271,7 +272,3 @@ config UNWINDER_GUESS
overhead.

endchoice
-
-config FRAME_POINTER
- depends on !UNWINDER_ORC && !UNWINDER_GUESS
- bool
--
2.40.1



2024-03-20 15:31:16

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] x86: select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

Ping?


On Sun, Feb 4, 2024 at 9:20 PM Masahiro Yamada <[email protected]> wrote:
>
> It took me some time to understand the purpose of the tricky code at
> the end of arch/x86/Kconfig.debug.
>
> Without it, the following would be shown:
>
> WARNING: unmet direct dependencies detected for FRAME_POINTER
>
> because commit 81d387190039 ("x86/kconfig: Consolidate unwinders into
> multiple choice selection") removed 'select ARCH_WANT_FRAME_POINTERS'.
>
> The correct and more straightforward approach should have been to move
> it where 'select FRAME_POINTER' is located.
>
> Several architectures properly handle the conditional selection of
> ARCH_WANT_FRAME_POINTERS. For example, 'config UNWINDER_FRAME_POINTER'
> in arch/arm/Kconfig.debug.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/x86/Kconfig.debug | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
> index c5d614d28a75..74777a97e394 100644
> --- a/arch/x86/Kconfig.debug
> +++ b/arch/x86/Kconfig.debug
> @@ -248,6 +248,7 @@ config UNWINDER_ORC
>
> config UNWINDER_FRAME_POINTER
> bool "Frame pointer unwinder"
> + select ARCH_WANT_FRAME_POINTERS
> select FRAME_POINTER
> help
> This option enables the frame pointer unwinder for unwinding kernel
> @@ -271,7 +272,3 @@ config UNWINDER_GUESS
> overhead.
>
> endchoice
> -
> -config FRAME_POINTER
> - depends on !UNWINDER_ORC && !UNWINDER_GUESS
> - bool
> --
> 2.40.1
>


--
Best Regards
Masahiro Yamada

2024-05-17 07:19:02

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] x86: select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

Hi, x86 maintainers


Please check this.


And, please also note the current code is incorrect,
and it may get broken with future Kconfig refactoring.





On Thu, Mar 21, 2024 at 12:30 AM Masahiro Yamada <[email protected]> wrote:
>
> Ping?
>
>
> On Sun, Feb 4, 2024 at 9:20 PM Masahiro Yamada <[email protected]> wrote:
> >
> > It took me some time to understand the purpose of the tricky code at
> > the end of arch/x86/Kconfig.debug.
> >
> > Without it, the following would be shown:
> >
> > WARNING: unmet direct dependencies detected for FRAME_POINTER
> >
> > because commit 81d387190039 ("x86/kconfig: Consolidate unwinders into
> > multiple choice selection") removed 'select ARCH_WANT_FRAME_POINTERS'.
> >
> > The correct and more straightforward approach should have been to move
> > it where 'select FRAME_POINTER' is located.
> >
> > Several architectures properly handle the conditional selection of
> > ARCH_WANT_FRAME_POINTERS. For example, 'config UNWINDER_FRAME_POINTER'
> > in arch/arm/Kconfig.debug.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > arch/x86/Kconfig.debug | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
> > index c5d614d28a75..74777a97e394 100644
> > --- a/arch/x86/Kconfig.debug
> > +++ b/arch/x86/Kconfig.debug
> > @@ -248,6 +248,7 @@ config UNWINDER_ORC
> >
> > config UNWINDER_FRAME_POINTER
> > bool "Frame pointer unwinder"
> > + select ARCH_WANT_FRAME_POINTERS
> > select FRAME_POINTER
> > help
> > This option enables the frame pointer unwinder for unwinding kernel
> > @@ -271,7 +272,3 @@ config UNWINDER_GUESS
> > overhead.
> >
> > endchoice
> > -
> > -config FRAME_POINTER
> > - depends on !UNWINDER_ORC && !UNWINDER_GUESS
> > - bool
> > --
> > 2.40.1
> >
>
>
> --
> Best Regards
> Masahiro Yamada



--
Best Regards
Masahiro Yamada

2024-05-18 01:47:10

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86: select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

On Sun, Feb 04, 2024 at 09:20:03PM +0900, Masahiro Yamada wrote:
> It took me some time to understand the purpose of the tricky code at
> the end of arch/x86/Kconfig.debug.
>
> Without it, the following would be shown:
>
> WARNING: unmet direct dependencies detected for FRAME_POINTER
>
> because commit 81d387190039 ("x86/kconfig: Consolidate unwinders into
> multiple choice selection") removed 'select ARCH_WANT_FRAME_POINTERS'.
>
> The correct and more straightforward approach should have been to move
> it where 'select FRAME_POINTER' is located.
>
> Several architectures properly handle the conditional selection of
> ARCH_WANT_FRAME_POINTERS. For example, 'config UNWINDER_FRAME_POINTER'
> in arch/arm/Kconfig.debug.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Looks good, thanks for fixing that!

Fixes: 81d387190039 ("x86/kconfig: Consolidate unwinders into multiple choice selection")
Acked-by: Josh Poimboeuf <[email protected]>

--
Josh

Subject: [tip: x86/urgent] x86/kconfig: Select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 66ee3636eddcc82ab82b539d08b85fb5ac1dff9b
Gitweb: https://git.kernel.org/tip/66ee3636eddcc82ab82b539d08b85fb5ac1dff9b
Author: Masahiro Yamada <[email protected]>
AuthorDate: Sun, 04 Feb 2024 21:20:03 +09:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 20 May 2024 11:37:23 +02:00

x86/kconfig: Select ARCH_WANT_FRAME_POINTERS again when UNWINDER_FRAME_POINTER=y

It took me some time to understand the purpose of the tricky code at
the end of arch/x86/Kconfig.debug.

Without it, the following would be shown:

WARNING: unmet direct dependencies detected for FRAME_POINTER

because

81d387190039 ("x86/kconfig: Consolidate unwinders into multiple choice selection")

removed 'select ARCH_WANT_FRAME_POINTERS'.

The correct and more straightforward approach should have been to move
it where 'select FRAME_POINTER' is located.

Several architectures properly handle the conditional selection of
ARCH_WANT_FRAME_POINTERS. For example, 'config UNWINDER_FRAME_POINTER'
in arch/arm/Kconfig.debug.

Fixes: 81d387190039 ("x86/kconfig: Consolidate unwinders into multiple choice selection")
Signed-off-by: Masahiro Yamada <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/Kconfig.debug | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index c5d614d..74777a9 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -248,6 +248,7 @@ config UNWINDER_ORC

config UNWINDER_FRAME_POINTER
bool "Frame pointer unwinder"
+ select ARCH_WANT_FRAME_POINTERS
select FRAME_POINTER
help
This option enables the frame pointer unwinder for unwinding kernel
@@ -271,7 +272,3 @@ config UNWINDER_GUESS
overhead.

endchoice
-
-config FRAME_POINTER
- depends on !UNWINDER_ORC && !UNWINDER_GUESS
- bool