[PATCH-for-8.0 v2 0/2] target/arm/gdbstub: Fix builds when TCG is disabled

[PATCH-for-8.0 v2 0/2] target/arm/gdbstub: Fix builds when TCG is disabled

[Philippe Mathieu-Daudé]

Since v1:
- Replace "Restrict aarch64_gdb_get_pauth_reg() to TCG / KVM"
  by a new patch "Inline pauth_param_mask() and pauth_ptr_mask()".

Fix when building QEMU configured with --disable-tcg:

  Undefined symbols for architecture arm64:
    "_arm_v7m_get_sp_ptr", referenced from:
        _m_sysreg_get in target_arm_gdbstub.c.o
    "_arm_v7m_mrs_control", referenced from:
        _arm_gdb_get_m_systemreg in target_arm_gdbstub.c.o
    "_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)

Philippe Mathieu-Daudé (2):
  target/arm/gdbstub: Only advertise M-profile features if TCG available
  target/arm/pauth: Inline pauth_param_mask() and pauth_ptr_mask()

 target/arm/internals.h        | 16 +++++++++++++++-
 target/arm/gdbstub.c          |  5 +++--
 target/arm/tcg/pauth_helper.c | 18 +-----------------
 3 files changed, 19 insertions(+), 20 deletions(-)

-- 
2.38.1

[PATCH-for-8.0 v2 1/2] target/arm/gdbstub: Only advertise M-profile features if TCG available

[Philippe Mathieu-Daudé]

Cortex-M profile is only emulable from TCG accelerator. Restrict
the GDBstub features to its availability in order to avoid a link
error when TCG is not enabled:

  Undefined symbols for architecture arm64:
    "_arm_v7m_get_sp_ptr", referenced from:
        _m_sysreg_get in target_arm_gdbstub.c.o
    "_arm_v7m_mrs_control", referenced from:
        _arm_gdb_get_m_systemreg in target_arm_gdbstub.c.o
  ld: symbol(s) not found for architecture arm64
  clang: error: linker command failed with exit code 1 (use -v to see invocation)

Fixes: 7d8b28b8b5 ("target/arm: Implement gdbstub m-profile systemreg and secext")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/gdbstub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 3bd86cee97..13fbe9b0d7 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -21,6 +21,7 @@
 #include "cpu.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/helpers.h"
+#include "sysemu/tcg.h"
 #include "internals.h"
 #include "cpregs.h"
 
@@ -553,7 +554,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
                                      2, "arm-vfp-sysregs.xml", 0);
         }
     }
-    if (cpu_isar_feature(aa32_mve, cpu)) {
+    if (cpu_isar_feature(aa32_mve, cpu) && tcg_enabled()) {
         gdb_register_coprocessor(cs, mve_gdb_get_reg, mve_gdb_set_reg,
                                  1, "arm-m-profile-mve.xml", 0);
     }
@@ -561,7 +562,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
                              arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
                              "system-registers.xml", 0);
 
-    if (arm_feature(env, ARM_FEATURE_M)) {
+    if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
         gdb_register_coprocessor(cs,
             arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
             arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
-- 
2.38.1

[PATCH-for-8.0 v2 2/2] target/arm/pauth: Inline pauth_param_mask() and pauth_ptr_mask()

[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)

- Rename pauth_ptr_mask_internal() as pauth_param_mask() and
  inline it in "internals.h",
- Inline pauth_ptr_mask() in "internals.h".

Fixes: e995d5cce4 ("target/arm: Implement gdbstub pauth extension")
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/internals.h        | 16 +++++++++++++++-
 target/arm/tcg/pauth_helper.c | 18 +-----------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 673519a24a..a617466fa8 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1389,6 +1389,14 @@ int exception_target_el(CPUARMState *env);
 bool arm_singlestep_active(CPUARMState *env);
 bool arm_generate_debug_exceptions(CPUARMState *env);
 
+static inline uint64_t pauth_param_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);
+}
+
 /**
  * pauth_ptr_mask:
  * @env: cpu context
@@ -1397,7 +1405,13 @@ bool arm_generate_debug_exceptions(CPUARMState *env);
  *
  * 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(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_param_mask(param);
+}
 
 /* Add the cpreg definitions for debug related system registers */
 void define_debug_regs(ARMCPU *cpu);
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 20f347332d..c78dafda4e 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_param_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

Re: [PATCH-for-8.0 v2 2/2] target/arm/pauth: Inline pauth_param_mask() and pauth_ptr_mask()

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> 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)
>
> - Rename pauth_ptr_mask_internal() as pauth_param_mask() and
>   inline it in "internals.h",
> - Inline pauth_ptr_mask() in "internals.h".
>
> Fixes: e995d5cce4 ("target/arm: Implement gdbstub pauth extension")
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH-for-8.0 v2 2/2] target/arm/pauth: Inline pauth_param_mask() and pauth_ptr_mask()

[Richard Henderson]

On 3/28/23 06:30, Philippe Mathieu-Daudé wrote:
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 673519a24a..a617466fa8 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1389,6 +1389,14 @@ int exception_target_el(CPUARMState *env);
>   bool arm_singlestep_active(CPUARMState *env);
>   bool arm_generate_debug_exceptions(CPUARMState *env);
>   
> +static inline uint64_t pauth_param_mask(ARMVAParameters param)

Perhaps pauth_ptr_mask_param, or just pauth_ptr_mask (see below).

> +static inline 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_param_mask(param);
> +}

This is only used by gdbstub.  Just put it there.  Perhaps merge it with 
aarch64_gdb_get_pauth_reg directly, so that we can use the simpler 'pauth_ptr_mask' name 
above.


Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~