Re: [PATCH v14 16/19] unwind_user/sframe: Add support for SFrame V3 flexible FDEs
Jens Remus <[email protected]> Wed, 13 May 2026 15:50:29 +0200
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Organization | IBM Deutschland Research & Development GmbH |
| Message-ID | <[email protected]> |
On 5/13/2026 8:26 AM, Indu Bhagat wrote: > On 2026-05-07 08:30, Jens Remus wrote: >> On 5/5/2026 8:55 PM, [email protected] wrote: >> static __always_inline int >> >>>> sframe_init_rule_data(struct unwind_user_rule_data *rule_data, >>>> - s32 offset) >>>> + u32 ctlword, s32 offset) >>>> { >>>> - if (offset) { >>>> - rule_data->rule = UNWIND_USER_RULE_CFA_OFFSET_DEREF; >>>> - rule_data->offset = offset; >>>> - } else { >>>> + bool deref_p = SFRAME_V3_FLEX_FDE_CTRLWORD_DEREF_P(ctlword); >> i>> + bool reg_p = SFRAME_V3_FLEX_FDE_CTRLWORD_REG_P(ctlword); >> >> bool reserved_p = SFRAME_V3_FLEX_FDE_CTRLWORD_RESERVED_P(ctlword); >> unsigned int regnum = SFRAME_V3_FLEX_FDE_CTRLWORD_REGNUM(ctlword); >> >>>> + >>>> + if (!ctlword && !offset) { >>>> rule_data->rule = UNWIND_USER_RULE_RETAIN; >>>> + return; >> >> return 0; >> >>>> + } >> >> if (reserved_p) >> return -EINVAL; >> >> @Indu: Although the SFrame spec does only state "unused Unused bit." I >> think it would be good for the logic to reject any value other than zero >> as that could be used in future extensions of the SFrame format. Do you >> agree? >> > > For unused bits, it will be ideal to not associate any "observable" > behaviour on the consumer side, before the bits find their use. So I > suggest the recommended way is to ignore the unused bits completely, > i.e., mask them out and not report any error (if they are set). I disagree. Masking unused/reserved bits would cause the unwinder to ignore those even if they get assigned a meaning in the future that it needs to respect. It could thus cause the unwinder to erroneously proceed with wrong results instead of stopping. > > On the producer side, unused bits should be set to zero (which I think > we are doing). Makes sense. I also think that this is how GNU assembler/linker behave. > >>>> + if (reg_p) { >>>> + unsigned int regnum = SFRAME_V3_FLEX_FDE_CTRLWORD_REGNUM(ctlword); >> >> Drop line above. >> >>>> + >>>> + rule_data->rule = UNWIND_USER_RULE_REG_OFFSET; >>>> + rule_data->regnum = regnum; >>>> + } else { >> >> if (regnum) >> return -EINVAL; >> >> @Indu: Is that too strict? The SFrame spec does only state that regnum >> is "Effective only if reg_p is 1.". Shall I better ignore any non-zero >> value if reg_p=0? >> > > Yes, ignoring the bits if reg_p = 0 is better behaviour. I agree. This is different from the unused/reserved bit, as the spec clearly states that the value of regnum is only effective if reg_p=1. Regards, Jens -- Jens Remus Linux on Z Development (D3303) [email protected] / [email protected] IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/