* [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask()
@ 2023-03-28 21:25 Philippe Mathieu-Daudé
2023-03-30 10:31 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-28 21:25 UTC (permalink / raw)
To: qemu-devel, Richard Henderson
Cc: qemu-arm, Peter Maydell, Claudio Fontana, Alex Bennée,
Fabiano Rosas, Philippe Mathieu-Daudé
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);
+}
/* Add the cpreg definitions for debug related system registers */
void define_debug_regs(ARMCPU *cpu);
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index ec1e07f139..c1f7e8c934 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -230,8 +230,11 @@ int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg)
{
bool is_data = !(reg & 1);
bool is_high = reg & 2;
- uint64_t mask = pauth_ptr_mask(env, -is_high, is_data);
- return gdb_get_reg64(buf, mask);
+ ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+ ARMVAParameters param;
+
+ param = aa64_va_parameters(env, -is_high, mmu_idx, is_data);
+ return gdb_get_reg64(buf, pauth_ptr_mask(param));
}
default:
return 0;
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 20f347332d..de067fa716 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -339,17 +339,9 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
return pac | ext | ptr;
}
-static uint64_t pauth_ptr_mask_internal(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);
-}
-
static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
{
- uint64_t mask = pauth_ptr_mask_internal(param);
+ uint64_t mask = pauth_ptr_mask(param);
/* Note that bit 55 is used whether or not the regime has 2 ranges. */
if (extract64(ptr, 55, 1)) {
@@ -359,14 +351,6 @@ static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
}
}
-uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data)
-{
- ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
- ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, data);
-
- return pauth_ptr_mask_internal(param);
-}
-
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
ARMPACKey *key, bool data, int keynumber)
{
--
2.38.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask()
2023-03-28 21:25 [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask() Philippe Mathieu-Daudé
@ 2023-03-30 10:31 ` Peter Maydell
2023-03-30 10:34 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2023-03-30 10:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Richard Henderson, qemu-arm, Claudio Fontana,
Alex Bennée, Fabiano Rosas
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 ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask()
2023-03-30 10:31 ` Peter Maydell
@ 2023-03-30 10:34 ` Peter Maydell
2023-03-30 13:00 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2023-03-30 10:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Richard Henderson, qemu-arm, Claudio Fontana,
Alex Bennée, Fabiano Rosas
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.
*/
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask()
2023-03-30 10:34 ` Peter Maydell
@ 2023-03-30 13:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-30 13:00 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Richard Henderson, qemu-arm, Claudio Fontana,
Alex Bennée, Fabiano Rosas
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-30 13:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-28 21:25 [PATCH-for-8.0 v3] target/arm: Fix non-TCG build failure by inlining pauth_ptr_mask() Philippe Mathieu-Daudé
2023-03-30 10:31 ` Peter Maydell
2023-03-30 10:34 ` Peter Maydell
2023-03-30 13:00 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.