[PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB

Vacha Bhavsar posted 3 patches 3 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB
Posted by Vacha Bhavsar 3 months, 3 weeks ago
This patch adds a test case to test SME register exposure to
a remote gdb debugging session. This test simply sets and
reads SME registers.

Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
---
Changes since v5:
- added copyright and SPDX line
- added functionality to avoid casting a gdb.Value object
to int when testing the za quadwords to address bug found
during review, this change is declared to users via a
warning message included in the test results file
run-gdbstub-sysregs-sme.out
---
 configure                             |  11 ++
 tests/tcg/aarch64/Makefile.target     |  33 +++++-
 tests/tcg/aarch64/gdbstub/test-sme.py | 165 ++++++++++++++++++++++++++
 3 files changed, 208 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py

diff --git a/configure b/configure
index 274a778764..9e2ae174dc 100755
--- a/configure
+++ b/configure
@@ -1839,6 +1839,17 @@ for target in $target_list; do
           echo "GDB=$gdb_bin" >> $config_target_mak
       fi
 
+      if test "${gdb_arches#*$arch}" != "$gdb_arches" && version_ge $gdb_version 14.1; then
+          echo "GDB_HAS_SME_TILES=y" >> $config_target_mak
+          if test "$gdb_version" = "15.0.50.20240403-git"; then
+            echo "GDB_HAS_INT_CAST_SUPPORT=n" >> $config_target_mak
+          else
+            echo "GDB_HAS_INT_CAST_SUPPORT=y" >> $config_target_mak
+          fi
+       else
+          echo "GDB_HAS_SME_TILES=n" >> $config_target_mak
+      fi
+
       if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge $gdb_version 15.1; then
           echo "GDB_HAS_MTE=y" >> $config_target_mak
       fi
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 16ddcf4f88..f9304d29cf 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -132,7 +132,38 @@ run-gdbstub-sve-ioctls: sve-ioctls
 		--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
 	basic gdbstub SVE ZLEN support)
 
-EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
+ifneq ($(CROSS_AS_HAS_ARMV9_SME),)
+# SME gdbstub test
+ifeq ($(GDB_HAS_SME_TILES),y)
+ifeq ($(GDB_HAS_INT_CAST_SUPPORT),y)
+run-gdbstub-sysregs-sme: sysregs
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
+		-- test_sme --gdb_sme_tile_support --gdb_int_cast_support, \
+	basic gdbstub SME support)
+else
+run-gdbstub-sysregs-sme: sysregs
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
+		-- test_sme --gdb_sme_tile_support, \
+	basic gdbstub SME support)
+endif
+else
+run-gdbstub-sysregs-sme: sysregs
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py, \
+	basic gdbstub SME support)
+
+endif
+endif
+
+EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls run-gdbstub-sysregs-sme
 
 ifeq ($(GDB_HAS_MTE),y)
 run-gdbstub-mte: mte-8
diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py b/tests/tcg/aarch64/gdbstub/test-sme.py
new file mode 100644
index 0000000000..e27a37631b
--- /dev/null
+++ b/tests/tcg/aarch64/gdbstub/test-sme.py
@@ -0,0 +1,165 @@
+#
+# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+from __future__ import print_function
+#
+# Test the SME registers are visible and changeable via gdbstub
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import argparse
+import gdb
+from test_gdbstub import main, report
+
+MAGIC = 0x01020304
+INT_CAST_SUPPORT = 0
+
+def run_test():
+    "Run through the tests one by one"
+
+    frame = gdb.selected_frame()
+    rname = "za"
+    za = frame.read_register(rname)
+    report(True, "Reading %s" % rname)
+
+    for i in range(0, 16):
+        for j in range(0, 16):
+            cmd = "set $za[%d][%d] = 0x01" % (i, j)
+            gdb.execute(cmd)
+            report(True, "%s" % cmd)
+    for i in range(0, 16):
+        for j in range(0, 16):
+            reg = "$za[%d][%d]" % (i, j)
+            v = gdb.parse_and_eval(reg)
+            report(str(v.type) == "uint8_t",
+                    "size of %s" % (reg))
+            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))
+
+def run_test_slices():
+    "Run through the tests one by one"
+
+    frame = gdb.selected_frame()
+    rname = "za"
+    za = frame.read_register(rname)
+    report(True, "Reading %s" % rname)
+
+    for i in range(0, 16):
+        for j in range(0, 16):
+            cmd = "set $za[%d][%d] = 0x01" % (i, j)
+            gdb.execute(cmd)
+            report(True, "%s" % cmd)
+    for i in range(0, 16):
+        for j in range(0, 16):
+            reg = "$za[%d][%d]" % (i, j)
+            v = gdb.parse_and_eval(reg)
+            report(str(v.type) == "uint8_t",
+                    "size of %s" % (reg))
+            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))
+
+    if INT_CAST_SUPPORT:
+        for i in range(0, 4):
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                    gdb.execute(cmd)
+                    report(True, "%s" % cmd)
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    reg = "$za%dhq%d[%d]" % (i, j, k)
+                    v = gdb.parse_and_eval(reg)
+                    report(str(v.type) == "uint128_t",
+                        "size of %s" % (reg))
+                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+            
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                    gdb.execute(cmd)
+                    report(True, "%s" % cmd)
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    reg = "$za%dvq%d[%d]" % (i, j, k)
+                    v = gdb.parse_and_eval(reg)
+                    report(str(v.type) == "uint128_t",
+                        "size of %s" % (reg))
+                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+
+    else:
+        for i in range(0, 4):
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                    gdb.execute(cmd)
+                    report(True, "%s" % cmd)
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    reg = "$za%dhq%d[%d]" % (i, j, k)
+                    v = gdb.parse_and_eval(reg)
+                    report(str(v.type) == "uint128_t",
+                        "size of %s" % (reg))
+                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+            
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                    gdb.execute(cmd)
+                    report(True, "%s" % cmd)
+            for j in range(0, 4):
+                for k in range(0, 4):
+                    reg = "$za%dvq%d[%d]" % (i, j, k)
+                    v = gdb.parse_and_eval(reg)
+                    report(str(v.type) == "uint128_t",
+                        "size of %s" % (reg))
+                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+
+    for i in range(0, 4):
+        for j in range(0, 4):
+            for k in range(0, 4):
+                cmd = "set $za%dhd%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                gdb.execute(cmd)
+                report(True, "%s" % cmd)
+        for j in range(0, 4):
+            for k in range(0, 4):
+                reg = "$za%dhd%d[%d]" % (i, j, k)
+                v = gdb.parse_and_eval(reg)
+                report(str(v.type) == "uint64_t",
+                    "size of %s" % (reg))
+                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+        
+        for j in range(0, 4):
+            for k in range(0, 4):
+                cmd = "set $za%dvd%d[%d] = 0x%x" % (i, j, k, MAGIC)
+                gdb.execute(cmd)
+                report(True, "%s" % cmd)
+        for j in range(0, 4):
+            for k in range(0, 4):
+                reg = "$za%dvd%d[%d]" % (i, j, k)
+                v = gdb.parse_and_eval(reg)
+                report(str(v.type) == "uint64_t",
+                    "size of %s" % (reg))
+                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
+
+
+parser = argparse.ArgumentParser(description="A gdbstub test for SME support")
+parser.add_argument("--gdb_sme_tile_support", help="GDB support for SME tiles", \
+                    action="store_true")
+parser.add_argument("--gdb_int_cast_support", 
+                    help="GDB support for 128bit int cast", \
+                    action="store_true")
+args = parser.parse_args()
+
+if args.gdb_sme_tile_support:
+    if args.gdb_int_cast_support:
+        INT_CAST_SUPPORT = 1
+    else:
+        print("WARNING: The version of gdb used (15.0.50.20240403-git)\n"
+        "does not support casting a gdb.Value object to 128 bit python\n"
+        "integer. Thus, the testing for the ZA quadwords will be done\n"
+        "without int casting. Refer to tests/tcg/aarch64/gdbstub/test-sme.py\n"
+        "for details.")
+    main(run_test_slices, expected_arch="aarch64")
+else:
+    main(run_test, expected_arch="aarch64")
-- 
2.34.1
Re: [PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB
Posted by Peter Maydell 3 months, 2 weeks ago
On Tue, 26 Aug 2025 at 19:45, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:
>
> This patch adds a test case to test SME register exposure to
> a remote gdb debugging session. This test simply sets and
> reads SME registers.
>
> Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
> ---
> Changes since v5:
> - added copyright and SPDX line
> - added functionality to avoid casting a gdb.Value object
> to int when testing the za quadwords to address bug found
> during review, this change is declared to users via a
> warning message included in the test results file
> run-gdbstub-sysregs-sme.out
> ---
>  configure                             |  11 ++
>  tests/tcg/aarch64/Makefile.target     |  33 +++++-
>  tests/tcg/aarch64/gdbstub/test-sme.py | 165 ++++++++++++++++++++++++++
>  3 files changed, 208 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py
>
> diff --git a/configure b/configure
> index 274a778764..9e2ae174dc 100755
> --- a/configure
> +++ b/configure
> @@ -1839,6 +1839,17 @@ for target in $target_list; do
>            echo "GDB=$gdb_bin" >> $config_target_mak
>        fi
>
> +      if test "${gdb_arches#*$arch}" != "$gdb_arches" && version_ge $gdb_version 14.1; then
> +          echo "GDB_HAS_SME_TILES=y" >> $config_target_mak
> +          if test "$gdb_version" = "15.0.50.20240403-git"; then
> +            echo "GDB_HAS_INT_CAST_SUPPORT=n" >> $config_target_mak
> +          else
> +            echo "GDB_HAS_INT_CAST_SUPPORT=y" >> $config_target_mak
> +          fi
> +       else
> +          echo "GDB_HAS_SME_TILES=n" >> $config_target_mak
> +      fi
> +
>        if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge $gdb_version 15.1; then
>            echo "GDB_HAS_MTE=y" >> $config_target_mak
>        fi
> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
> index 16ddcf4f88..f9304d29cf 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -132,7 +132,38 @@ run-gdbstub-sve-ioctls: sve-ioctls
>                 --bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
>         basic gdbstub SVE ZLEN support)
>
> -EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
> +ifneq ($(CROSS_AS_HAS_ARMV9_SME),)
> +# SME gdbstub test
> +ifeq ($(GDB_HAS_SME_TILES),y)
> +ifeq ($(GDB_HAS_INT_CAST_SUPPORT),y)
> +run-gdbstub-sysregs-sme: sysregs
> +       $(call run-test, $@, $(GDB_SCRIPT) \
> +               --gdb $(GDB) \
> +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
> +               -- test_sme --gdb_sme_tile_support --gdb_int_cast_support, \
> +       basic gdbstub SME support)
> +else
> +run-gdbstub-sysregs-sme: sysregs
> +       $(call run-test, $@, $(GDB_SCRIPT) \
> +               --gdb $(GDB) \
> +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
> +               -- test_sme --gdb_sme_tile_support, \
> +       basic gdbstub SME support)
> +endif
> +else
> +run-gdbstub-sysregs-sme: sysregs
> +       $(call run-test, $@, $(GDB_SCRIPT) \
> +               --gdb $(GDB) \
> +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py, \
> +       basic gdbstub SME support)
> +
> +endif
> +endif
> +
> +EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls run-gdbstub-sysregs-sme
>
>  ifeq ($(GDB_HAS_MTE),y)
>  run-gdbstub-mte: mte-8
> diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py b/tests/tcg/aarch64/gdbstub/test-sme.py
> new file mode 100644
> index 0000000000..e27a37631b
> --- /dev/null
> +++ b/tests/tcg/aarch64/gdbstub/test-sme.py
> @@ -0,0 +1,165 @@
> +#
> +# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +from __future__ import print_function
> +#
> +# Test the SME registers are visible and changeable via gdbstub
> +#
> +# This is launched via tests/guest-debug/run-test.py
> +#
> +
> +import argparse
> +import gdb
> +from test_gdbstub import main, report
> +
> +MAGIC = 0x01020304
> +INT_CAST_SUPPORT = 0
> +
> +def run_test():
> +    "Run through the tests one by one"

