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/