Re: [PATCH v16 16/20] unwind_user/sframe: Add support for SFrame V3 flexible FDEs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mismatch between SFrame V3 flexible FDE control word specification in the commit message and the actual implementation.
- [Low] The patch uses undefined macros `SFRAME_REG_SP` and `SFRAME_REG_FP` and includes a missing architecture header, breaking the build.
--

commit 82ce04f2a50a8ff116e0ca155714efff95332739
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/sframe: Add support for SFrame V3 flexible FDEs

This commit introduces support for SFrame V3 flexible FDEs, which encode CFA,
RA, and FP tracking information using FRE data words or padding words. It
adds logic to read flexible FDE control words and initialize the appropriate
CFA and rule data structures.

>     This enables the following flexible CFA and FP/RA recovery rules:
>     - CFA = register + offset       // reg_p=1, deref_p=0
>     - CFA = *(register + offset)    // reg_p=1, deref_p=1
>     - FP/RA = *(CFA + offset)       // reg_p=0, deref_p=0

[Severity: Medium]
Is this description in the commit message accurate?

The commit message states that the FP/RA rule uses reg_p=0, deref_p=0.
However, the implementation for this rule in __read_flex_fde_fre_datawords()
uses ra_ctl = 2, which corresponds to deref_p=1. 

Also, the commit message later notes that reg_p=0 and deref_p=0 is actually
an invalid padding word.

Could the commit message be updated to match the code's behavior to avoid
confusing future maintainers debugging the SFrame unwinder?

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index daa97d8b02316..b623dca072daa 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -12,6 +12,7 @@
>  #include <linux/mm.h>
>  #include <linux/string_helpers.h>
>  #include <linux/sframe.h>
> +#include <asm/unwind_user_sframe.h>
>  #include <linux/unwind_user_types.h>

[Severity: Low]
Does this include break the build for intermediate commits?

This architecture header, along with the SFRAME_REG_SP and SFRAME_REG_FP
macros used later in this patch, do not appear to exist in the tree at this
commit.

Although this is fixed later in the patch series by the commit that enables
sframe unwinding on x86, missing headers and undefined macros break git
bisectability. 

Should these definitions be introduced in an earlier patch or within this
one so that this patch compiles cleanly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521142546.3908498-1-jremus@linux.ibm.com?part=16