This is a very unhelpful name and comment for this function.

> +
> +    frame = gdb.selected_frame()
> +    rname = "za"
> +    za = frame.read_register(rname)
> +    report(True, "Reading %s" % rname)
> +
> +    for i in range(0, 16):
> +        for j in range(0, 16):
> +            cmd = "set $za[%d][%d] = 0x01" % (i, j)
> +            gdb.execute(cmd)
> +            report(True, "%s" % cmd)
> +    for i in range(0, 16):
> +        for j in range(0, 16):
> +            reg = "$za[%d][%d]" % (i, j)
> +            v = gdb.parse_and_eval(reg)
> +            report(str(v.type) == "uint8_t",
> +                    "size of %s" % (reg))
> +            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))
> +
> +def run_test_slices():
> +    "Run through the tests one by one"
> +
> +    frame = gdb.selected_frame()
> +    rname = "za"
> +    za = frame.read_register(rname)
> +    report(True, "Reading %s" % rname)
> +
> +    for i in range(0, 16):
> +        for j in range(0, 16):
> +            cmd = "set $za[%d][%d] = 0x01" % (i, j)
> +            gdb.execute(cmd)
> +            report(True, "%s" % cmd)
> +    for i in range(0, 16):
> +        for j in range(0, 16):
> +            reg = "$za[%d][%d]" % (i, j)
> +            v = gdb.parse_and_eval(reg)
> +            report(str(v.type) == "uint8_t",
> +                    "size of %s" % (reg))
> +            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))

