Re: [PATCH 1/4] arm64: signal: Preserve POR_EL0 if poe_context is missing

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 22/04/2026 14:19, Will Deacon wrote:
>> @@ -74,8 +76,12 @@ struct rt_sigframe_user_layout {
>>   * This state needs to be carefully managed to ensure that it doesn't cause
>>   * uaccess to fail when setting up the signal frame, and the signal handler
>>   * itself also expects a well-defined state when entered.
>> + *
>> + * The valid_fields member is a bitfield (see UA_STATE_HAS_*), specifying which
>> + * of the remaining fields is valid (has been set to a value).
>>   */
>>  struct user_access_state {
>> +	unsigned int valid_fields;
>>  	u64 por_el0;
>>  };
> Do you think it would be worth adding some accessors to make it easier
> to keep the flags in sync? For example:
>
> /* Stores por_el0 into uas->por_el0 and sets UA_STATE_HAS_POR_EL0 */
> void set_ua_state_por_el0(struct user_access_state *uas, u64 por_el0);
>
> /*
>  * If UA_STATE_HAS_POR_EL0, *por_el0 = uas->por_el0 and return 0.
>  * Otherwise, return -ENOENT.
>  */
> int get_ua_state_por_el0(struct user_access_state *uas, u64 *por_el0);
>
> WDYT?

I did get a feeling having helpers would be a good idea. I wonder if
getters/setters aren't a bit overkill though, as they make accesses to
the struct more cumbersome and we'd need a pair for every member (unless
we use some macro magic). Maybe it would be sufficient to have say
ua_state_has_field(POR_EL0) to check if the bit is set, and
ua_state_set_field_valid(POR_EL0) to set the bit?

>> @@ -1095,7 +1104,7 @@ SYSCALL_DEFINE0(rt_sigreturn)
>>  {
>>  	struct pt_regs *regs = current_pt_regs();
>>  	struct rt_sigframe __user *frame;
>> -	struct user_access_state ua_state;
>> +	struct user_access_state ua_state = {0};
> nit: {} should do (no need for the '0'). Same in setup_rt_frame().

Will change it, I have some vague recollection that GCC and Clang
disagreed about the meaning of {}... but that's probably fixed nowadays.

Thanks for the review!

- Kevin