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

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

[Philippe Mathieu-Daudé]

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: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG
  target/arm/gdbstub: Only advertise M-profile features if TCG available

 target/arm/gdbstub.c   | 7 ++++---
 target/arm/gdbstub64.c | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.38.1

[PATCH-for-8.0 1/2] target/arm/gdbstub: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG

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

Restrict aarch64_gdb_get_pauth_reg() to TCG 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)

Fixes: e995d5cce4 ("target/arm: Implement gdbstub pauth extension")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/gdbstub.c   | 3 ++-
 target/arm/gdbstub64.c | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 3bd86cee97..655369dc74 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"
 
@@ -526,7 +527,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
          * crash if they see this XML from QEMU; disable it for the 8.0
          * release, pending a better solution.
          */
-        if (isar_feature_aa64_pauth(&cpu->isar)) {
+        if (isar_feature_aa64_pauth(&cpu->isar) && tcg_enabled()) {
             gdb_register_coprocessor(cs, aarch64_gdb_get_pauth_reg,
                                      aarch64_gdb_set_pauth_reg,
                                      4, "aarch64-pauth.xml", 0);
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index ec1e07f139..3bb0923cbf 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,6 +210,8 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
     return 0;
 }
 
+#ifdef CONFIG_TCG
+
 int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg)
 {
     switch (reg) {
@@ -244,6 +246,8 @@ int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg)
     return 0;
 }
 
+#endif
+
 static void output_vector_union_type(GString *s, int reg_width,
                                      const char *name)
 {
-- 
2.38.1

Re: [PATCH-for-8.0 1/2] target/arm/gdbstub: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG

[Richard Henderson]

On 3/22/23 07:29, Philippe Mathieu-Daudé 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.
> 
> Restrict aarch64_gdb_get_pauth_reg() to TCG 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)

I guess we should move this function somewhere else, because KVM can certainly use pauth, 
and we do want to be able to produce nice backtraces with gdb.


r~

Re: [PATCH-for-8.0 1/2] target/arm/gdbstub: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG

[Philippe Mathieu-Daudé]

On 22/3/23 16:15, Richard Henderson wrote:
> On 3/22/23 07:29, Philippe Mathieu-Daudé 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.
>>
>> Restrict aarch64_gdb_get_pauth_reg() to TCG 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)
> 
> I guess we should move this function somewhere else, because KVM can 
> certainly use pauth, and we do want to be able to produce nice 
> backtraces with gdb.

I ended inlining it (along with pauth_ptr_mask_internal,
renamed as pauth_param_mask):

https://lore.kernel.org/qemu-devel/20230328133054.6553-3-philmd@linaro.org/

[PATCH-for-8.0 2/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")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/gdbstub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 655369dc74..2986ed6fc8 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -554,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);
     }
@@ -562,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

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

[Richard Henderson]

On 3/22/23 07:29, Philippe Mathieu-Daudé wrote:
> 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")
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/gdbstub.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

r~

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

[Alex Bennée]

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

> 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")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

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

[Philippe Mathieu-Daudé]

On 22/3/23 15:29, Philippe Mathieu-Daudé wrote:
> 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)

Beside having the non-TCG configs tested in CI, (I think) we can avoid
such breakage by moving the TCG-specific declarations from
target/arm/internals.h to some target/arm/tcg/tcg-internals.h header.
(target/arm/internals.h is 1400+ LoC anyway). Worth it?

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

[Fabiano Rosas]

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

> On 22/3/23 15:29, Philippe Mathieu-Daudé wrote:
>> 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)
>
> Beside having the non-TCG configs tested in CI, (I think) we can avoid
> such breakage by moving the TCG-specific declarations from
> target/arm/internals.h to some target/arm/tcg/tcg-internals.h header.
> (target/arm/internals.h is 1400+ LoC anyway). Worth it?

I think it would be worth it. That still leaves the issue of these small
functions that are semantically related to what is being emulated but
don't need any TCG-specific symbols. It would be nice if we had a
default approach for where to put them. Without going back to sticking
everything in helper.c, of course. =)

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

[Peter Maydell]

On Wed, 22 Mar 2023 at 14:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> 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: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG
>   target/arm/gdbstub: Only advertise M-profile features if TCG available

I've applied patch 2 to target-arm.next; thanks.

-- PMM

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

[Philippe Mathieu-Daudé]

On 28/3/23 11:54, Peter Maydell wrote:
> On Wed, 22 Mar 2023 at 14:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> 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: Restrict aarch64_gdb_get_pauth_reg() to CONFIG_TCG
>>    target/arm/gdbstub: Only advertise M-profile features if TCG available
> 
> I've applied patch 2 to target-arm.next; thanks.

If you only take #2, then you need to squash this from #1:

-- >8 --
diff --git 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"
---

I can respin if it eases your workflow.

Thanks,

Phil.

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

[Philippe Mathieu-Daudé]

On 28/3/23 15:20, Philippe Mathieu-Daudé wrote:
> On 28/3/23 11:54, Peter Maydell wrote:
>> On Wed, 22 Mar 2023 at 14:29, Philippe Mathieu-Daudé 
>> <philmd@linaro.org> wrote:
>>>
>>> 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: Restrict aarch64_gdb_get_pauth_reg() to 
>>> CONFIG_TCG
>>>    target/arm/gdbstub: Only advertise M-profile features if TCG 
>>> available
>>
>> I've applied patch 2 to target-arm.next; thanks.
> 
> If you only take #2, then you need to squash this from #1:
> 
> -- >8 --
> diff --git 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"
> ---
> 
> I can respin if it eases your workflow.

Posted as:
https://lore.kernel.org/qemu-devel/20230328133054.6553-2-philmd@linaro.org/

Along with rth's suggestion as another patch.