This first part is exactly the same as the run_test()
function is testing, isn't it?

> +
> +    if INT_CAST_SUPPORT:
> +        for i in range(0, 4):
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                    gdb.execute(cmd)
> +                    report(True, "%s" % cmd)
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    reg = "$za%dhq%d[%d]" % (i, j, k)
> +                    v = gdb.parse_and_eval(reg)
> +                    report(str(v.type) == "uint128_t",
> +                        "size of %s" % (reg))
> +                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> +
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                    gdb.execute(cmd)
> +                    report(True, "%s" % cmd)
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    reg = "$za%dvq%d[%d]" % (i, j, k)
> +                    v = gdb.parse_and_eval(reg)
> +                    report(str(v.type) == "uint128_t",
> +                        "size of %s" % (reg))
> +                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> +
> +    else:
> +        for i in range(0, 4):
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                    gdb.execute(cmd)
> +                    report(True, "%s" % cmd)
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    reg = "$za%dhq%d[%d]" % (i, j, k)
> +                    v = gdb.parse_and_eval(reg)
> +                    report(str(v.type) == "uint128_t",
> +                        "size of %s" % (reg))
> +                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))

So the only difference between these two branches is that we are
checking "int(v) == MAGIC" rather than "v == MAGIC" ?

Is this a "one GDB only works one way, and the other GDB only
works the other way" case? Or is there a real interesting thing
we'd like to test involving the cast ?

Either way, this code is way too repetitive. Don't copy-and-paste
like this, it's very hard to review.

> +
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                    gdb.execute(cmd)
> +                    report(True, "%s" % cmd)
> +            for j in range(0, 4):
> +                for k in range(0, 4):
> +                    reg = "$za%dvq%d[%d]" % (i, j, k)
> +                    v = gdb.parse_and_eval(reg)
> +                    report(str(v.type) == "uint128_t",
> +                        "size of %s" % (reg))
> +                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> +
> +    for i in range(0, 4):
> +        for j in range(0, 4):
> +            for k in range(0, 4):
> +                cmd = "set $za%dhd%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                gdb.execute(cmd)
> +                report(True, "%s" % cmd)
> +        for j in range(0, 4):
> +            for k in range(0, 4):
> +                reg = "$za%dhd%d[%d]" % (i, j, k)
> +                v = gdb.parse_and_eval(reg)
> +                report(str(v.type) == "uint64_t",
> +                    "size of %s" % (reg))
> +                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> +
> +        for j in range(0, 4):
> +            for k in range(0, 4):
> +                cmd = "set $za%dvd%d[%d] = 0x%x" % (i, j, k, MAGIC)
> +                gdb.execute(cmd)
> +                report(True, "%s" % cmd)
> +        for j in range(0, 4):
> +            for k in range(0, 4):
> +                reg = "$za%dvd%d[%d]" % (i, j, k)
> +                v = gdb.parse_and_eval(reg)
> +                report(str(v.type) == "uint64_t",
> +                    "size of %s" % (reg))
> +                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> +
> +
> +parser = argparse.ArgumentParser(description="A gdbstub test for SME support")
> +parser.add_argument("--gdb_sme_tile_support", help="GDB support for SME tiles", \
> +                    action="store_true")
> +parser.add_argument("--gdb_int_cast_support",
> +                    help="GDB support for 128bit int cast", \
> +                    action="store_true")
> +args = parser.parse_args()
> +
> +if args.gdb_sme_tile_support:
> +    if args.gdb_int_cast_support:
> +        INT_CAST_SUPPORT = 1
> +    else:
> +        print("WARNING: The version of gdb used (15.0.50.20240403-git)\n"

This is super misleading: it looks like it's printing out what
gdb version we ran the tests with, but actually this version
string is just hardcoded !

> +        "does not support casting a gdb.Value object to 128 bit python\n"
> +        "integer. Thus, the testing for the ZA quadwords will be done\n"
> +        "without int casting. Refer to tests/tcg/aarch64/gdbstub/test-sme.py\n"
> +        "for details.")

