Re: [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 30/3/23 12:34, Peter Maydell wrote:
> On Thu, 30 Mar 2023 at 11:31, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Tue, 28 Mar 2023 at 22:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>
>>> aarch64_gdb_get_pauth_reg() -- although disabled since commit
>>> 5787d17a42 ("target/arm: Don't advertise aarch64-pauth.xml to
>>> gdb") is still compiled in. It calls pauth_ptr_mask() which is
>>> located in target/arm/tcg/pauth_helper.c, a TCG specific helper.
>>>
>>> To avoid a linking error when TCG is not enabled:
>>>
>>>    Undefined symbols for architecture arm64:
>>>      "_pauth_ptr_mask", referenced from:
>>>          _aarch64_gdb_get_pauth_reg in target_arm_gdbstub64.c.o
>>>    ld: symbol(s) not found for architecture arm64
>>>    clang: error: linker command failed with exit code 1 (use -v to see invocation)
>>>
>>> - Inline pauth_ptr_mask() in aarch64_gdb_get_pauth_reg()
>>>    (this is the single user),
>>> - Rename pauth_ptr_mask_internal() as pauth_ptr_mask() and
>>>    inline it in "internals.h",
>>>
>>> Fixes: e995d5cce4 ("target/arm: Implement gdbstub pauth extension")
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> Supersedes: <20230328133054.6553-1-philmd@linaro.org>
>>>
>>> Since v2:
>>> - Rebased (patch #1 merged)
>>> - Addressed rth's comments
>>> - Added R-b tags
>>> ---
>>>   target/arm/internals.h        | 16 +++++++---------
>>>   target/arm/gdbstub64.c        |  7 +++++--
>>>   target/arm/tcg/pauth_helper.c | 18 +-----------------
>>>   3 files changed, 13 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/target/arm/internals.h b/target/arm/internals.h
>>> index 673519a24a..71f4c6d8a2 100644
>>> --- a/target/arm/internals.h
>>> +++ b/target/arm/internals.h
>>> @@ -1389,15 +1389,13 @@ int exception_target_el(CPUARMState *env);
>>>   bool arm_singlestep_active(CPUARMState *env);
>>>   bool arm_generate_debug_exceptions(CPUARMState *env);
>>>
>>> -/**
>>> - * pauth_ptr_mask:
>>> - * @env: cpu context
>>> - * @ptr: selects between TTBR0 and TTBR1
>>> - * @data: selects between TBI and TBID
>>> - *
>>> - * Return a mask of the bits of @ptr that contain the authentication code.
>>> - */
>>> -uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data);
>>> +static inline uint64_t pauth_ptr_mask(ARMVAParameters param)
>>> +{
>>> +    int bot_pac_bit = 64 - param.tsz;
>>> +    int top_pac_bit = 64 - 8 * param.tbi;
>>> +
>>> +    return MAKE_64BIT_MASK(bot_pac_bit, top_pac_bit - bot_pac_bit);
>>> +}
>>
>> Any reason for deleting the doc comment ?
> 
> Applied to target-arm.next with a doc comment:
> 
> /**
>   * pauth_ptr_mask:
>   * @param: parameters defining the MMU setup
>   *
>   * Return a mask of the address bits that contain the authentication code,
>   * given the MMU config defined by @param.
>   */

Thank you!