Peter Maydell <peter.mayd...@linaro.org> writes:
> On Mon, 16 Jun 2025 at 11:02, Alex Bennée <alex.ben...@linaro.org> wrote:
>> The code just does:
>>
>> static bool trans_RETA(DisasContext *s, arg_reta *a)
>> {
>> TCGv_i64 dst;
>>
>> dst = auth_branch_target(s, cpu_reg(s, 30), cpu_X[31], !a->m);
>> gen_a64_set_pc(s, dst);
>> s->base.is_jmp = DISAS_JUMP;
>> return true;
>> }
>>
>>
>> Where auth_branch_target does:
>>
>> static TCGv_i64 auth_branch_target(DisasContext *s, TCGv_i64 dst,
>> TCGv_i64 modifier, bool use_key_a)
>> {
>> TCGv_i64 truedst;
>> /*
>> * Return the branch target for a BRAA/RETA/etc, which is either
>> * just the destination dst, or that value with the pauth check
>> * done and the code removed from the high bits.
>> */
>> if (!s->pauth_active) {
>> return dst;
>> }
>>
>> truedst = tcg_temp_new_i64();
>> if (use_key_a) {
>> gen_helper_autia_combined(truedst, tcg_env, dst, modifier);
>> } else {
>> gen_helper_autib_combined(truedst, tcg_env, dst, modifier);
>> }
>> return truedst;
>> }
>>
>> Essentially if no pauth is active then just returns dest. I think this
>> matches the pseudocode:
>> [...]
>> which has no explicit UNDEF for the instruction.
>
>
> The UNDEF is in the "Decode for all variants of this encoding"
> pseudocode for RETAA, RETAB:
>
> if !IsFeatureImplemented(FEAT_PAuth) then EndOfDecode(Decode_UNDEF);
Doh - totally missed that section which is right above Operation on the
web page :-/
> This is an accidental regression introduced in commit 0ebbe9021254f
> where we converted these insns to decodetree. The old hand
> written decoder has:
>
> - if (!dc_isar_feature(aa64_pauth, s)) {
> - goto do_unallocated;
> - }
>
> in the codepath for RETAA/RETAB/BRAAZ/BRABZ/BLRAAZ/BLRABZ,
> and although we put the check in the new trans_BRAZ,
> trans_BLRAZ we forgot it in trans_RETA.
Are you going to fix this up or do you want a patch?
It does still seem weird - I thought the point of PAUTH using the NOP
hint space was to allow for binaries to optionally add pointer
authentication? I guess if it has to pair with RETAA anyway that doesn't
work.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