This is a big warning that doesn't convey anything useful
to the reader (why do I care whether your test case
is using int casting or not?).

If there is part of the gdb view of the register that we
can't test on some gdb versions, that's fine. To handle
that, split the test case into two, so that for the
test that needs a newer gdb we report the test as SKIPPED
because gdb is too old.

Please also write some comments that tell me what your
test is testing. I should not have to reverse-engineer
your intentions by reading the code...

thanks
-- PMM
Re: [PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB
Posted by Vacha Bhavsar 3 months, 1 week ago
Hi,



Thank you for the feedback!



Regarding the following:



>So the only difference between these two branches is that we are
>checking "int(v) == MAGIC" rather than "v == MAGIC" ?



>Is this a "one GDB only works one way, and the other GDB only
>works the other way" case? Or is there a real interesting thing
>we'd like to test involving the cast ?



Yes, the only difference between the two branches is the presence
of the int cast. This seems to be an issue limited to specific versions

of gdb. This has been discussed with the gdb team
(https://sourceware.org/pipermail/gdb/2025-August/051868.html,
https://sourceware.org/pipermail/gdb/2025-August/051878.html) and
a bug has been filed. With additional tests I have found that the
int cast causes no issues with the testcase when running gdb16.1
or newer. Other than this issue there is no intention on our end
of testing anything interesting regarding casting as the int cast
was included to stay aligned with the existing SVE test.



I have sent a new patch version in which we have opted to go with
your suggestion of reporting the test as SKIPPED when the detected
gdb version does not allow for int casting of integers greater than
8 bytes (or does not have SME tile support).



To accomplish this, we have split the test into 3:



- one to test basic access to za (always run regardless of gdb version)
- one to test access to doubleword tile slices (run only with gdb14.1
or newer)
- one to test access to quadword tile slices (run only with gdb16.1 or
newer)



I have also restructured the test file to make the code more compact
and added additional comments.



Looking forward to your input on this approach.



Thanks,

Vacha

On Tue, Sep 2, 2025 at 6:29 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 26 Aug 2025 at 19:45, Vacha Bhavsar
> <vacha.bhavsar@oss.qualcomm.com> wrote:
> >
> > This patch adds a test case to test SME register exposure to
> > a remote gdb debugging session. This test simply sets and
> > reads SME registers.
> >
> > Signed-off-by: Vacha Bhavsar <vacha.bhavsar@oss.qualcomm.com>
> > ---
> > Changes since v5:
> > - added copyright and SPDX line
> > - added functionality to avoid casting a gdb.Value object
> > to int when testing the za quadwords to address bug found
> > during review, this change is declared to users via a
> > warning message included in the test results file
> > run-gdbstub-sysregs-sme.out
> > ---
> >  configure                             |  11 ++
> >  tests/tcg/aarch64/Makefile.target     |  33 +++++-
> >  tests/tcg/aarch64/gdbstub/test-sme.py | 165 ++++++++++++++++++++++++++
> >  3 files changed, 208 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/tcg/aarch64/gdbstub/test-sme.py
> >
> > diff --git a/configure b/configure
> > index 274a778764..9e2ae174dc 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1839,6 +1839,17 @@ for target in $target_list; do
> >            echo "GDB=$gdb_bin" >> $config_target_mak
> >        fi
> >
> > +      if test "${gdb_arches#*$arch}" != "$gdb_arches" && version_ge
> $gdb_version 14.1; then
> > +          echo "GDB_HAS_SME_TILES=y" >> $config_target_mak
> > +          if test "$gdb_version" = "15.0.50.20240403-git"; then
> > +            echo "GDB_HAS_INT_CAST_SUPPORT=n" >> $config_target_mak
> > +          else
> > +            echo "GDB_HAS_INT_CAST_SUPPORT=y" >> $config_target_mak
> > +          fi
> > +       else
> > +          echo "GDB_HAS_SME_TILES=n" >> $config_target_mak
> > +      fi
> > +
> >        if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge
> $gdb_version 15.1; then
> >            echo "GDB_HAS_MTE=y" >> $config_target_mak
> >        fi
> > diff --git a/tests/tcg/aarch64/Makefile.target
> b/tests/tcg/aarch64/Makefile.target
> > index 16ddcf4f88..f9304d29cf 100644
> > --- a/tests/tcg/aarch64/Makefile.target
> > +++ b/tests/tcg/aarch64/Makefile.target
> > @@ -132,7 +132,38 @@ run-gdbstub-sve-ioctls: sve-ioctls
> >                 --bin $< --test
> $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
> >         basic gdbstub SVE ZLEN support)
> >
> > -EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
> > +ifneq ($(CROSS_AS_HAS_ARMV9_SME),)
> > +# SME gdbstub test
> > +ifeq ($(GDB_HAS_SME_TILES),y)
> > +ifeq ($(GDB_HAS_INT_CAST_SUPPORT),y)
> > +run-gdbstub-sysregs-sme: sysregs
> > +       $(call run-test, $@, $(GDB_SCRIPT) \
> > +               --gdb $(GDB) \
> > +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> > +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
> > +               -- test_sme --gdb_sme_tile_support
> --gdb_int_cast_support, \
> > +       basic gdbstub SME support)
> > +else
> > +run-gdbstub-sysregs-sme: sysregs
> > +       $(call run-test, $@, $(GDB_SCRIPT) \
> > +               --gdb $(GDB) \
> > +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> > +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py \
> > +               -- test_sme --gdb_sme_tile_support, \
> > +       basic gdbstub SME support)
> > +endif
> > +else
> > +run-gdbstub-sysregs-sme: sysregs
> > +       $(call run-test, $@, $(GDB_SCRIPT) \
> > +               --gdb $(GDB) \
> > +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> > +               --bin $< --test $(AARCH64_SRC)/gdbstub/test-sme.py, \
> > +       basic gdbstub SME support)
> > +
> > +endif
> > +endif
> > +
> > +EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
> run-gdbstub-sysregs-sme
> >
> >  ifeq ($(GDB_HAS_MTE),y)
> >  run-gdbstub-mte: mte-8
> > diff --git a/tests/tcg/aarch64/gdbstub/test-sme.py
> b/tests/tcg/aarch64/gdbstub/test-sme.py
> > new file mode 100644
> > index 0000000000..e27a37631b
> > --- /dev/null
> > +++ b/tests/tcg/aarch64/gdbstub/test-sme.py
> > @@ -0,0 +1,165 @@
> > +#
> > +# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +from __future__ import print_function
> > +#
> > +# Test the SME registers are visible and changeable via gdbstub
> > +#
> > +# This is launched via tests/guest-debug/run-test.py
> > +#
> > +
> > +import argparse
> > +import gdb
> > +from test_gdbstub import main, report
> > +
> > +MAGIC = 0x01020304
> > +INT_CAST_SUPPORT = 0
> > +
> > +def run_test():
> > +    "Run through the tests one by one"
>
> This is a very unhelpful name and comment for this function.
>
> > +
> > +    frame = gdb.selected_frame()
> > +    rname = "za"
> > +    za = frame.read_register(rname)
> > +    report(True, "Reading %s" % rname)
> > +
> > +    for i in range(0, 16):
> > +        for j in range(0, 16):
> > +            cmd = "set $za[%d][%d] = 0x01" % (i, j)
> > +            gdb.execute(cmd)
> > +            report(True, "%s" % cmd)
> > +    for i in range(0, 16):
> > +        for j in range(0, 16):
> > +            reg = "$za[%d][%d]" % (i, j)
> > +            v = gdb.parse_and_eval(reg)
> > +            report(str(v.type) == "uint8_t",
> > +                    "size of %s" % (reg))
> > +            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))
> > +
> > +def run_test_slices():
> > +    "Run through the tests one by one"
> > +
> > +    frame = gdb.selected_frame()
> > +    rname = "za"
> > +    za = frame.read_register(rname)
> > +    report(True, "Reading %s" % rname)
> > +
> > +    for i in range(0, 16):
> > +        for j in range(0, 16):
> > +            cmd = "set $za[%d][%d] = 0x01" % (i, j)
> > +            gdb.execute(cmd)
> > +            report(True, "%s" % cmd)
> > +    for i in range(0, 16):
> > +        for j in range(0, 16):
> > +            reg = "$za[%d][%d]" % (i, j)
> > +            v = gdb.parse_and_eval(reg)
> > +            report(str(v.type) == "uint8_t",
> > +                    "size of %s" % (reg))
> > +            report(int(v) == 0x1, "%s is 0x%x" % (reg, 0x1))
>
> This first part is exactly the same as the run_test()
> function is testing, isn't it?
>
> > +
> > +    if INT_CAST_SUPPORT:
> > +        for i in range(0, 4):
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                    gdb.execute(cmd)
> > +                    report(True, "%s" % cmd)
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    reg = "$za%dhq%d[%d]" % (i, j, k)
> > +                    v = gdb.parse_and_eval(reg)
> > +                    report(str(v.type) == "uint128_t",
> > +                        "size of %s" % (reg))
> > +                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> > +
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                    gdb.execute(cmd)
> > +                    report(True, "%s" % cmd)
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    reg = "$za%dvq%d[%d]" % (i, j, k)
> > +                    v = gdb.parse_and_eval(reg)
> > +                    report(str(v.type) == "uint128_t",
> > +                        "size of %s" % (reg))
> > +                    report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> > +
> > +    else:
> > +        for i in range(0, 4):
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    cmd = "set $za%dhq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                    gdb.execute(cmd)
> > +                    report(True, "%s" % cmd)
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    reg = "$za%dhq%d[%d]" % (i, j, k)
> > +                    v = gdb.parse_and_eval(reg)
> > +                    report(str(v.type) == "uint128_t",
> > +                        "size of %s" % (reg))
> > +                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))
>
> So the only difference between these two branches is that we are
> checking "int(v) == MAGIC" rather than "v == MAGIC" ?
>
> Is this a "one GDB only works one way, and the other GDB only
> works the other way" case? Or is there a real interesting thing
> we'd like to test involving the cast ?
>
> Either way, this code is way too repetitive. Don't copy-and-paste
> like this, it's very hard to review.
>
> > +
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    cmd = "set $za%dvq%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                    gdb.execute(cmd)
> > +                    report(True, "%s" % cmd)
> > +            for j in range(0, 4):
> > +                for k in range(0, 4):
> > +                    reg = "$za%dvq%d[%d]" % (i, j, k)
> > +                    v = gdb.parse_and_eval(reg)
> > +                    report(str(v.type) == "uint128_t",
> > +                        "size of %s" % (reg))
> > +                    report(v == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> > +
> > +    for i in range(0, 4):
> > +        for j in range(0, 4):
> > +            for k in range(0, 4):
> > +                cmd = "set $za%dhd%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                gdb.execute(cmd)
> > +                report(True, "%s" % cmd)
> > +        for j in range(0, 4):
> > +            for k in range(0, 4):
> > +                reg = "$za%dhd%d[%d]" % (i, j, k)
> > +                v = gdb.parse_and_eval(reg)
> > +                report(str(v.type) == "uint64_t",
> > +                    "size of %s" % (reg))
> > +                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> > +
> > +        for j in range(0, 4):
> > +            for k in range(0, 4):
> > +                cmd = "set $za%dvd%d[%d] = 0x%x" % (i, j, k, MAGIC)
> > +                gdb.execute(cmd)
> > +                report(True, "%s" % cmd)
> > +        for j in range(0, 4):
> > +            for k in range(0, 4):
> > +                reg = "$za%dvd%d[%d]" % (i, j, k)
> > +                v = gdb.parse_and_eval(reg)
> > +                report(str(v.type) == "uint64_t",
> > +                    "size of %s" % (reg))
> > +                report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
> > +
> > +
> > +parser = argparse.ArgumentParser(description="A gdbstub test for SME
> support")
> > +parser.add_argument("--gdb_sme_tile_support", help="GDB support for SME
> tiles", \
> > +                    action="store_true")
> > +parser.add_argument("--gdb_int_cast_support",
> > +                    help="GDB support for 128bit int cast", \
> > +                    action="store_true")
> > +args = parser.parse_args()
> > +
> > +if args.gdb_sme_tile_support:
> > +    if args.gdb_int_cast_support:
> > +        INT_CAST_SUPPORT = 1
> > +    else:
> > +        print("WARNING: The version of gdb used
> (15.0.50.20240403-git)\n"
>
> This is super misleading: it looks like it's printing out what
> gdb version we ran the tests with, but actually this version
> string is just hardcoded !
>
> > +        "does not support casting a gdb.Value object to 128 bit
> python\n"
> > +        "integer. Thus, the testing for the ZA quadwords will be done\n"
> > +        "without int casting. Refer to
> tests/tcg/aarch64/gdbstub/test-sme.py\n"
> > +        "for details.")
>
> This is a big warning that doesn't convey anything useful
> to the reader (why do I care whether your test case
> is using int casting or not?).
>
> If there is part of the gdb view of the register that we
> can't test on some gdb versions, that's fine. To handle
> that, split the test case into two, so that for the
> test that needs a newer gdb we report the test as SKIPPED
> because gdb is too old.
>
> Please also write some comments that tell me what your
> test is testing. I should not have to reverse-engineer
> your intentions by reading the code...
>
> thanks
> -- PMM
>
Re: [PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB
Posted by Peter Maydell 3 months, 1 week ago
On Mon, 8 Sept 2025 at 19:32, Vacha Bhavsar
<vacha.bhavsar@oss.qualcomm.com> wrote:

> >So the only difference between these two branches is that we are
> >checking "int(v) == MAGIC" rather than "v == MAGIC" ?
>
>
>
> >Is this a "one GDB only works one way, and the other GDB only
> >works the other way" case? Or is there a real interesting thing
> >we'd like to test involving the cast ?

> Yes, the only difference between the two branches is the presence
> of the int cast. This seems to be an issue limited to specific versions
>
> of gdb. This has been discussed with the gdb team
> (https://sourceware.org/pipermail/gdb/2025-August/051868.html,
> https://sourceware.org/pipermail/gdb/2025-August/051878.html) and
> a bug has been filed. With additional tests I have found that the
> int cast causes no issues with the testcase when running gdb16.1
> or newer. Other than this issue there is no intention on our end
> of testing anything interesting regarding casting as the int cast
> was included to stay aligned with the existing SVE test.

So if we can test QEMU's gdbstub functionality by writing
the test in a way that avoids the int cast and which doesn't
lose coverage, we should just do that. That would mean
we don't need to have checks for whether the gdb can do
the cast and multiple paths through the test code.

-- PMM
Re: [PATCH v6 3/3] target/arm: Added test case for SME register exposure to GDB
Posted by Vacha Bhavsar 3 months, 1 week ago
Hi,

Got it! I have sent over an updated version that avoids the
int cast and associated checks for an appropriate gdb version.

Thanks,
Vacha

On Tue, Sep 9, 2025 at 4:55 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 8 Sept 2025 at 19:32, Vacha Bhavsar
> <vacha.bhavsar@oss.qualcomm.com> wrote:
>
> > >So the only difference between these two branches is that we are
> > >checking "int(v) == MAGIC" rather than "v == MAGIC" ?
> >
> >
> >
> > >Is this a "one GDB only works one way, and the other GDB only
> > >works the other way" case? Or is there a real interesting thing
> > >we'd like to test involving the cast ?
>
> > Yes, the only difference between the two branches is the presence
> > of the int cast. This seems to be an issue limited to specific versions
> >
> > of gdb. This has been discussed with the gdb team
> > (https://sourceware.org/pipermail/gdb/2025-August/051868.html,
> > https://sourceware.org/pipermail/gdb/2025-August/051878.html) and
> > a bug has been filed. With additional tests I have found that the
> > int cast causes no issues with the testcase when running gdb16.1
> > or newer. Other than this issue there is no intention on our end
> > of testing anything interesting regarding casting as the int cast
> > was included to stay aligned with the existing SVE test.
>
> So if we can test QEMU's gdbstub functionality by writing
> the test in a way that avoids the int cast and which doesn't
> lose coverage, we should just do that. That would mean
> we don't need to have checks for whether the gdb can do
> the cast and multiple paths through the test code.
>
> -- PMM
>