All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH  v1 00/20] gdbstub, semihosting and test/tool updates (pre PR)
@ 2021-01-08 22:42 Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 01/20] tests/docker: Remove Debian 9 remnant lines Alex Bennée
                   ` (19 more replies)
  0 siblings, 20 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

Hi,

This is gathering together my maintainer trees into one place in
advance of putting together a pull request next week. There are:

  - gdbstub: more tests and tweaks to SVE handling for ARM
  - semihosting: common code and enabling for RiscV
  - some minor test and devtool tweaks

Last chance to object to any of the changes ;-)

Alex Bennée (9):
  test/guest-debug: echo QEMU command as well
  configure: gate our use of GDB to 8.3.1 or above
  Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1
    test"
  gdbstub: implement a softmmu based test
  gdbstub: drop CPUEnv from gdb_exit()
  gdbstub: drop gdbserver_cleanup in favour of gdb_exit
  gdbstub: ensure we clean-up when terminated
  target/arm: use official org.gnu.gdb.aarch64.sve layout for registers
  Makefile: add GNU global tags support

Keith Packard (8):
  semihosting: Move ARM semihosting code to shared directories
  semihosting: Change common-semi API to be architecture-independent
  semihosting: Change internal common-semi interfaces to use CPUState *
  semihosting: Support SYS_HEAPINFO when env->boot_info is not set
  riscv: Add semihosting support
  semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ
  semihosting: Implement SYS_TMPNAM
  semihosting: Implement SYS_ISERROR

Kito Cheng (1):
  riscv: Add semihosting support for user mode

Lirong Yuan (1):
  gdbstub: add support to Xfer:auxv:read: packet

Philippe Mathieu-Daudé (1):
  tests/docker: Remove Debian 9 remnant lines

 configure                                     |   7 +-
 Makefile                                      |   9 +-
 default-configs/devices/arm-softmmu.mak       |   1 +
 default-configs/devices/riscv32-softmmu.mak   |   2 +
 default-configs/devices/riscv64-softmmu.mak   |   2 +
 .../targets/aarch64-linux-user.mak            |   1 +
 .../targets/aarch64_be-linux-user.mak         |   1 +
 default-configs/targets/arm-linux-user.mak    |   1 +
 default-configs/targets/armeb-linux-user.mak  |   1 +
 .../targets/riscv32-linux-user.mak            |   1 +
 .../targets/riscv64-linux-user.mak            |   1 +
 hw/semihosting/common-semi.h                  |  39 ++
 include/exec/gdbstub.h                        |  14 +-
 include/qemu/timer.h                          |   2 +
 linux-user/qemu.h                             |   4 +-
 target/arm/cpu.h                              |   8 -
 target/riscv/cpu_bits.h                       |   1 +
 bsd-user/syscall.c                            |   6 +-
 gdbstub.c                                     |  65 ++-
 .../semihosting/common-semi.c                 | 525 ++++++++++++------
 linux-user/aarch64/cpu_loop.c                 |   3 +-
 linux-user/arm/cpu_loop.c                     |   3 +-
 linux-user/exit.c                             |   2 +-
 linux-user/riscv/cpu_loop.c                   |   5 +
 linux-user/{arm => }/semihost.c               |   8 +-
 softmmu/runstate.c                            |   2 +-
 target/arm/gdbstub.c                          |  75 +--
 target/arm/helper.c                           |   7 +-
 target/arm/m_helper.c                         |   7 +-
 target/m68k/m68k-semi.c                       |   2 +-
 target/nios2/nios2-semi.c                     |   2 +-
 target/riscv/cpu_helper.c                     |  10 +
 target/riscv/translate.c                      |  11 +
 util/qemu-timer-common.c                      |   4 +
 .../riscv/insn_trans/trans_privileged.c.inc   |  37 +-
 .gitignore                                    |   3 +
 MAINTAINERS                                   |   1 +
 hw/semihosting/Kconfig                        |   3 +
 hw/semihosting/meson.build                    |   3 +
 linux-user/arm/meson.build                    |   3 -
 linux-user/meson.build                        |   1 +
 qemu-options.hx                               |  10 +-
 target/arm/meson.build                        |   2 -
 tests/docker/Makefile.include                 |   1 -
 tests/guest-debug/run-test.py                 |  35 +-
 tests/tcg/aarch64/Makefile.softmmu-target     |   1 +
 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py   |  11 +
 tests/tcg/aarch64/system/boot.S               |   1 +
 tests/tcg/i386/Makefile.softmmu-target        |   1 +
 tests/tcg/i386/system/boot.S                  |   2 +-
 tests/tcg/multiarch/Makefile.target           |  13 +-
 tests/tcg/multiarch/gdbstub/memory.py         | 130 +++++
 .../multiarch/gdbstub/test-qxfer-auxv-read.py |  57 ++
 .../multiarch/system/Makefile.softmmu-target  |  19 +-
 tests/tcg/x86_64/Makefile.softmmu-target      |   1 +
 tests/tcg/x86_64/system/boot.S                |   2 +-
 56 files changed, 888 insertions(+), 281 deletions(-)
 create mode 100644 hw/semihosting/common-semi.h
 rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (66%)
 rename linux-user/{arm => }/semihost.c (89%)
 create mode 100644 tests/tcg/multiarch/gdbstub/memory.py
 create mode 100644 tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py

-- 
2.20.1



^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH  v1 01/20] tests/docker: Remove Debian 9 remnant lines
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 02/20] test/guest-debug: echo QEMU command as well Alex Bennée
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Philippe Mathieu-Daudé,
	Thomas Huth, Alex Bennée, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Debian 9 base container has been removed in commits
e3755276d1f and c9d78b06c06. Remove the last remnants.

Fixes: e3755276d1f ("tests/docker: Remove old Debian 9 containers")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210107072933.3828450-1-f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/docker/Makefile.include | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index c254ac38d0..0779dab5b9 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -108,7 +108,6 @@ ifneq ($(HOST_ARCH),x86_64)
 DOCKER_PARTIAL_IMAGES += debian-mips-cross debian-mipsel-cross debian-mips64el-cross
 DOCKER_PARTIAL_IMAGES += debian-ppc64el-cross
 DOCKER_PARTIAL_IMAGES += debian-s390x-cross
-DOCKER_PARTIAL_IMAGES += debian-win32-cross debian-win64-cross
 DOCKER_PARTIAL_IMAGES += fedora travis
 endif
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 02/20] test/guest-debug: echo QEMU command as well
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 01/20] tests/docker: Remove Debian 9 remnant lines Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-11 15:27   ` Philippe Mathieu-Daudé
  2021-01-08 22:42 ` [PATCH v1 03/20] configure: gate our use of GDB to 8.3.1 or above Alex Bennée
                   ` (17 subsequent siblings)
  19 siblings, 1 reply; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

This helps with debugging.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201214153012.12723-2-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-2-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/guest-debug/run-test.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 71c5569054..0c4f5c3808 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -53,6 +53,7 @@ if __name__ == '__main__':
         cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name,
                                   args.binary)
 
+    print("QEMU CMD: %s" % (cmd))
     inferior = subprocess.Popen(shlex.split(cmd))
 
     # Now launch gdb with our test and collect the result
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 03/20] configure: gate our use of GDB to 8.3.1 or above
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 01/20] tests/docker: Remove Debian 9 remnant lines Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 02/20] test/guest-debug: echo QEMU command as well Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 04/20] Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1 test" Alex Bennée
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

The support of socket based debugging which we need for linux-user
testing is only really stable as of 8.3.1 so lets gate our use of GDB
on having a relatively modern version.

For direct testing you can just point to a locally compiled version of
gdb via configure, e.g.:

  ../../configure --gdb=$HOME/src/binutils-gdb.git/builds/all/install/bin/gdb

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201214153012.12723-3-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-3-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 5860bdb77b..96cc7d9b9f 100755
--- a/configure
+++ b/configure
@@ -6239,8 +6239,11 @@ if test "$plugins" = "yes" ; then
     fi
 fi
 
-if test -n "$gdb_bin" ; then
-    echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
+if test -n "$gdb_bin"; then
+    gdb_version=$($gdb_bin --version | head -n 1)
+    if version_ge ${gdb_version##* } 8.3.1; then
+        echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
+    fi
 fi
 
 if test "$secret_keyring" = "yes" ; then
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 04/20] Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1 test"
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (2 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 03/20] configure: gate our use of GDB to 8.3.1 or above Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 05/20] gdbstub: implement a softmmu based test Alex Bennée
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

We won't attempt to run the test now it's gated on a newer version of
gdb.

This reverts commit a930cadd83b4681a98ce72abf530a791ee2e42a6.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-4-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/multiarch/Makefile.target | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 230eb9a95e..cb49cc9ccb 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -54,9 +54,7 @@ run-gdbstub-sha1: sha1
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/sha1.py, \
 	"basic gdbstub support")
 
-# Disable this for now -- it provokes a gdb internal-error on
-# Ubuntu gdb 8.1.1-0ubuntu1.
-# EXTRA_RUNS += run-gdbstub-sha1
+EXTRA_RUNS += run-gdbstub-sha1
 endif
 
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 05/20] gdbstub: implement a softmmu based test
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (3 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 04/20] Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1 test" Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 06/20] gdbstub: add support to Xfer:auxv:read: packet Alex Bennée
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Philippe Mathieu-Daudé,
	Richard Henderson, Philippe Mathieu-Daudé,
	open list:ARM TCG CPUs, Paolo Bonzini, Alex Bennée

This adds a new tests that allows us to test softmmu only features
including watchpoints. To do achieve this we need to:

  - add _exit: labels to the boot codes
  - write a memory.py test case
  - plumb the test case into the build system
  - tweak the run_test script to:
    - re-direct output when asked
    - use socket based connection for all tests
    - add a small pause before connection

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201218112707.28348-5-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/guest-debug/run-test.py                 |  36 +++--
 tests/tcg/aarch64/Makefile.softmmu-target     |   1 +
 tests/tcg/aarch64/system/boot.S               |   1 +
 tests/tcg/i386/Makefile.softmmu-target        |   1 +
 tests/tcg/i386/system/boot.S                  |   2 +-
 tests/tcg/multiarch/gdbstub/memory.py         | 130 ++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  |  19 ++-
 tests/tcg/x86_64/Makefile.softmmu-target      |   1 +
 tests/tcg/x86_64/system/boot.S                |   2 +-
 9 files changed, 181 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/memory.py

diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 0c4f5c3808..8b91ff95af 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -16,6 +16,7 @@ import subprocess
 import shutil
 import shlex
 import os
+from time import sleep
 from tempfile import TemporaryDirectory
 
 def get_args():
@@ -27,10 +28,21 @@ def get_args():
                         required=True)
     parser.add_argument("--test", help="GDB test script",
                         required=True)
-    parser.add_argument("--gdb", help="The gdb binary to use", default=None)
+    parser.add_argument("--gdb", help="The gdb binary to use",
+                        default=None)
+    parser.add_argument("--output", help="A file to redirect output to")
 
     return parser.parse_args()
 
+
+def log(output, msg):
+    if output:
+        output.write(msg + "\n")
+        output.flush()
+    else:
+        print(msg)
+
+
 if __name__ == '__main__':
     args = get_args()
 
@@ -42,18 +54,25 @@ if __name__ == '__main__':
     if not args.gdb:
         print("We need gdb to run the test")
         exit(-1)
+    if args.output:
+        output = open(args.output, "w")
+    else:
+        output = None
 
     socket_dir = TemporaryDirectory("qemu-gdbstub")
     socket_name = os.path.join(socket_dir.name, "gdbstub.socket")
 
     # Launch QEMU with binary
     if "system" in args.qemu:
-        cmd = "%s %s %s -s -S" % (args.qemu, args.qargs, args.binary)
+        cmd = "%s %s %s -gdb unix:path=%s,server" % (args.qemu,
+                                                     args.qargs,
+                                                     args.binary,
+                                                     socket_name)
     else:
         cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name,
                                   args.binary)
 
-    print("QEMU CMD: %s" % (cmd))
+    log(output, "QEMU CMD: %s" % (cmd))
     inferior = subprocess.Popen(shlex.split(cmd))
 
     # Now launch gdb with our test and collect the result
@@ -63,16 +82,15 @@ if __name__ == '__main__':
     # disable prompts in case of crash
     gdb_cmd += " -ex 'set confirm off'"
     # connect to remote
-    if "system" in args.qemu:
-        gdb_cmd += " -ex 'target remote localhost:1234'"
-    else:
-        gdb_cmd += " -ex 'target remote %s'" % (socket_name)
+    gdb_cmd += " -ex 'target remote %s'" % (socket_name)
     # finally the test script itself
     gdb_cmd += " -x %s" % (args.test)
 
-    print("GDB CMD: %s" % (gdb_cmd))
 
-    result = subprocess.call(gdb_cmd, shell=True);
+    sleep(1)
+    log(output, "GDB CMD: %s" % (gdb_cmd))
+
+    result = subprocess.call(gdb_cmd, shell=True, stdout=output)
 
     # A negative result is the result of an internal gdb failure like
     # a crash. We force a return of 0 so we don't fail the test on
diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target
index 1057a8ac49..a7286ac295 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -15,6 +15,7 @@ CRT_PATH=$(AARCH64_SYSTEM_SRC)
 LINK_SCRIPT=$(AARCH64_SYSTEM_SRC)/kernel.ld
 LDFLAGS=-Wl,-T$(LINK_SCRIPT)
 TESTS+=$(AARCH64_TESTS) $(MULTIARCH_TESTS)
+EXTRA_RUNS+=$(MULTIARCH_RUNS)
 CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
diff --git a/tests/tcg/aarch64/system/boot.S b/tests/tcg/aarch64/system/boot.S
index b14e94f332..e190b1efa6 100644
--- a/tests/tcg/aarch64/system/boot.S
+++ b/tests/tcg/aarch64/system/boot.S
@@ -197,6 +197,7 @@ __start:
 	bl	main
 
 	/* pass return value to sys exit */
+_exit:
 	mov    x1, x0
 	ldr    x0, =0x20026 /* ADP_Stopped_ApplicationExit */
 	stp    x0, x1, [sp, #-16]!
diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target
index 1c8790eecd..5266f2335a 100644
--- a/tests/tcg/i386/Makefile.softmmu-target
+++ b/tests/tcg/i386/Makefile.softmmu-target
@@ -19,6 +19,7 @@ CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
 TESTS+=$(MULTIARCH_TESTS)
+EXTRA_RUNS+=$(MULTIARCH_RUNS)
 
 # building head blobs
 .PRECIOUS: $(CRT_OBJS)
diff --git a/tests/tcg/i386/system/boot.S b/tests/tcg/i386/system/boot.S
index 90aa174908..794c2cb0ad 100644
--- a/tests/tcg/i386/system/boot.S
+++ b/tests/tcg/i386/system/boot.S
@@ -76,7 +76,7 @@ _start:
          */
         call main
 
-        /* output any non-zero result in eax to isa-debug-exit device */
+_exit:	/* output any non-zero result in eax to isa-debug-exit device */
         test %al, %al
         jz 1f
         out %ax, $0xf4
diff --git a/tests/tcg/multiarch/gdbstub/memory.py b/tests/tcg/multiarch/gdbstub/memory.py
new file mode 100644
index 0000000000..67864ad902
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/memory.py
@@ -0,0 +1,130 @@
+from __future__ import print_function
+#
+# Test some of the softmmu debug features with the multiarch memory
+# test. It is a port of the original vmlinux focused test case but
+# using the "memory" test instead.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+failcount = 0
+
+
+def report(cond, msg):
+    "Report success/fail of test"
+    if cond:
+        print("PASS: %s" % (msg))
+    else:
+        print("FAIL: %s" % (msg))
+        global failcount
+        failcount += 1
+
+
+def check_step():
+    "Step an instruction, check it moved."
+    start_pc = gdb.parse_and_eval('$pc')
+    gdb.execute("si")
+    end_pc = gdb.parse_and_eval('$pc')
+
+    return not (start_pc == end_pc)
+
+
+#
+# Currently it's hard to create a hbreak with the pure python API and
+# manually matching PC to symbol address is a bit flaky thanks to
+# function prologues. However internally QEMU's gdbstub treats them
+# the same as normal breakpoints so it will do for now.
+#
+def check_break(sym_name):
+    "Setup breakpoint, continue and check we stopped."
+    sym, ok = gdb.lookup_symbol(sym_name)
+    bp = gdb.Breakpoint(sym_name, gdb.BP_BREAKPOINT)
+
+    gdb.execute("c")
+
+    # hopefully we came back
+    end_pc = gdb.parse_and_eval('$pc')
+    report(bp.hit_count == 1,
+           "break @ %s (%s %d hits)" % (end_pc, sym.value(), bp.hit_count))
+
+    bp.delete()
+
+
+def do_one_watch(sym, wtype, text):
+
+    wp = gdb.Breakpoint(sym, gdb.BP_WATCHPOINT, wtype)
+    gdb.execute("c")
+    report_str = "%s for %s" % (text, sym)
+
+    if wp.hit_count > 0:
+        report(True, report_str)
+        wp.delete()
+    else:
+        report(False, report_str)
+
+
+def check_watches(sym_name):
+    "Watch a symbol for any access."
+
+    # Should hit for any read
+    do_one_watch(sym_name, gdb.WP_ACCESS, "awatch")
+
+    # Again should hit for reads
+    do_one_watch(sym_name, gdb.WP_READ, "rwatch")
+
+    # Finally when it is written
+    do_one_watch(sym_name, gdb.WP_WRITE, "watch")
+
+
+def run_test():
+    "Run through the tests one by one"
+
+    print("Checking we can step the first few instructions")
+    step_ok = 0
+    for i in range(3):
+        if check_step():
+            step_ok += 1
+
+    report(step_ok == 3, "single step in boot code")
+
+    # If we get here we have missed some of the other breakpoints.
+    print("Setup catch-all for _exit")
+    cbp = gdb.Breakpoint("_exit", gdb.BP_BREAKPOINT)
+
+    check_break("main")
+    check_watches("test_data[128]")
+
+    report(cbp.hit_count == 0, "didn't reach backstop")
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+    inferior = gdb.selected_inferior()
+    arch = inferior.architecture()
+    print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+    print("SKIPPING (not connected)", file=sys.stderr)
+    exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+    print("SKIP: PC not set")
+    exit(0)
+
+try:
+    # These are not very useful in scripts
+    gdb.execute("set pagination off")
+
+    # Run the actual tests
+    run_test()
+except (gdb.error):
+    print("GDB Exception: %s" % (sys.exc_info()[0]))
+    failcount += 1
+    pass
+
+# Finally kill the inferior and exit gdb with a count of failures
+gdb.execute("kill")
+exit(failcount)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index db4bbeda44..4657f6e4cf 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -7,8 +7,25 @@
 # complications of building.
 #
 
-MULTIARCH_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/multiarch/system
+MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
+MULTIARCH_SYSTEM_SRC=$(MULTIARCH_SRC)/system
 VPATH+=$(MULTIARCH_SYSTEM_SRC)
 
 MULTIARCH_TEST_SRCS=$(wildcard $(MULTIARCH_SYSTEM_SRC)/*.c)
 MULTIARCH_TESTS = $(patsubst $(MULTIARCH_SYSTEM_SRC)/%.c, %, $(MULTIARCH_TEST_SRCS))
+
+ifneq ($(HAVE_GDB_BIN),)
+GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
+
+run-gdbstub-memory: memory
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(HAVE_GDB_BIN) \
+		--qemu $(QEMU) \
+		--output $<.gdb.out \
+		--qargs \
+		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \
+	"softmmu gdbstub support")
+
+MULTIARCH_RUNS += run-gdbstub-memory
+endif
diff --git a/tests/tcg/x86_64/Makefile.softmmu-target b/tests/tcg/x86_64/Makefile.softmmu-target
index df252e761c..1bd763f2e6 100644
--- a/tests/tcg/x86_64/Makefile.softmmu-target
+++ b/tests/tcg/x86_64/Makefile.softmmu-target
@@ -19,6 +19,7 @@ CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
 TESTS+=$(MULTIARCH_TESTS)
+EXTRA_RUNS+=$(MULTIARCH_RUNS)
 
 # building head blobs
 .PRECIOUS: $(CRT_OBJS)
diff --git a/tests/tcg/x86_64/system/boot.S b/tests/tcg/x86_64/system/boot.S
index 73b19a2bda..f8a2fcc839 100644
--- a/tests/tcg/x86_64/system/boot.S
+++ b/tests/tcg/x86_64/system/boot.S
@@ -124,7 +124,7 @@ _start:
         /* don't worry about stack frame, assume everthing is garbage when we return */
 	call main
 
-        /* output any non-zero result in eax to isa-debug-exit device */
+_exit:	/* output any non-zero result in eax to isa-debug-exit device */
         test %al, %al
         jz 1f
         out %ax, $0xf4
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 06/20] gdbstub: add support to Xfer:auxv:read: packet
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (4 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 05/20] gdbstub: implement a softmmu based test Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 07/20] gdbstub: drop CPUEnv from gdb_exit() Alex Bennée
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Lirong Yuan,
	Philippe Mathieu-Daudé

From: Lirong Yuan <yuanzi@google.com>

This allows gdb to access the target’s auxiliary vector,
which can be helpful for telling system libraries important details
about the hardware, operating system, and process.

[AJB: minor tweaks to test case, update MAINTAINERS]

Signed-off-by: Lirong Yuan <yuanzi@google.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200730193932.3654677-1-yuanzi@google.com>
Message-Id: <20201214153012.12723-4-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-6-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c                                     | 54 ++++++++++++++++++
 MAINTAINERS                                   |  1 +
 tests/tcg/multiarch/Makefile.target           |  9 +++
 .../multiarch/gdbstub/test-qxfer-auxv-read.py | 57 +++++++++++++++++++
 4 files changed, 121 insertions(+)
 create mode 100644 tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py

diff --git a/gdbstub.c b/gdbstub.c
index d99bc0bf2e..15d3a8e1f5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2172,6 +2172,12 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
             ";ReverseStep+;ReverseContinue+");
     }
 
+#ifdef CONFIG_USER_ONLY
+    if (gdbserver_state.c_cpu->opaque) {
+        g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
+    }
+#endif
+
     if (gdb_ctx->num_params &&
         strstr(gdb_ctx->params[0].data, "multiprocess+")) {
         gdbserver_state.multiprocess = true;
@@ -2233,6 +2239,46 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
                       gdbserver_state.str_buf->len, true);
 }
 
+#ifdef CONFIG_USER_ONLY
+static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    TaskState *ts;
+    unsigned long offset, len, saved_auxv, auxv_len;
+    const char *mem;
+
+    if (gdb_ctx->num_params < 2) {
+        put_packet("E22");
+        return;
+    }
+
+    offset = gdb_ctx->params[0].val_ul;
+    len = gdb_ctx->params[1].val_ul;
+    ts = gdbserver_state.c_cpu->opaque;
+    saved_auxv = ts->info->saved_auxv;
+    auxv_len = ts->info->auxv_len;
+    mem = (const char *)(saved_auxv + offset);
+    if (offset > auxv_len) {
+        put_packet("E00");
+        return;
+    }
+
+    if (len > (MAX_PACKET_LENGTH - 5) / 2) {
+        len = (MAX_PACKET_LENGTH - 5) / 2;
+    }
+
+    if (len < auxv_len - offset) {
+        g_string_assign(gdbserver_state.str_buf, "m");
+        memtox(gdbserver_state.str_buf, mem, len);
+    } else {
+        g_string_assign(gdbserver_state.str_buf, "l");
+        memtox(gdbserver_state.str_buf, mem, auxv_len - offset);
+    }
+
+    put_packet_binary(gdbserver_state.str_buf->str,
+                      gdbserver_state.str_buf->len, true);
+}
+#endif
+
 static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     put_packet(GDB_ATTACHED);
@@ -2338,6 +2384,14 @@ static GdbCmdParseEntry gdb_gen_query_table[] = {
         .cmd_startswith = 1,
         .schema = "s:l,l0"
     },
+#ifdef CONFIG_USER_ONLY
+    {
+        .handler = handle_query_xfer_auxv,
+        .cmd = "Xfer:auxv:read::",
+        .cmd_startswith = 1,
+        .schema = "l,l0"
+    },
+#endif
     {
         .handler = handle_query_attached,
         .cmd = "Attached:",
diff --git a/MAINTAINERS b/MAINTAINERS
index 4be087b88e..990554cda1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2322,6 +2322,7 @@ R: Philippe Mathieu-Daudé <philmd@redhat.com>
 S: Maintained
 F: gdbstub*
 F: gdb-xml/
+F: tests/tcg/multiarch/gdbstub/
 
 Memory API
 M: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index cb49cc9ccb..1dd0f64d23 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -55,6 +55,15 @@ run-gdbstub-sha1: sha1
 	"basic gdbstub support")
 
 EXTRA_RUNS += run-gdbstub-sha1
+
+run-gdbstub-qxfer-auxv-read: sha1
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(HAVE_GDB_BIN) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \
+	"basic gdbstub qXfer:auxv:read support")
+
+EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read
 endif
 
 
diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
new file mode 100644
index 0000000000..d91e8fdf19
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
@@ -0,0 +1,57 @@
+from __future__ import print_function
+#
+# Test auxiliary vector is loaded via gdbstub
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+failcount = 0
+
+def report(cond, msg):
+    "Report success/fail of test"
+    if cond:
+        print ("PASS: %s" % (msg))
+    else:
+        print ("FAIL: %s" % (msg))
+        global failcount
+        failcount += 1
+
+def run_test():
+    "Run through the tests one by one"
+
+    auxv = gdb.execute("info auxv", False, True)
+    report(isinstance(auxv, str), "Fetched auxv from inferior")
+    report(auxv.find("sha1"), "Found test binary name in auxv")
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+    inferior = gdb.selected_inferior()
+    arch = inferior.architecture()
+    print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+    print("SKIPPING (not connected)", file=sys.stderr)
+    exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+    print("SKIP: PC not set")
+    exit(0)
+
+try:
+    # These are not very useful in scripts
+    gdb.execute("set pagination off")
+    gdb.execute("set confirm off")
+
+    # Run the actual tests
+    run_test()
+except (gdb.error):
+    print ("GDB Exception: %s" % (sys.exc_info()[0]))
+    failcount += 1
+    pass
+
+print("All tests complete: %d failures" % failcount)
+exit(failcount)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 07/20] gdbstub: drop CPUEnv from gdb_exit()
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (5 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 06/20] gdbstub: add support to Xfer:auxv:read: packet Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 08/20] gdbstub: drop gdbserver_cleanup in favour of gdb_exit Alex Bennée
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marek Vasut, Peter Maydell, Alex Bennée, Chris Wulff,
	Richard Henderson, Laurent Vivier, Philippe Mathieu-Daudé,
	open list:ARM TCG CPUs, Philippe Mathieu-Daudé

gdb_exit() has never needed anything from env and I doubt we are going
to start now.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201214153012.12723-5-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-7-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/gdbstub.h    | 2 +-
 bsd-user/syscall.c        | 6 +++---
 gdbstub.c                 | 2 +-
 linux-user/exit.c         | 2 +-
 target/arm/arm-semi.c     | 2 +-
 target/m68k/m68k-semi.c   | 2 +-
 target/nios2/nios2-semi.c | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 94d8f83e92..492db0f512 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -46,7 +46,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
 void gdb_set_stop_cpu(CPUState *cpu);
-void gdb_exit(CPUArchState *, int);
+void gdb_exit(int);
 #ifdef CONFIG_USER_ONLY
 /**
  * gdb_handlesig: yield control to gdb
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index d38ec7a162..adc3d21b54 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -333,7 +333,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef CONFIG_GPROF
         _mcleanup();
 #endif
-        gdb_exit(cpu_env, arg1);
+        gdb_exit(arg1);
         qemu_plugin_atexit_cb();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
@@ -435,7 +435,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef CONFIG_GPROF
         _mcleanup();
 #endif
-        gdb_exit(cpu_env, arg1);
+        gdb_exit(arg1);
         qemu_plugin_atexit_cb();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
@@ -514,7 +514,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef CONFIG_GPROF
         _mcleanup();
 #endif
-        gdb_exit(cpu_env, arg1);
+        gdb_exit(arg1);
         qemu_plugin_atexit_cb();
         /* XXX: should free thread stack and CPU env */
         _exit(arg1);
diff --git a/gdbstub.c b/gdbstub.c
index 15d3a8e1f5..afa553e8fc 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3068,7 +3068,7 @@ static void gdb_read_byte(uint8_t ch)
 }
 
 /* Tell the remote gdb that the process has exited.  */
-void gdb_exit(CPUArchState *env, int code)
+void gdb_exit(int code)
 {
   char buf[4];
 
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 1594015444..70b344048c 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -34,6 +34,6 @@ void preexit_cleanup(CPUArchState *env, int code)
 #ifdef CONFIG_GCOV
         __gcov_dump();
 #endif
-        gdb_exit(env, code);
+        gdb_exit(code);
         qemu_plugin_atexit_cb();
 }
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index f7b7bff522..93360e28c7 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -1101,7 +1101,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
              */
             ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
         }
-        gdb_exit(env, ret);
+        gdb_exit(ret);
         exit(ret);
     case TARGET_SYS_SYNCCACHE:
         /*
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 27600e0cc0..d919245e4f 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -195,7 +195,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
     args = env->dregs[1];
     switch (nr) {
     case HOSTED_EXIT:
-        gdb_exit(env, env->dregs[0]);
+        gdb_exit(env->dregs[0]);
         exit(env->dregs[0]);
     case HOSTED_OPEN:
         GET_ARG(0);
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index d7a80dd303..e508b2fafc 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -215,7 +215,7 @@ void do_nios2_semihosting(CPUNios2State *env)
     args = env->regs[R_ARG1];
     switch (nr) {
     case HOSTED_EXIT:
-        gdb_exit(env, env->regs[R_ARG0]);
+        gdb_exit(env->regs[R_ARG0]);
         exit(env->regs[R_ARG0]);
     case HOSTED_OPEN:
         GET_ARG(0);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 08/20] gdbstub: drop gdbserver_cleanup in favour of gdb_exit
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (6 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 07/20] gdbstub: drop CPUEnv from gdb_exit() Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 09/20] gdbstub: ensure we clean-up when terminated Alex Bennée
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé,
	Philippe Mathieu-Daudé

Despite it's name it didn't actually clean-up so let us document
gdb_exit() better and use that.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201214153012.12723-6-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-8-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/gdbstub.h | 14 +++++++++++---
 gdbstub.c              |  7 -------
 softmmu/runstate.c     |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 492db0f512..ff0b7bc45e 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -46,7 +46,17 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
 void gdb_set_stop_cpu(CPUState *cpu);
-void gdb_exit(int);
+
+/**
+ * gdb_exit: exit gdb session, reporting inferior status
+ * @code: exit code reported
+ *
+ * This closes the session and sends a final packet to GDB reporting
+ * the exit status of the program. It also cleans up any connections
+ * detritus before returning.
+ */
+void gdb_exit(int code);
+
 #ifdef CONFIG_USER_ONLY
 /**
  * gdb_handlesig: yield control to gdb
@@ -187,8 +197,6 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
  */
 int gdbserver_start(const char *port_or_device);
 
-void gdbserver_cleanup(void);
-
 /**
  * gdb_has_xml:
  * This is an ugly hack to cope with both new and old gdb.
diff --git a/gdbstub.c b/gdbstub.c
index afa553e8fc..bab8476357 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3547,13 +3547,6 @@ int gdbserver_start(const char *device)
     return 0;
 }
 
-void gdbserver_cleanup(void)
-{
-    if (gdbserver_state.init) {
-        put_packet("W00");
-    }
-}
-
 static void register_types(void)
 {
     type_register_static(&char_gdb_type_info);
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index 636aab0add..6177693a30 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -775,7 +775,7 @@ void qemu_init_subsystems(void)
 
 void qemu_cleanup(void)
 {
-    gdbserver_cleanup();
+    gdb_exit(0);
 
     /*
      * cleaning up the migration object cancels any existing migration
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 09/20] gdbstub: ensure we clean-up when terminated
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (7 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 08/20] gdbstub: drop gdbserver_cleanup in favour of gdb_exit Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers Alex Bennée
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé,
	Philippe Mathieu-Daudé

If you kill the inferior from GDB we end up leaving our socket lying
around. Fix this by calling gdb_exit() first.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201214153012.12723-7-alex.bennee@linaro.org>
Message-Id: <20201218112707.28348-9-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index bab8476357..8c301edf32 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1978,6 +1978,7 @@ static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
     /* Kill the target */
     put_packet("OK");
     error_report("QEMU: Terminated via GDBstub");
+    gdb_exit(0);
     exit(0);
 }
 
@@ -2539,6 +2540,7 @@ static int gdb_handle_packet(const char *line_buf)
     case 'k':
         /* Kill the target */
         error_report("QEMU: Terminated via GDBstub");
+        gdb_exit(0);
         exit(0);
     case 'D':
         {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (8 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 09/20] gdbstub: ensure we clean-up when terminated Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-11 13:20   ` Luis Machado
  2021-01-08 22:42 ` [PATCH v1 11/20] Makefile: add GNU global tags support Alex Bennée
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luis Machado, open list:ARM TCG CPUs, Alex Bennée, Peter Maydell

While GDB can work with any XML description given to it there is
special handling for SVE registers on the GDB side which makes the
users life a little better. The changes aren't that major and all the
registers save the $vg reported the same. All that changes is:

  - report org.gnu.gdb.aarch64.sve
  - use gdb nomenclature for names and types
  - minor re-ordering of the types to match reference
  - re-enable ieee_half (as we know gdb supports it now)
  - $vg is now a 64 bit int
  - check $vN and $zN aliasing in test

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Luis Machado <luis.machado@linaro.org>
Message-Id: <20201218112707.28348-10-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/gdbstub.c                        | 75 ++++++++-------------
 target/arm/helper.c                         |  2 +-
 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 11 +++
 3 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 866595b4f1..a8fff2a3d0 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
     { "uint128", 128, 'q', 'u' },
     { "int128", 128, 'q', 's' },
     /* 64 bit */
+    { "ieee_double", 64, 'd', 'f' },
     { "uint64", 64, 'd', 'u' },
     { "int64", 64, 'd', 's' },
-    { "ieee_double", 64, 'd', 'f' },
     /* 32 bit */
+    { "ieee_single", 32, 's', 'f' },
     { "uint32", 32, 's', 'u' },
     { "int32", 32, 's', 's' },
-    { "ieee_single", 32, 's', 'f' },
     /* 16 bit */
+    { "ieee_half", 16, 'h', 'f' },
     { "uint16", 16, 'h', 'u' },
     { "int16", 16, 'h', 's' },
-    /*
-     * TODO: currently there is no reliable way of telling
-     * if the remote gdb actually understands ieee_half so
-     * we don't expose it in the target description for now.
-     * { "ieee_half", 16, 'h', 'f' },
-     */
     /* bytes */
     { "uint8", 8, 'b', 'u' },
     { "int8", 8, 'b', 's' },
@@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
     GString *s = g_string_new(NULL);
     DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
     g_autoptr(GString) ts = g_string_new("");
-    int i, bits, reg_width = (cpu->sve_max_vq * 128);
+    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
     info->num = 0;
     g_string_printf(s, "<?xml version=\"1.0\"?>");
     g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
-    g_string_append_printf(s, "<feature name=\"org.qemu.gdb.aarch64.sve\">");
+    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
 
     /* First define types and totals in a whole VL */
     for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
         int count = reg_width / vec_lanes[i].size;
-        g_string_printf(ts, "vq%d%c%c", count,
-                        vec_lanes[i].sz, vec_lanes[i].suffix);
+        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
         g_string_append_printf(s,
                                "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
                                ts->str, vec_lanes[i].gdb_type, count);
@@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
      * signed and potentially float versions of each size from 128 to
      * 8 bits.
      */
-    for (bits = 128; bits >= 8; bits /= 2) {
-        int count = reg_width / bits;
-        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
-        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-            if (vec_lanes[i].size == bits) {
-                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
-                                       vec_lanes[i].suffix,
-                                       count,
-                                       vec_lanes[i].sz, vec_lanes[i].suffix);
+    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
+        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
+            if (vec_lanes[j].size == bits) {
+                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
+                                       vec_lanes[j].suffix,
+                                       vec_lanes[j].sz, vec_lanes[j].suffix);
             }
         }
         g_string_append(s, "</union>");
     }
     /* And now the final union of unions */
-    g_string_append(s, "<union id=\"vq\">");
-    for (bits = 128; bits >= 8; bits /= 2) {
-        int count = reg_width / bits;
-        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-            if (vec_lanes[i].size == bits) {
-                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
-                                       vec_lanes[i].sz, count);
-                break;
-            }
-        }
+    g_string_append(s, "<union id=\"svev\">");
+    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
+                               suf[i], suf[i]);
     }
     g_string_append(s, "</union>");
 
+    /* Finally the sve prefix type */
+    g_string_append_printf(s,
+                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
+                           reg_width / 8);
+
     /* Then define each register in parts for each vq */
     for (i = 0; i < 32; i++) {
         g_string_append_printf(s,
                                "<reg name=\"z%d\" bitsize=\"%d\""
-                               " regnum=\"%d\" group=\"vector\""
-                               " type=\"vq\"/>",
+                               " regnum=\"%d\" type=\"svev\"/>",
                                i, reg_width, base_reg++);
         info->num++;
     }
@@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
                            " regnum=\"%d\" group=\"float\""
                            " type=\"int\"/>", base_reg++);
     info->num += 2;
-    /*
-     * Predicate registers aren't so big they are worth splitting up
-     * but we do need to define a type to hold the array of quad
-     * references.
-     */
-    g_string_append_printf(s,
-                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
-                           cpu->sve_max_vq);
+
     for (i = 0; i < 16; i++) {
         g_string_append_printf(s,
                                "<reg name=\"p%d\" bitsize=\"%d\""
-                               " regnum=\"%d\" group=\"vector\""
-                               " type=\"vqp\"/>",
+                               " regnum=\"%d\" type=\"svep\"/>",
                                i, cpu->sve_max_vq * 16, base_reg++);
         info->num++;
     }
     g_string_append_printf(s,
                            "<reg name=\"ffr\" bitsize=\"%d\""
                            " regnum=\"%d\" group=\"vector\""
-                           " type=\"vqp\"/>",
+                           " type=\"svep\"/>",
                            cpu->sve_max_vq * 16, base_reg++);
     g_string_append_printf(s,
                            "<reg name=\"vg\" bitsize=\"64\""
-                           " regnum=\"%d\" group=\"vector\""
-                           " type=\"uint32\"/>",
+                           " regnum=\"%d\" type=\"int\"/>",
                            base_reg++);
     info->num += 2;
     g_string_append_printf(s, "</feature>");
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d077dd9ef5..d434044f07 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
          * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
          */
         int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
-        return gdb_get_reg32(buf, vq * 2);
+        return gdb_get_reg64(buf, vq * 2);
     }
     default:
         /* gdbstub asked for something out our range */
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index 972cf73c31..b9ef169c1a 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
         except gdb.error:
             report(False, "checking zregs (out of range)")
 
+        # Check the aliased V registers are set and GDB has correctly
+        # created them for us having recognised and handled SVE.
+        try:
+            for i in range(0, 16):
+                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
+                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
+                report(int(val_z) == int(val_v),
+                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
+        except gdb.error:
+            report(False, "checking vregs (out of range)")
+
 
 def run_test():
     "Run through the tests one by one"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 11/20] Makefile: add GNU global tags support
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (9 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories Alex Bennée
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

GNU Global is another tags engine which is more like cscope in being
able to support finding both references and definitions. You will be
un-surprised to know it also integrates well with Emacs.

The main benefit of integrating it into find-src-path is it takes less
time to rebuild the database from scratch when you have a lot of build
directories under your source tree.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 Makefile   | 9 ++++++++-
 .gitignore | 3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index fb9923ff22..66eec99685 100644
--- a/Makefile
+++ b/Makefile
@@ -253,6 +253,13 @@ ctags:
 	rm -f "$(SRC_PATH)/"tags
 	$(find-src-path) -exec ctags -f "$(SRC_PATH)/"tags --append {} +
 
+.PHONY: gtags
+gtags:
+	rm -f "$(SRC_PATH)/"GTAGS
+	rm -f "$(SRC_PATH)/"GRTAGS
+	rm -f "$(SRC_PATH)/"GPATH
+	$(find-src-path) | gtags -f -
+
 .PHONY: TAGS
 TAGS:
 	rm -f "$(SRC_PATH)/"TAGS
@@ -279,7 +286,7 @@ help:
 	$(call print-help,all,Build all)
 	$(call print-help,dir/file.o,Build specified target only)
 	$(call print-help,install,Install QEMU, documentation and tools)
-	$(call print-help,ctags/TAGS,Generate tags file for editors)
+	$(call print-help,ctags/gtags/TAGS,Generate tags file for editors)
 	$(call print-help,cscope,Generate cscope index)
 	$(call print-help,sparse,Run sparse on the QEMU source)
 	@echo  ''
diff --git a/.gitignore b/.gitignore
index b32bca1315..75a4be0724 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,9 @@
 cscope.*
 tags
 TAGS
+GPATH
+GRTAGS
+GTAGS
 *~
 *.ast_raw
 *.depend_raw
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (10 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 11/20] Makefile: add GNU global tags support Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-11 15:30   ` Philippe Mathieu-Daudé
  2021-01-11 15:46   ` Philippe Mathieu-Daudé
  2021-01-08 22:42 ` [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent Alex Bennée
                   ` (7 subsequent siblings)
  19 siblings, 2 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Keith Packard, Laurent Vivier,
	open list:ARM TCG CPUs, Alistair Francis, Alex Bennée

From: Keith Packard <keithp@keithp.com>

This commit renames two files which provide ARM semihosting support so
that they can be shared by other architectures:

 1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
 2. linux-user/arm/semihost.c -> linux-user/semihost.c

The build system was modified use a new config variable,
CONFIG_ARM_COMPATIBLE_SEMIHOSTING, which has been added to the ARM
softmmu and linux-user default configs. The contents of the source
files has not been changed in this patch.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210107170717.2098982-2-keithp@keithp.com>
---
 default-configs/devices/arm-softmmu.mak               | 1 +
 default-configs/targets/aarch64-linux-user.mak        | 1 +
 default-configs/targets/aarch64_be-linux-user.mak     | 1 +
 default-configs/targets/arm-linux-user.mak            | 1 +
 default-configs/targets/armeb-linux-user.mak          | 1 +
 target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
 linux-user/{arm => }/semihost.c                       | 0
 hw/semihosting/Kconfig                                | 3 +++
 hw/semihosting/meson.build                            | 3 +++
 linux-user/arm/meson.build                            | 3 ---
 linux-user/meson.build                                | 1 +
 target/arm/meson.build                                | 2 --
 12 files changed, 12 insertions(+), 5 deletions(-)
 rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
 rename linux-user/{arm => }/semihost.c (100%)

diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
index 08a32123b4..0500156a0c 100644
--- a/default-configs/devices/arm-softmmu.mak
+++ b/default-configs/devices/arm-softmmu.mak
@@ -42,4 +42,5 @@ CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
 CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 CONFIG_ALLWINNER_H3=y
diff --git a/default-configs/targets/aarch64-linux-user.mak b/default-configs/targets/aarch64-linux-user.mak
index 163c9209f4..4713253709 100644
--- a/default-configs/targets/aarch64-linux-user.mak
+++ b/default-configs/targets/aarch64-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=aarch64
 TARGET_BASE_ARCH=arm
 TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_HAS_BFLT=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/default-configs/targets/aarch64_be-linux-user.mak b/default-configs/targets/aarch64_be-linux-user.mak
index 4c953cf8c5..fae831558d 100644
--- a/default-configs/targets/aarch64_be-linux-user.mak
+++ b/default-configs/targets/aarch64_be-linux-user.mak
@@ -3,3 +3,4 @@ TARGET_BASE_ARCH=arm
 TARGET_WORDS_BIGENDIAN=y
 TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_HAS_BFLT=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/default-configs/targets/arm-linux-user.mak b/default-configs/targets/arm-linux-user.mak
index c7cd872e86..e741ffd4d3 100644
--- a/default-configs/targets/arm-linux-user.mak
+++ b/default-configs/targets/arm-linux-user.mak
@@ -3,3 +3,4 @@ TARGET_SYSTBL_ABI=common,oabi
 TARGET_SYSTBL=syscall.tbl
 TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_HAS_BFLT=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/default-configs/targets/armeb-linux-user.mak b/default-configs/targets/armeb-linux-user.mak
index 79bf10e99b..255e44e8b0 100644
--- a/default-configs/targets/armeb-linux-user.mak
+++ b/default-configs/targets/armeb-linux-user.mak
@@ -4,3 +4,4 @@ TARGET_SYSTBL=syscall.tbl
 TARGET_WORDS_BIGENDIAN=y
 TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
 TARGET_HAS_BFLT=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/target/arm/arm-semi.c b/hw/semihosting/common-semi.c
similarity index 100%
rename from target/arm/arm-semi.c
rename to hw/semihosting/common-semi.c
diff --git a/linux-user/arm/semihost.c b/linux-user/semihost.c
similarity index 100%
rename from linux-user/arm/semihost.c
rename to linux-user/semihost.c
diff --git a/hw/semihosting/Kconfig b/hw/semihosting/Kconfig
index efe0a30734..4c30dc6b16 100644
--- a/hw/semihosting/Kconfig
+++ b/hw/semihosting/Kconfig
@@ -1,3 +1,6 @@
 
 config SEMIHOSTING
        bool
+
+config ARM_COMPATIBLE_SEMIHOSTING
+       bool
diff --git a/hw/semihosting/meson.build b/hw/semihosting/meson.build
index f40ac574c4..5b4a170270 100644
--- a/hw/semihosting/meson.build
+++ b/hw/semihosting/meson.build
@@ -2,3 +2,6 @@ specific_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files(
   'config.c',
   'console.c',
 ))
+
+specific_ss.add(when: ['CONFIG_ARM_COMPATIBLE_SEMIHOSTING'],
+		if_true: files('common-semi.c'))
diff --git a/linux-user/arm/meson.build b/linux-user/arm/meson.build
index 432984b58e..5a93c925cf 100644
--- a/linux-user/arm/meson.build
+++ b/linux-user/arm/meson.build
@@ -1,6 +1,3 @@
-linux_user_ss.add(when: 'TARGET_AARCH64', if_true: files('semihost.c'))
-linux_user_ss.add(when: 'TARGET_ARM', if_true: files('semihost.c'))
-
 subdir('nwfpe')
 
 syscall_nr_generators += {
diff --git a/linux-user/meson.build b/linux-user/meson.build
index 2b94e4ba24..7fe28d659e 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -16,6 +16,7 @@ linux_user_ss.add(rt)
 
 linux_user_ss.add(when: 'TARGET_HAS_BFLT', if_true: files('flatload.c'))
 linux_user_ss.add(when: 'TARGET_I386', if_true: files('vm86.c'))
+linux_user_ss.add(when: 'CONFIG_ARM_COMPATIBLE_SEMIHOSTING', if_true: files('semihost.c'))
 
 
 syscall_nr_generators = {}
diff --git a/target/arm/meson.build b/target/arm/meson.build
index f5de2a77b8..15b936c101 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -32,8 +32,6 @@ arm_ss.add(files(
 ))
 arm_ss.add(zlib)
 
-arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))
-
 arm_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (11 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-11 15:34   ` Philippe Mathieu-Daudé
  2021-01-08 22:42 ` [PATCH v1 14/20] semihosting: Change internal common-semi interfaces to use CPUState * Alex Bennée
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Keith Packard, Laurent Vivier,
	open list:ARM TCG CPUs, Alistair Francis, Alex Bennée

From: Keith Packard <keithp@keithp.com>

The public API is now defined in
hw/semihosting/common-semi.h. do_common_semihosting takes CPUState *
instead of CPUARMState *. All internal functions have been renamed
common_semi_ instead of arm_semi_ or arm_. Aside from the API change,
there are no functional changes in this patch.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20210107170717.2098982-3-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/semihosting/common-semi.h  | 36 +++++++++++++++++++++++++++++++++++
 target/arm/cpu.h              |  8 --------
 hw/semihosting/common-semi.c  | 16 ++++++++++------
 linux-user/aarch64/cpu_loop.c |  3 ++-
 linux-user/arm/cpu_loop.c     |  3 ++-
 target/arm/helper.c           |  5 +++--
 target/arm/m_helper.c         |  7 ++++++-
 7 files changed, 59 insertions(+), 19 deletions(-)
 create mode 100644 hw/semihosting/common-semi.h

diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
new file mode 100644
index 0000000000..bc53e92c79
--- /dev/null
+++ b/hw/semihosting/common-semi.h
@@ -0,0 +1,36 @@
+/*
+ *  Semihosting support for systems modeled on the Arm "Angel"
+ *  semihosting syscalls design.
+ *
+ *  Copyright (c) 2005, 2007 CodeSourcery.
+ *  Copyright (c) 2019 Linaro
+ *  Written by Paul Brook.
+ *
+ *  Copyright © 2020 by Keith Packard <keithp@keithp.com>
+ *  Adapted for systems other than ARM, including RISC-V, by Keith Packard
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ *  ARM Semihosting is documented in:
+ *     Semihosting for AArch32 and AArch64 Release 2.0
+ *     https://static.docs.arm.com/100863/0200/semihosting.pdf
+ *
+ */
+
+#ifndef COMMON_SEMI_H
+#define COMMON_SEMI_H
+
+target_ulong do_common_semihosting(CPUState *cs);
+
+#endif /* COMMON_SEMI_H */
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 7e6c881a7e..49d9a314db 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1068,14 +1068,6 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o,
 static inline void aarch64_add_sve_properties(Object *obj) { }
 #endif
 
-#if !defined(CONFIG_TCG)
-static inline target_ulong do_arm_semihosting(CPUARMState *env)
-{
-    g_assert_not_reached();
-}
-#else
-target_ulong do_arm_semihosting(CPUARMState *env);
-#endif
 void aarch64_sync_32_to_64(CPUARMState *env);
 void aarch64_sync_64_to_32(CPUARMState *env);
 
diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 93360e28c7..2e959aba08 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -1,10 +1,14 @@
 /*
- *  Arm "Angel" semihosting syscalls
+ *  Semihosting support for systems modeled on the Arm "Angel"
+ *  semihosting syscalls design.
  *
  *  Copyright (c) 2005, 2007 CodeSourcery.
  *  Copyright (c) 2019 Linaro
  *  Written by Paul Brook.
  *
+ *  Copyright © 2020 by Keith Packard <keithp@keithp.com>
+ *  Adapted for systems other than ARM, including RISC-V, by Keith Packard
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
  *  the Free Software Foundation; either version 2 of the License, or
@@ -373,12 +377,12 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
      * do anything with its return value, because it is not necessarily
      * the result of the syscall, but could just be the old value of X0.
      * The only thing safe to do with this is that the callers of
-     * do_arm_semihosting() will write it straight back into X0.
+     * do_common_semihosting() will write it straight back into X0.
      * (In linux-user mode, the callback will have happened before
      * gdb_do_syscallv() returns.)
      *
      * We should tidy this up so neither this function nor
-     * do_arm_semihosting() return a value, so the mistake of
+     * do_common_semihosting() return a value, so the mistake of
      * doing something with the return value is not possible to make.
      */
 
@@ -675,10 +679,10 @@ static const GuestFDFunctions guestfd_fns[] = {
  * leave the register unchanged. We use 0xdeadbeef as the return value
  * when there isn't a defined return value for the call.
  */
-target_ulong do_arm_semihosting(CPUARMState *env)
+target_ulong do_common_semihosting(CPUState *cs)
 {
-    ARMCPU *cpu = env_archcpu(env);
-    CPUState *cs = env_cpu(env);
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
     target_ulong args;
     target_ulong arg0, arg1, arg2, arg3;
     char * s;
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index bbe9fefca8..42b9c15f53 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -22,6 +22,7 @@
 #include "qemu.h"
 #include "cpu_loop-common.h"
 #include "qemu/guest-random.h"
+#include "hw/semihosting/common-semi.h"
 
 #define get_user_code_u32(x, gaddr, env)                \
     ({ abi_long __r = get_user_u32((x), (gaddr));       \
@@ -129,7 +130,7 @@ void cpu_loop(CPUARMState *env)
             queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
             break;
         case EXCP_SEMIHOST:
-            env->xregs[0] = do_arm_semihosting(env);
+            env->xregs[0] = do_common_semihosting(cs);
             env->pc += 4;
             break;
         case EXCP_YIELD:
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 3d272b56ef..cadfb7fa43 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -22,6 +22,7 @@
 #include "qemu.h"
 #include "elf.h"
 #include "cpu_loop-common.h"
+#include "hw/semihosting/common-semi.h"
 
 #define get_user_code_u32(x, gaddr, env)                \
     ({ abi_long __r = get_user_u32((x), (gaddr));       \
@@ -421,7 +422,7 @@ void cpu_loop(CPUARMState *env)
             }
             break;
         case EXCP_SEMIHOST:
-            env->regs[0] = do_arm_semihosting(env);
+            env->regs[0] = do_common_semihosting(cs);
             env->regs[15] += env->thumb ? 2 : 4;
             break;
         case EXCP_INTERRUPT:
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d434044f07..a2ad77eb4e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -34,6 +34,7 @@
 #ifdef CONFIG_TCG
 #include "arm_ldst.h"
 #include "exec/cpu_ldst.h"
+#include "hw/semihosting/common-semi.h"
 #endif
 
 #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
@@ -9875,13 +9876,13 @@ static void handle_semihosting(CPUState *cs)
         qemu_log_mask(CPU_LOG_INT,
                       "...handling as semihosting call 0x%" PRIx64 "\n",
                       env->xregs[0]);
-        env->xregs[0] = do_arm_semihosting(env);
+        env->xregs[0] = do_common_semihosting(cs);
         env->pc += 4;
     } else {
         qemu_log_mask(CPU_LOG_INT,
                       "...handling as semihosting call 0x%x\n",
                       env->regs[0]);
-        env->regs[0] = do_arm_semihosting(env);
+        env->regs[0] = do_common_semihosting(cs);
         env->regs[15] += env->thumb ? 2 : 4;
     }
 }
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 643dcafb83..6176003029 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -31,6 +31,7 @@
 #ifdef CONFIG_TCG
 #include "arm_ldst.h"
 #include "exec/cpu_ldst.h"
+#include "hw/semihosting/common-semi.h"
 #endif
 
 static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,
@@ -2306,7 +2307,11 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
         qemu_log_mask(CPU_LOG_INT,
                       "...handling as semihosting call 0x%x\n",
                       env->regs[0]);
-        env->regs[0] = do_arm_semihosting(env);
+#ifdef CONFIG_TCG
+        env->regs[0] = do_common_semihosting(cs);
+#else
+        g_assert_not_reached();
+#endif
         env->regs[15] += env->thumb ? 2 : 4;
         return;
     case EXCP_BKPT:
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 14/20] semihosting: Change internal common-semi interfaces to use CPUState *
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (12 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 15/20] semihosting: Support SYS_HEAPINFO when env->boot_info is not set Alex Bennée
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée, Keith Packard, Alistair Francis

From: Keith Packard <keithp@keithp.com>

This makes all of the internal interfaces architecture-independent and
renames the internal functions to use the 'common_semi' prefix instead
of 'arm' or 'arm_semi'.

To do this, some new architecture-specific internal helper functions
were created:

    static inline target_ulong
    common_semi_arg(CPUState *cs, int argno)

	Returns the argno'th semihosting argument, where argno can be
	either 0 or 1.

    static inline void
    common_semi_set_ret(CPUState *cs, target_ulong ret)

	Sets the semihosting return value.

    static inline bool
    common_semi_sys_exit_extended(CPUState *cs, int nr)

	This detects whether the specified semihosting call, which
	is either TARGET_SYS_EXIT or TARGET_SYS_EXIT_EXTENDED should
	be executed using the TARGET_SYS_EXIT_EXTENDED semantics.

    static inline target_ulong
    common_semi_rambase(CPUState *cs)

	Returns the base of RAM region used for heap and stack. This
	is used to construct plausible values for the SYS_HEAPINFO
	call.

In addition, several existing functions have been changed to flag
areas of code which are architecture specific:

    static target_ulong
    common_semi_flen_buf(CPUState *cs)

	Returns the current stack pointer minus 64, which is
	where a stat structure will be placed on the stack

    #define GET_ARG(n)

	This fetches arguments from the semihosting command's argument
	block. The address of this is available implicitly through the
	local 'args' variable. This is *mostly* architecture
	independent, but does depend on the current ABI's notion of
	the size of a 'long' parameter, which may need run-time checks
	(as it does on AARCH64)

    #define SET_ARG(n, val)

	This mirrors GET_ARG and stores data back into the argument
	block.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20210107170717.2098982-4-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/semihosting/common-semi.c | 349 +++++++++++++++++++----------------
 1 file changed, 186 insertions(+), 163 deletions(-)

diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 2e959aba08..ac1271545e 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -32,15 +32,18 @@
 #include "cpu.h"
 #include "hw/semihosting/semihost.h"
 #include "hw/semihosting/console.h"
+#include "hw/semihosting/common-semi.h"
 #include "qemu/log.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 
-#define ARM_ANGEL_HEAP_SIZE (128 * 1024 * 1024)
+#define COMMON_SEMI_HEAP_SIZE (128 * 1024 * 1024)
 #else
 #include "exec/gdbstub.h"
 #include "qemu/cutils.h"
+#ifdef TARGET_ARM
 #include "hw/arm/boot.h"
+#endif
 #include "hw/boards.h"
 #endif
 
@@ -134,6 +137,50 @@ typedef struct GuestFD {
 
 static GArray *guestfd_array;
 
+#ifdef TARGET_ARM
+static inline target_ulong
+common_semi_arg(CPUState *cs, int argno)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    if (is_a64(env)) {
+        return env->xregs[argno];
+    } else {
+        return env->regs[argno];
+    }
+}
+
+static inline void
+common_semi_set_ret(CPUState *cs, target_ulong ret)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    if (is_a64(env)) {
+        env->xregs[0] = ret;
+    } else {
+        env->regs[0] = ret;
+    }
+}
+
+static inline bool
+common_semi_sys_exit_extended(CPUState *cs, int nr)
+{
+    return (nr == TARGET_SYS_EXIT_EXTENDED || is_a64(cs->env_ptr));
+}
+
+#ifndef CONFIG_USER_ONLY
+#include "hw/arm/boot.h"
+static inline target_ulong
+common_semi_rambase(CPUState *cs)
+{
+    CPUArchState *env = cs->env_ptr;
+    const struct arm_boot_info *info = env->boot_info;
+    return info->loader_start;
+}
+#endif
+
+#endif /* TARGET_ARM */
+
 /*
  * Allocate a new guest file descriptor and return it; if we
  * couldn't allocate a new fd then return -1.
@@ -239,11 +286,10 @@ static target_ulong syscall_err;
 #include "exec/softmmu-semi.h"
 #endif
 
-static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code)
+static inline uint32_t set_swi_errno(CPUState *cs, uint32_t code)
 {
     if (code == (uint32_t)-1) {
 #ifdef CONFIG_USER_ONLY
-        CPUState *cs = env_cpu(env);
         TaskState *ts = cs->opaque;
 
         ts->swi_errno = errno;
@@ -254,10 +300,9 @@ static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code)
     return code;
 }
 
-static inline uint32_t get_swi_errno(CPUARMState *env)
+static inline uint32_t get_swi_errno(CPUState *cs)
 {
 #ifdef CONFIG_USER_ONLY
-    CPUState *cs = env_cpu(env);
     TaskState *ts = cs->opaque;
 
     return ts->swi_errno;
@@ -266,24 +311,22 @@ static inline uint32_t get_swi_errno(CPUARMState *env)
 #endif
 }
 
-static target_ulong arm_semi_syscall_len;
+static target_ulong common_semi_syscall_len;
 
-static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
+static void common_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-    target_ulong reg0 = is_a64(env) ? env->xregs[0] : env->regs[0];
+    target_ulong reg0 = common_semi_arg(cs, 0);
 
     if (ret == (target_ulong)-1) {
         errno = err;
-        set_swi_errno(env, -1);
+        set_swi_errno(cs, -1);
         reg0 = ret;
     } else {
         /* Fixup syscalls that use nonstardard return conventions.  */
         switch (reg0) {
         case TARGET_SYS_WRITE:
         case TARGET_SYS_READ:
-            reg0 = arm_semi_syscall_len - ret;
+            reg0 = common_semi_syscall_len - ret;
             break;
         case TARGET_SYS_SEEK:
             reg0 = 0;
@@ -293,77 +336,66 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
             break;
         }
     }
-    if (is_a64(env)) {
-        env->xregs[0] = reg0;
-    } else {
-        env->regs[0] = reg0;
-    }
+    common_semi_set_ret(cs, reg0);
 }
 
-static target_ulong arm_flen_buf(ARMCPU *cpu)
+static target_ulong common_semi_flen_buf(CPUState *cs)
 {
+    target_ulong sp;
+#ifdef TARGET_ARM
     /* Return an address in target memory of 64 bytes where the remote
      * gdb should write its stat struct. (The format of this structure
      * is defined by GDB's remote protocol and is not target-specific.)
      * We put this on the guest's stack just below SP.
      */
+    ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    target_ulong sp;
 
     if (is_a64(env)) {
         sp = env->xregs[31];
     } else {
         sp = env->regs[13];
     }
+#endif
 
     return sp - 64;
 }
 
-static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
+static void
+common_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
     /* The size is always stored in big-endian order, extract
        the value. We assume the size always fit in 32 bits.  */
     uint32_t size;
-    cpu_memory_rw_debug(cs, arm_flen_buf(cpu) + 32, (uint8_t *)&size, 4, 0);
+    cpu_memory_rw_debug(cs, common_semi_flen_buf(cs) + 32,
+                        (uint8_t *)&size, 4, 0);
     size = be32_to_cpu(size);
-    if (is_a64(env)) {
-        env->xregs[0] = size;
-    } else {
-        env->regs[0] = size;
-    }
+    common_semi_set_ret(cs, size);
     errno = err;
-    set_swi_errno(env, -1);
+    set_swi_errno(cs, -1);
 }
 
-static int arm_semi_open_guestfd;
+static int common_semi_open_guestfd;
 
-static void arm_semi_open_cb(CPUState *cs, target_ulong ret, target_ulong err)
+static void
+common_semi_open_cb(CPUState *cs, target_ulong ret, target_ulong err)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
     if (ret == (target_ulong)-1) {
         errno = err;
-        set_swi_errno(env, -1);
-        dealloc_guestfd(arm_semi_open_guestfd);
+        set_swi_errno(cs, -1);
+        dealloc_guestfd(common_semi_open_guestfd);
     } else {
-        associate_guestfd(arm_semi_open_guestfd, ret);
-        ret = arm_semi_open_guestfd;
-    }
-
-    if (is_a64(env)) {
-        env->xregs[0] = ret;
-    } else {
-        env->regs[0] = ret;
+        associate_guestfd(common_semi_open_guestfd, ret);
+        ret = common_semi_open_guestfd;
     }
+    common_semi_set_ret(cs, ret);
 }
 
-static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
-                                    const char *fmt, ...)
+static target_ulong
+common_semi_gdb_syscall(CPUState *cs, gdb_syscall_complete_cb cb,
+                        const char *fmt, ...)
 {
     va_list va;
-    CPUARMState *env = &cpu->env;
 
     va_start(va, fmt);
     gdb_do_syscallv(cb, fmt, va);
@@ -386,7 +418,7 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
      * doing something with the return value is not possible to make.
      */
 
-    return is_a64(env) ? env->xregs[0] : env->regs[0];
+    return common_semi_arg(cs, 0);
 }
 
 /*
@@ -395,20 +427,18 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
  * do the work and return the required return value for the guest,
  * setting the guest errno if appropriate.
  */
-typedef uint32_t sys_closefn(ARMCPU *cpu, GuestFD *gf);
-typedef uint32_t sys_writefn(ARMCPU *cpu, GuestFD *gf,
+typedef uint32_t sys_closefn(CPUState *cs, GuestFD *gf);
+typedef uint32_t sys_writefn(CPUState *cs, GuestFD *gf,
                              target_ulong buf, uint32_t len);
-typedef uint32_t sys_readfn(ARMCPU *cpu, GuestFD *gf,
+typedef uint32_t sys_readfn(CPUState *cs, GuestFD *gf,
                             target_ulong buf, uint32_t len);
-typedef uint32_t sys_isattyfn(ARMCPU *cpu, GuestFD *gf);
-typedef uint32_t sys_seekfn(ARMCPU *cpu, GuestFD *gf,
+typedef uint32_t sys_isattyfn(CPUState *cs, GuestFD *gf);
+typedef uint32_t sys_seekfn(CPUState *cs, GuestFD *gf,
                             target_ulong offset);
-typedef uint32_t sys_flenfn(ARMCPU *cpu, GuestFD *gf);
+typedef uint32_t sys_flenfn(CPUState *cs, GuestFD *gf);
 
-static uint32_t host_closefn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t host_closefn(CPUState *cs, GuestFD *gf)
 {
-    CPUARMState *env = &cpu->env;
-
     /*
      * Only close the underlying host fd if it's one we opened on behalf
      * of the guest in SYS_OPEN.
@@ -418,20 +448,21 @@ static uint32_t host_closefn(ARMCPU *cpu, GuestFD *gf)
         gf->hostfd == STDERR_FILENO) {
         return 0;
     }
-    return set_swi_errno(env, close(gf->hostfd));
+    return set_swi_errno(cs, close(gf->hostfd));
 }
 
-static uint32_t host_writefn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t host_writefn(CPUState *cs, GuestFD *gf,
                              target_ulong buf, uint32_t len)
 {
+    CPUArchState *env = cs->env_ptr;
     uint32_t ret;
-    CPUARMState *env = &cpu->env;
     char *s = lock_user(VERIFY_READ, buf, len, 1);
+    (void) env; /* Used in arm softmmu lock_user implicitly */
     if (!s) {
         /* Return bytes not written on error */
         return len;
     }
-    ret = set_swi_errno(env, write(gf->hostfd, s, len));
+    ret = set_swi_errno(cs, write(gf->hostfd, s, len));
     unlock_user(s, buf, 0);
     if (ret == (uint32_t)-1) {
         ret = 0;
@@ -440,18 +471,19 @@ static uint32_t host_writefn(ARMCPU *cpu, GuestFD *gf,
     return len - ret;
 }
 
-static uint32_t host_readfn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t host_readfn(CPUState *cs, GuestFD *gf,
                             target_ulong buf, uint32_t len)
 {
+    CPUArchState *env = cs->env_ptr;
     uint32_t ret;
-    CPUARMState *env = &cpu->env;
     char *s = lock_user(VERIFY_WRITE, buf, len, 0);
+    (void) env; /* Used in arm softmmu lock_user implicitly */
     if (!s) {
         /* return bytes not read */
         return len;
     }
     do {
-        ret = set_swi_errno(env, read(gf->hostfd, s, len));
+        ret = set_swi_errno(cs, read(gf->hostfd, s, len));
     } while (ret == -1 && errno == EINTR);
     unlock_user(s, buf, len);
     if (ret == (uint32_t)-1) {
@@ -461,68 +493,66 @@ static uint32_t host_readfn(ARMCPU *cpu, GuestFD *gf,
     return len - ret;
 }
 
-static uint32_t host_isattyfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t host_isattyfn(CPUState *cs, GuestFD *gf)
 {
     return isatty(gf->hostfd);
 }
 
-static uint32_t host_seekfn(ARMCPU *cpu, GuestFD *gf, target_ulong offset)
+static uint32_t host_seekfn(CPUState *cs, GuestFD *gf, target_ulong offset)
 {
-    CPUARMState *env = &cpu->env;
-    uint32_t ret = set_swi_errno(env, lseek(gf->hostfd, offset, SEEK_SET));
+    uint32_t ret = set_swi_errno(cs, lseek(gf->hostfd, offset, SEEK_SET));
     if (ret == (uint32_t)-1) {
         return -1;
     }
     return 0;
 }
 
-static uint32_t host_flenfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t host_flenfn(CPUState *cs, GuestFD *gf)
 {
-    CPUARMState *env = &cpu->env;
     struct stat buf;
-    uint32_t ret = set_swi_errno(env, fstat(gf->hostfd, &buf));
+    uint32_t ret = set_swi_errno(cs, fstat(gf->hostfd, &buf));
     if (ret == (uint32_t)-1) {
         return -1;
     }
     return buf.st_size;
 }
 
-static uint32_t gdb_closefn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t gdb_closefn(CPUState *cs, GuestFD *gf)
 {
-    return arm_gdb_syscall(cpu, arm_semi_cb, "close,%x", gf->hostfd);
+    return common_semi_gdb_syscall(cs, common_semi_cb, "close,%x", gf->hostfd);
 }
 
-static uint32_t gdb_writefn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t gdb_writefn(CPUState *cs, GuestFD *gf,
                             target_ulong buf, uint32_t len)
 {
-    arm_semi_syscall_len = len;
-    return arm_gdb_syscall(cpu, arm_semi_cb, "write,%x,%x,%x",
-                           gf->hostfd, buf, len);
+    common_semi_syscall_len = len;
+    return common_semi_gdb_syscall(cs, common_semi_cb, "write,%x,%x,%x",
+                                   gf->hostfd, buf, len);
 }
 
-static uint32_t gdb_readfn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t gdb_readfn(CPUState *cs, GuestFD *gf,
                            target_ulong buf, uint32_t len)
 {
-    arm_semi_syscall_len = len;
-    return arm_gdb_syscall(cpu, arm_semi_cb, "read,%x,%x,%x",
-                           gf->hostfd, buf, len);
+    common_semi_syscall_len = len;
+    return common_semi_gdb_syscall(cs, common_semi_cb, "read,%x,%x,%x",
+                                   gf->hostfd, buf, len);
 }
 
-static uint32_t gdb_isattyfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t gdb_isattyfn(CPUState *cs, GuestFD *gf)
 {
-    return arm_gdb_syscall(cpu, arm_semi_cb, "isatty,%x", gf->hostfd);
+    return common_semi_gdb_syscall(cs, common_semi_cb, "isatty,%x", gf->hostfd);
 }
 
-static uint32_t gdb_seekfn(ARMCPU *cpu, GuestFD *gf, target_ulong offset)
+static uint32_t gdb_seekfn(CPUState *cs, GuestFD *gf, target_ulong offset)
 {
-    return arm_gdb_syscall(cpu, arm_semi_cb, "lseek,%x,%x,0",
-                           gf->hostfd, offset);
+    return common_semi_gdb_syscall(cs, common_semi_cb, "lseek,%x,%x,0",
+                                   gf->hostfd, offset);
 }
 
-static uint32_t gdb_flenfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t gdb_flenfn(CPUState *cs, GuestFD *gf)
 {
-    return arm_gdb_syscall(cpu, arm_semi_flen_cb, "fstat,%x,%x",
-                           gf->hostfd, arm_flen_buf(cpu));
+    return common_semi_gdb_syscall(cs, common_semi_flen_cb, "fstat,%x,%x",
+                                   gf->hostfd, common_semi_flen_buf(cs));
 }
 
 #define SHFB_MAGIC_0 0x53
@@ -551,31 +581,29 @@ static void init_featurefile_guestfd(int guestfd)
     gf->featurefile_offset = 0;
 }
 
-static uint32_t featurefile_closefn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t featurefile_closefn(CPUState *cs, GuestFD *gf)
 {
     /* Nothing to do */
     return 0;
 }
 
-static uint32_t featurefile_writefn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t featurefile_writefn(CPUState *cs, GuestFD *gf,
                                     target_ulong buf, uint32_t len)
 {
     /* This fd can never be open for writing */
-    CPUARMState *env = &cpu->env;
 
     errno = EBADF;
-    return set_swi_errno(env, -1);
+    return set_swi_errno(cs, -1);
 }
 
-static uint32_t featurefile_readfn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t featurefile_readfn(CPUState *cs, GuestFD *gf,
                                    target_ulong buf, uint32_t len)
 {
+    CPUArchState *env = cs->env_ptr;
     uint32_t i;
-#ifndef CONFIG_USER_ONLY
-    CPUARMState *env = &cpu->env;
-#endif
     char *s;
 
+    (void) env; /* Used in arm softmmu lock_user implicitly */
     s = lock_user(VERIFY_WRITE, buf, len, 0);
     if (!s) {
         return len;
@@ -595,19 +623,19 @@ static uint32_t featurefile_readfn(ARMCPU *cpu, GuestFD *gf,
     return len - i;
 }
 
-static uint32_t featurefile_isattyfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t featurefile_isattyfn(CPUState *cs, GuestFD *gf)
 {
     return 0;
 }
 
-static uint32_t featurefile_seekfn(ARMCPU *cpu, GuestFD *gf,
+static uint32_t featurefile_seekfn(CPUState *cs, GuestFD *gf,
                                    target_ulong offset)
 {
     gf->featurefile_offset = offset;
     return 0;
 }
 
-static uint32_t featurefile_flenfn(ARMCPU *cpu, GuestFD *gf)
+static uint32_t featurefile_flenfn(CPUState *cs, GuestFD *gf)
 {
     return sizeof(featurefile_data);
 }
@@ -651,16 +679,17 @@ static const GuestFDFunctions guestfd_fns[] = {
 /* Read the input value from the argument block; fail the semihosting
  * call if the memory read fails.
  */
+#ifdef TARGET_ARM
 #define GET_ARG(n) do {                                 \
     if (is_a64(env)) {                                  \
         if (get_user_u64(arg ## n, args + (n) * 8)) {   \
             errno = EFAULT;                             \
-            return set_swi_errno(env, -1);              \
+            return set_swi_errno(cs, -1);              \
         }                                               \
     } else {                                            \
         if (get_user_u32(arg ## n, args + (n) * 4)) {   \
             errno = EFAULT;                             \
-            return set_swi_errno(env, -1);              \
+            return set_swi_errno(cs, -1);              \
         }                                               \
     }                                                   \
 } while (0)
@@ -669,6 +698,7 @@ static const GuestFDFunctions guestfd_fns[] = {
     (is_a64(env) ?                                      \
      put_user_u64(val, args + (n) * 8) :                \
      put_user_u32(val, args + (n) * 4))
+#endif
 
 /*
  * Do a semihosting call.
@@ -681,8 +711,7 @@ static const GuestFDFunctions guestfd_fns[] = {
  */
 target_ulong do_common_semihosting(CPUState *cs)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
+    CPUArchState *env = cs->env_ptr;
     target_ulong args;
     target_ulong arg0, arg1, arg2, arg3;
     char * s;
@@ -691,14 +720,9 @@ target_ulong do_common_semihosting(CPUState *cs)
     uint32_t len;
     GuestFD *gf;
 
-    if (is_a64(env)) {
-        /* Note that the syscall number is in W0, not X0 */
-        nr = env->xregs[0] & 0xffffffffU;
-        args = env->xregs[1];
-    } else {
-        nr = env->regs[0];
-        args = env->regs[1];
-    }
+    (void) env; /* Used implicitly by arm lock_user macro */
+    nr = common_semi_arg(cs, 0) & 0xffffffffU;
+    args = common_semi_arg(cs, 1);
 
     switch (nr) {
     case TARGET_SYS_OPEN:
@@ -711,19 +735,19 @@ target_ulong do_common_semihosting(CPUState *cs)
         s = lock_user_string(arg0);
         if (!s) {
             errno = EFAULT;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
         if (arg1 >= 12) {
             unlock_user(s, arg0, 0);
             errno = EINVAL;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
         guestfd = alloc_guestfd();
         if (guestfd < 0) {
             unlock_user(s, arg0, 0);
             errno = EMFILE;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
         if (strcmp(s, ":tt") == 0) {
@@ -752,18 +776,19 @@ target_ulong do_common_semihosting(CPUState *cs)
             if (arg1 != 0 && arg1 != 1) {
                 dealloc_guestfd(guestfd);
                 errno = EACCES;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
             init_featurefile_guestfd(guestfd);
             return guestfd;
         }
 
         if (use_gdb_syscalls()) {
-            arm_semi_open_guestfd = guestfd;
-            ret = arm_gdb_syscall(cpu, arm_semi_open_cb, "open,%s,%x,1a4", arg0,
-                                  (int)arg2 + 1, gdb_open_modeflags[arg1]);
+            common_semi_open_guestfd = guestfd;
+            ret = common_semi_gdb_syscall(cs, common_semi_open_cb,
+                                          "open,%s,%x,1a4", arg0, (int)arg2 + 1,
+                                          gdb_open_modeflags[arg1]);
         } else {
-            ret = set_swi_errno(env, open(s, open_modeflags[arg1], 0644));
+            ret = set_swi_errno(cs, open(s, open_modeflags[arg1], 0644));
             if (ret == (uint32_t)-1) {
                 dealloc_guestfd(guestfd);
             } else {
@@ -780,17 +805,17 @@ target_ulong do_common_semihosting(CPUState *cs)
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        ret = guestfd_fns[gf->type].closefn(cpu, gf);
+        ret = guestfd_fns[gf->type].closefn(cs, gf);
         dealloc_guestfd(arg0);
         return ret;
     case TARGET_SYS_WRITEC:
-        qemu_semihosting_console_outc(env, args);
+        qemu_semihosting_console_outc(cs->env_ptr, args);
         return 0xdeadbeef;
     case TARGET_SYS_WRITE0:
-        return qemu_semihosting_console_outs(env, args);
+        return qemu_semihosting_console_outs(cs->env_ptr, args);
     case TARGET_SYS_WRITE:
         GET_ARG(0);
         GET_ARG(1);
@@ -800,10 +825,10 @@ target_ulong do_common_semihosting(CPUState *cs)
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        return guestfd_fns[gf->type].writefn(cpu, gf, arg1, len);
+        return guestfd_fns[gf->type].writefn(cs, gf, arg1, len);
     case TARGET_SYS_READ:
         GET_ARG(0);
         GET_ARG(1);
@@ -813,22 +838,22 @@ target_ulong do_common_semihosting(CPUState *cs)
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        return guestfd_fns[gf->type].readfn(cpu, gf, arg1, len);
+        return guestfd_fns[gf->type].readfn(cs, gf, arg1, len);
     case TARGET_SYS_READC:
-        return qemu_semihosting_console_inc(env);
+        return qemu_semihosting_console_inc(cs->env_ptr);
     case TARGET_SYS_ISTTY:
         GET_ARG(0);
 
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        return guestfd_fns[gf->type].isattyfn(cpu, gf);
+        return guestfd_fns[gf->type].isattyfn(cs, gf);
     case TARGET_SYS_SEEK:
         GET_ARG(0);
         GET_ARG(1);
@@ -836,20 +861,20 @@ target_ulong do_common_semihosting(CPUState *cs)
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        return guestfd_fns[gf->type].seekfn(cpu, gf, arg1);
+        return guestfd_fns[gf->type].seekfn(cs, gf, arg1);
     case TARGET_SYS_FLEN:
         GET_ARG(0);
 
         gf = get_guestfd(arg0);
         if (!gf) {
             errno = EBADF;
-            return set_swi_errno(env, -1);
+            return set_swi_errno(cs, -1);
         }
 
-        return guestfd_fns[gf->type].flenfn(cpu, gf);
+        return guestfd_fns[gf->type].flenfn(cs, gf);
     case TARGET_SYS_TMPNAM:
         qemu_log_mask(LOG_UNIMP, "%s: SYS_TMPNAM not implemented", __func__);
         return -1;
@@ -857,15 +882,15 @@ target_ulong do_common_semihosting(CPUState *cs)
         GET_ARG(0);
         GET_ARG(1);
         if (use_gdb_syscalls()) {
-            ret = arm_gdb_syscall(cpu, arm_semi_cb, "unlink,%s",
-                                  arg0, (int)arg1 + 1);
+            ret = common_semi_gdb_syscall(cs, common_semi_cb, "unlink,%s",
+                                          arg0, (int)arg1 + 1);
         } else {
             s = lock_user_string(arg0);
             if (!s) {
                 errno = EFAULT;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
-            ret =  set_swi_errno(env, remove(s));
+            ret =  set_swi_errno(cs, remove(s));
             unlock_user(s, arg0, 0);
         }
         return ret;
@@ -875,17 +900,18 @@ target_ulong do_common_semihosting(CPUState *cs)
         GET_ARG(2);
         GET_ARG(3);
         if (use_gdb_syscalls()) {
-            return arm_gdb_syscall(cpu, arm_semi_cb, "rename,%s,%s",
-                                   arg0, (int)arg1 + 1, arg2, (int)arg3 + 1);
+            return common_semi_gdb_syscall(cs, common_semi_cb, "rename,%s,%s",
+                                           arg0, (int)arg1 + 1, arg2,
+                                           (int)arg3 + 1);
         } else {
             char *s2;
             s = lock_user_string(arg0);
             s2 = lock_user_string(arg2);
             if (!s || !s2) {
                 errno = EFAULT;
-                ret = set_swi_errno(env, -1);
+                ret = set_swi_errno(cs, -1);
             } else {
-                ret = set_swi_errno(env, rename(s, s2));
+                ret = set_swi_errno(cs, rename(s, s2));
             }
             if (s2)
                 unlock_user(s2, arg2, 0);
@@ -896,25 +922,25 @@ target_ulong do_common_semihosting(CPUState *cs)
     case TARGET_SYS_CLOCK:
         return clock() / (CLOCKS_PER_SEC / 100);
     case TARGET_SYS_TIME:
-        return set_swi_errno(env, time(NULL));
+        return set_swi_errno(cs, time(NULL));
     case TARGET_SYS_SYSTEM:
         GET_ARG(0);
         GET_ARG(1);
         if (use_gdb_syscalls()) {
-            return arm_gdb_syscall(cpu, arm_semi_cb, "system,%s",
-                                   arg0, (int)arg1 + 1);
+            return common_semi_gdb_syscall(cs, common_semi_cb, "system,%s",
+                                           arg0, (int)arg1 + 1);
         } else {
             s = lock_user_string(arg0);
             if (!s) {
                 errno = EFAULT;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
-            ret = set_swi_errno(env, system(s));
+            ret = set_swi_errno(cs, system(s));
             unlock_user(s, arg0, 0);
             return ret;
         }
     case TARGET_SYS_ERRNO:
-        return get_swi_errno(env);
+        return get_swi_errno(cs);
     case TARGET_SYS_GET_CMDLINE:
         {
             /* Build a command-line from the original argv.
@@ -966,21 +992,21 @@ target_ulong do_common_semihosting(CPUState *cs)
             if (output_size > input_size) {
                 /* Not enough space to store command-line arguments.  */
                 errno = E2BIG;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
 
             /* Adjust the command-line length.  */
             if (SET_ARG(1, output_size - 1)) {
                 /* Couldn't write back to argument block */
                 errno = EFAULT;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
 
             /* Lock the buffer on the ARM side.  */
             output_buffer = lock_user(VERIFY_WRITE, arg0, output_size, 0);
             if (!output_buffer) {
                 errno = EFAULT;
-                return set_swi_errno(env, -1);
+                return set_swi_errno(cs, -1);
             }
 
             /* Copy the command-line arguments.  */
@@ -996,7 +1022,7 @@ target_ulong do_common_semihosting(CPUState *cs)
             if (copy_from_user(output_buffer, ts->info->arg_start,
                                output_size)) {
                 errno = EFAULT;
-                status = set_swi_errno(env, -1);
+                status = set_swi_errno(cs, -1);
                 goto out;
             }
 
@@ -1021,8 +1047,7 @@ target_ulong do_common_semihosting(CPUState *cs)
 #ifdef CONFIG_USER_ONLY
             TaskState *ts = cs->opaque;
 #else
-            const struct arm_boot_info *info = env->boot_info;
-            target_ulong rambase = info->loader_start;
+            target_ulong rambase = common_semi_rambase(cs);
 #endif
 
             GET_ARG(0);
@@ -1036,7 +1061,7 @@ target_ulong do_common_semihosting(CPUState *cs)
                 abi_ulong ret;
 
                 ts->heap_base = do_brk(0);
-                limit = ts->heap_base + ARM_ANGEL_HEAP_SIZE;
+                limit = ts->heap_base + COMMON_SEMI_HEAP_SIZE;
                 /* Try a big heap, and reduce the size if that fails.  */
                 for (;;) {
                     ret = do_brk(limit);
@@ -1064,23 +1089,19 @@ target_ulong do_common_semihosting(CPUState *cs)
             for (i = 0; i < ARRAY_SIZE(retvals); i++) {
                 bool fail;
 
-                if (is_a64(env)) {
-                    fail = put_user_u64(retvals[i], arg0 + i * 8);
-                } else {
-                    fail = put_user_u32(retvals[i], arg0 + i * 4);
-                }
+                fail = SET_ARG(i, retvals[i]);
 
                 if (fail) {
                     /* Couldn't write back to argument block */
                     errno = EFAULT;
-                    return set_swi_errno(env, -1);
+                    return set_swi_errno(cs, -1);
                 }
             }
             return 0;
         }
     case TARGET_SYS_EXIT:
     case TARGET_SYS_EXIT_EXTENDED:
-        if (nr == TARGET_SYS_EXIT_EXTENDED || is_a64(env)) {
+        if (common_semi_sys_exit_extended(cs, nr)) {
             /*
              * The A64 version of SYS_EXIT takes a parameter block,
              * so the application-exit type can return a subcode which
@@ -1113,9 +1134,11 @@ target_ulong do_common_semihosting(CPUState *cs)
          * virtual address range. This is a nop for us since we don't
          * implement caches. This is only present on A64.
          */
-        if (is_a64(env)) {
+#ifdef TARGET_ARM
+        if (is_a64(cs->env_ptr)) {
             return 0;
         }
+#endif
         /* fall through -- invalid for A32/T32 */
     default:
         fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 15/20] semihosting: Support SYS_HEAPINFO when env->boot_info is not set
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (13 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 14/20] semihosting: Change internal common-semi interfaces to use CPUState * Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42   ` Alex Bennée
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Keith Packard, Alex Bennée

From: Keith Packard <keithp@keithp.com>

env->boot_info is only set in some ARM startup paths, so we cannot
rely on it to support the SYS_HEAPINFO semihosting function. When not
available, fallback to finding a RAM memory region containing the
current stack and use the base of that.

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-5-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/semihosting/common-semi.c | 43 +++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index ac1271545e..293791f721 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -137,6 +137,36 @@ typedef struct GuestFD {
 
 static GArray *guestfd_array;
 
+#ifndef CONFIG_USER_ONLY
+#include "exec/address-spaces.h"
+/*
+ * Find the base of a RAM region containing the specified address
+ */
+static inline hwaddr
+common_semi_find_region_base(hwaddr addr)
+{
+    MemoryRegion *subregion;
+
+    /*
+     * Find the chunk of R/W memory containing the address.  This is
+     * used for the SYS_HEAPINFO semihosting call, which should
+     * probably be using information from the loaded application.
+     */
+    QTAILQ_FOREACH(subregion, &get_system_memory()->subregions,
+                   subregions_link) {
+        if (subregion->ram && !subregion->readonly) {
+            Int128 top128 = int128_add(int128_make64(subregion->addr),
+                                       subregion->size);
+            Int128 addr128 = int128_make64(addr);
+            if (subregion->addr <= addr && int128_lt(addr128, top128)) {
+                return subregion->addr;
+            }
+        }
+    }
+    return 0;
+}
+#endif
+
 #ifdef TARGET_ARM
 static inline target_ulong
 common_semi_arg(CPUState *cs, int argno)
@@ -175,7 +205,18 @@ common_semi_rambase(CPUState *cs)
 {
     CPUArchState *env = cs->env_ptr;
     const struct arm_boot_info *info = env->boot_info;
-    return info->loader_start;
+    target_ulong sp;
+
+    if (info) {
+        return info->loader_start;
+    }
+
+    if (is_a64(env)) {
+        sp = env->xregs[31];
+    } else {
+        sp = env->regs[13];
+    }
+    return common_semi_find_region_base(sp);
 }
 #endif
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 16/20] riscv: Add semihosting support
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
@ 2021-01-08 22:42   ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 02/20] test/guest-debug: echo QEMU command as well Alex Bennée
                     ` (18 subsequent siblings)
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Keith Packard, open list:RISC-V TCG CPUs, Sagar Karandikar,
	Bastian Koppelmann, Laurent Vivier, Palmer Dabbelt,
	Alistair Francis, Alex Bennée

From: Keith Packard <keithp@keithp.com>

Adapt the arm semihosting support code for RISCV. This implementation
is based on the standard for RISC-V semihosting version 0.2 as
documented in

   https://github.com/riscv/riscv-semihosting-spec/releases/tag/0.2

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-6-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 default-configs/devices/riscv32-softmmu.mak   |  2 +
 default-configs/devices/riscv64-softmmu.mak   |  2 +
 .../targets/riscv32-linux-user.mak            |  1 +
 .../targets/riscv64-linux-user.mak            |  1 +
 hw/semihosting/common-semi.h                  |  5 +-
 linux-user/qemu.h                             |  4 +-
 target/riscv/cpu_bits.h                       |  1 +
 hw/semihosting/common-semi.c                  | 82 ++++++++++++++++++-
 linux-user/semihost.c                         |  8 +-
 target/riscv/cpu_helper.c                     | 10 +++
 target/riscv/translate.c                      | 11 +++
 .../riscv/insn_trans/trans_privileged.c.inc   | 37 ++++++++-
 qemu-options.hx                               | 10 ++-
 13 files changed, 162 insertions(+), 12 deletions(-)

diff --git a/default-configs/devices/riscv32-softmmu.mak b/default-configs/devices/riscv32-softmmu.mak
index 94a236c9c2..d847bd5692 100644
--- a/default-configs/devices/riscv32-softmmu.mak
+++ b/default-configs/devices/riscv32-softmmu.mak
@@ -3,6 +3,8 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
+CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/default-configs/devices/riscv64-softmmu.mak b/default-configs/devices/riscv64-softmmu.mak
index 76b6195648..d5eec75f05 100644
--- a/default-configs/devices/riscv64-softmmu.mak
+++ b/default-configs/devices/riscv64-softmmu.mak
@@ -3,6 +3,8 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
+CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/default-configs/targets/riscv32-linux-user.mak b/default-configs/targets/riscv32-linux-user.mak
index dfb259e8aa..6a9d1b1bc1 100644
--- a/default-configs/targets/riscv32-linux-user.mak
+++ b/default-configs/targets/riscv32-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=riscv32
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-csr.xml gdb-xml/riscv-32bit-virtual.xml
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/default-configs/targets/riscv64-linux-user.mak b/default-configs/targets/riscv64-linux-user.mak
index b13895f3b0..0a92849a1b 100644
--- a/default-configs/targets/riscv64-linux-user.mak
+++ b/default-configs/targets/riscv64-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=riscv64
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-csr.xml gdb-xml/riscv-64bit-virtual.xml
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
index bc53e92c79..0bfab1c669 100644
--- a/hw/semihosting/common-semi.h
+++ b/hw/semihosting/common-semi.h
@@ -1,6 +1,6 @@
 /*
  *  Semihosting support for systems modeled on the Arm "Angel"
- *  semihosting syscalls design.
+ *  semihosting syscalls design. This includes Arm and RISC-V processors
  *
  *  Copyright (c) 2005, 2007 CodeSourcery.
  *  Copyright (c) 2019 Linaro
@@ -26,6 +26,9 @@
  *     Semihosting for AArch32 and AArch64 Release 2.0
  *     https://static.docs.arm.com/100863/0200/semihosting.pdf
  *
+ *  RISC-V Semihosting is documented in:
+ *     RISC-V Semihosting
+ *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
  */
 
 #ifndef COMMON_SEMI_H
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 534753ca12..17aa992165 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -109,6 +109,8 @@ typedef struct TaskState {
     /* FPA state */
     FPA11 fpa;
 # endif
+#endif
+#if defined(TARGET_ARM) || defined(TARGET_RISCV)
     int swi_errno;
 #endif
 #if defined(TARGET_I386) && !defined(TARGET_X86_64)
@@ -122,7 +124,7 @@ typedef struct TaskState {
 #ifdef TARGET_M68K
     abi_ulong tp_value;
 #endif
-#if defined(TARGET_ARM) || defined(TARGET_M68K)
+#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
     /* Extra fields for semihosted binaries.  */
     abi_ulong heap_base;
     abi_ulong heap_limit;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index b41e8836c3..4196ef8b69 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -542,6 +542,7 @@
 #define RISCV_EXCP_INST_PAGE_FAULT               0xc /* since: priv-1.10.0 */
 #define RISCV_EXCP_LOAD_PAGE_FAULT               0xd /* since: priv-1.10.0 */
 #define RISCV_EXCP_STORE_PAGE_FAULT              0xf /* since: priv-1.10.0 */
+#define RISCV_EXCP_SEMIHOST                      0x10
 #define RISCV_EXCP_INST_GUEST_PAGE_FAULT         0x14
 #define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT       0x15
 #define RISCV_EXCP_VIRT_INSTRUCTION_FAULT        0x16
diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 293791f721..5fcb8663c6 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -1,6 +1,6 @@
 /*
  *  Semihosting support for systems modeled on the Arm "Angel"
- *  semihosting syscalls design.
+ *  semihosting syscalls design. This includes Arm and RISC-V processors
  *
  *  Copyright (c) 2005, 2007 CodeSourcery.
  *  Copyright (c) 2019 Linaro
@@ -25,6 +25,10 @@
  *  ARM Semihosting is documented in:
  *     Semihosting for AArch32 and AArch64 Release 2.0
  *     https://static.docs.arm.com/100863/0200/semihosting.pdf
+ *
+ *  RISC-V Semihosting is documented in:
+ *     RISC-V Semihosting
+ *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
  */
 
 #include "qemu/osdep.h"
@@ -222,6 +226,42 @@ common_semi_rambase(CPUState *cs)
 
 #endif /* TARGET_ARM */
 
+#ifdef TARGET_RISCV
+static inline target_ulong
+common_semi_arg(CPUState *cs, int argno)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    return env->gpr[xA0 + argno];
+}
+
+static inline void
+common_semi_set_ret(CPUState *cs, target_ulong ret)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    env->gpr[xA0] = ret;
+}
+
+static inline bool
+common_semi_sys_exit_extended(CPUState *cs, int nr)
+{
+    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
+}
+
+#ifndef CONFIG_USER_ONLY
+
+static inline target_ulong
+common_semi_rambase(CPUState *cs)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    return common_semi_find_region_base(env->gpr[xSP]);
+}
+#endif
+
+#endif
+
 /*
  * Allocate a new guest file descriptor and return it; if we
  * couldn't allocate a new fd then return -1.
@@ -398,6 +438,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
         sp = env->regs[13];
     }
 #endif
+#ifdef TARGET_RISCV
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+
+    sp = env->gpr[xSP];
+#endif
 
     return sp - 64;
 }
@@ -741,6 +787,37 @@ static const GuestFDFunctions guestfd_fns[] = {
      put_user_u32(val, args + (n) * 4))
 #endif
 
+#ifdef TARGET_RISCV
+
+/*
+ * get_user_ual is defined as get_user_u32 in softmmu-semi.h,
+ * we need a macro that fetches a target_ulong
+ */
+#define get_user_utl(arg, p)                    \
+    ((sizeof(target_ulong) == 8) ?              \
+     get_user_u64(arg, p) :                     \
+     get_user_u32(arg, p))
+
+/*
+ * put_user_ual is defined as put_user_u32 in softmmu-semi.h,
+ * we need a macro that stores a target_ulong
+ */
+#define put_user_utl(arg, p)                    \
+    ((sizeof(target_ulong) == 8) ?              \
+     put_user_u64(arg, p) :                     \
+     put_user_u32(arg, p))
+
+#define GET_ARG(n) do {                                                 \
+        if (get_user_utl(arg ## n, args + (n) * sizeof(target_ulong))) { \
+            errno = EFAULT;                                             \
+            return set_swi_errno(cs, -1);                              \
+        }                                                               \
+    } while (0)
+
+#define SET_ARG(n, val)                                 \
+    put_user_utl(val, args + (n) * sizeof(target_ulong))
+#endif
+
 /*
  * Do a semihosting call.
  *
@@ -1179,6 +1256,9 @@ target_ulong do_common_semihosting(CPUState *cs)
         if (is_a64(cs->env_ptr)) {
             return 0;
         }
+#endif
+#ifdef TARGET_RISCV
+        return 0;
 #endif
         /* fall through -- invalid for A32/T32 */
     default:
diff --git a/linux-user/semihost.c b/linux-user/semihost.c
index a1f0f6050e..c0015ee7f6 100644
--- a/linux-user/semihost.c
+++ b/linux-user/semihost.c
@@ -1,11 +1,11 @@
 /*
- * ARM Semihosting Console Support
+ * ARM Compatible Semihosting Console Support.
  *
  * Copyright (c) 2019 Linaro Ltd
  *
- * Currently ARM is unique in having support for semihosting support
- * in linux-user. So for now we implement the common console API but
- * just for arm linux-user.
+ * Currently ARM and RISC-V are unique in having support for
+ * semihosting support in linux-user. So for now we implement the
+ * common console API but just for arm and risc-v linux-user.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a2afb95fa1..f8350f5f78 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "tcg/tcg-op.h"
 #include "trace.h"
+#include "hw/semihosting/common-semi.h"
 
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 {
@@ -847,6 +848,15 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     target_ulong htval = 0;
     target_ulong mtval2 = 0;
 
+    if  (cause == RISCV_EXCP_SEMIHOST) {
+        if (env->priv >= PRV_S) {
+            env->gpr[xA0] = do_common_semihosting(cs);
+            env->pc += 4;
+            return;
+        }
+        cause = RISCV_EXCP_BREAKPOINT;
+    }
+
     if (!async) {
         /* set tval to badaddr for traps with address information */
         switch (cause) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 554d52a4be..0f28b5f41e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -64,6 +64,7 @@ typedef struct DisasContext {
     uint16_t vlen;
     uint16_t mlen;
     bool vl_eq_vlmax;
+    CPUState *cs;
 } DisasContext;
 
 #ifdef TARGET_RISCV64
@@ -747,6 +748,15 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
     return true;
 }
 
+static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
+{
+    DisasContext *ctx = container_of(dcbase, DisasContext, base);
+    CPUState *cpu = ctx->cs;
+    CPURISCVState *env = cpu->env_ptr;
+
+    return cpu_ldl_code(env, pc);
+}
+
 /* Include insn module translation function */
 #include "insn_trans/trans_rvi.c.inc"
 #include "insn_trans/trans_rvm.c.inc"
@@ -814,6 +824,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
     ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
     ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
+    ctx->cs = cs;
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 2a61a853bf..32312be202 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -29,7 +29,42 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 
 static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
 {
-    generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
+    target_ulong    ebreak_addr = ctx->base.pc_next;
+    target_ulong    pre_addr = ebreak_addr - 4;
+    target_ulong    post_addr = ebreak_addr + 4;
+    uint32_t pre    = 0;
+    uint32_t ebreak = 0;
+    uint32_t post   = 0;
+
+    /*
+     * The RISC-V semihosting spec specifies the following
+     * three-instruction sequence to flag a semihosting call:
+     *
+     *      slli zero, zero, 0x1f       0x01f01013
+     *      ebreak                      0x00100073
+     *      srai zero, zero, 0x7        0x40705013
+     *
+     * The two shift operations on the zero register are no-ops, used
+     * here to signify a semihosting exception, rather than a breakpoint.
+     *
+     * Uncompressed instructions are required so that the sequence is easy
+     * to validate.
+     *
+     * The three instructions are required to lie in the same page so
+     * that no exception will be raised when fetching them.
+     */
+
+    if ((pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
+        pre    = opcode_at(&ctx->base, pre_addr);
+        ebreak = opcode_at(&ctx->base, ebreak_addr);
+        post   = opcode_at(&ctx->base, post_addr);
+    }
+
+    if  (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
+        generate_exception(ctx, RISCV_EXCP_SEMIHOST);
+    } else {
+        generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
+    }
     exit_tb(ctx); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
diff --git a/qemu-options.hx b/qemu-options.hx
index 1698a0c751..263caa21ec 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4197,10 +4197,10 @@ ERST
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
     "-semihosting    semihosting mode\n",
     QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
+    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
 SRST
 ``-semihosting``
-    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
+    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4212,10 +4212,10 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
     "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
     "                semihosting configuration\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
 SRST
 ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
-    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II
+    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
     only).
 
     Note that this allows guest direct access to the host filesystem, so
@@ -4230,6 +4230,8 @@ SRST
     open/read/write/seek/select. Tensilica baremetal libc for ISS and
     linux platform "sim" use this interface.
 
+    On RISC-V this implements the standard semihosting API, version 0.2.
+
     ``target=native|gdb|auto``
         Defines where the semihosting calls will be addressed, to QEMU
         (``native``) or to GDB (``gdb``). The default is ``auto``, which
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 16/20] riscv: Add semihosting support
@ 2021-01-08 22:42   ` Alex Bennée
  0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Keith Packard, Alex Bennée, Laurent Vivier, Palmer Dabbelt,
	Alistair Francis, Sagar Karandikar, Bastian Koppelmann,
	open list:RISC-V TCG CPUs

From: Keith Packard <keithp@keithp.com>

Adapt the arm semihosting support code for RISCV. This implementation
is based on the standard for RISC-V semihosting version 0.2 as
documented in

   https://github.com/riscv/riscv-semihosting-spec/releases/tag/0.2

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-6-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 default-configs/devices/riscv32-softmmu.mak   |  2 +
 default-configs/devices/riscv64-softmmu.mak   |  2 +
 .../targets/riscv32-linux-user.mak            |  1 +
 .../targets/riscv64-linux-user.mak            |  1 +
 hw/semihosting/common-semi.h                  |  5 +-
 linux-user/qemu.h                             |  4 +-
 target/riscv/cpu_bits.h                       |  1 +
 hw/semihosting/common-semi.c                  | 82 ++++++++++++++++++-
 linux-user/semihost.c                         |  8 +-
 target/riscv/cpu_helper.c                     | 10 +++
 target/riscv/translate.c                      | 11 +++
 .../riscv/insn_trans/trans_privileged.c.inc   | 37 ++++++++-
 qemu-options.hx                               | 10 ++-
 13 files changed, 162 insertions(+), 12 deletions(-)

diff --git a/default-configs/devices/riscv32-softmmu.mak b/default-configs/devices/riscv32-softmmu.mak
index 94a236c9c2..d847bd5692 100644
--- a/default-configs/devices/riscv32-softmmu.mak
+++ b/default-configs/devices/riscv32-softmmu.mak
@@ -3,6 +3,8 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
+CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/default-configs/devices/riscv64-softmmu.mak b/default-configs/devices/riscv64-softmmu.mak
index 76b6195648..d5eec75f05 100644
--- a/default-configs/devices/riscv64-softmmu.mak
+++ b/default-configs/devices/riscv64-softmmu.mak
@@ -3,6 +3,8 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
+CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/default-configs/targets/riscv32-linux-user.mak b/default-configs/targets/riscv32-linux-user.mak
index dfb259e8aa..6a9d1b1bc1 100644
--- a/default-configs/targets/riscv32-linux-user.mak
+++ b/default-configs/targets/riscv32-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=riscv32
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-csr.xml gdb-xml/riscv-32bit-virtual.xml
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/default-configs/targets/riscv64-linux-user.mak b/default-configs/targets/riscv64-linux-user.mak
index b13895f3b0..0a92849a1b 100644
--- a/default-configs/targets/riscv64-linux-user.mak
+++ b/default-configs/targets/riscv64-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=riscv64
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-csr.xml gdb-xml/riscv-64bit-virtual.xml
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
index bc53e92c79..0bfab1c669 100644
--- a/hw/semihosting/common-semi.h
+++ b/hw/semihosting/common-semi.h
@@ -1,6 +1,6 @@
 /*
  *  Semihosting support for systems modeled on the Arm "Angel"
- *  semihosting syscalls design.
+ *  semihosting syscalls design. This includes Arm and RISC-V processors
  *
  *  Copyright (c) 2005, 2007 CodeSourcery.
  *  Copyright (c) 2019 Linaro
@@ -26,6 +26,9 @@
  *     Semihosting for AArch32 and AArch64 Release 2.0
  *     https://static.docs.arm.com/100863/0200/semihosting.pdf
  *
+ *  RISC-V Semihosting is documented in:
+ *     RISC-V Semihosting
+ *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
  */
 
 #ifndef COMMON_SEMI_H
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 534753ca12..17aa992165 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -109,6 +109,8 @@ typedef struct TaskState {
     /* FPA state */
     FPA11 fpa;
 # endif
+#endif
+#if defined(TARGET_ARM) || defined(TARGET_RISCV)
     int swi_errno;
 #endif
 #if defined(TARGET_I386) && !defined(TARGET_X86_64)
@@ -122,7 +124,7 @@ typedef struct TaskState {
 #ifdef TARGET_M68K
     abi_ulong tp_value;
 #endif
-#if defined(TARGET_ARM) || defined(TARGET_M68K)
+#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
     /* Extra fields for semihosted binaries.  */
     abi_ulong heap_base;
     abi_ulong heap_limit;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index b41e8836c3..4196ef8b69 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -542,6 +542,7 @@
 #define RISCV_EXCP_INST_PAGE_FAULT               0xc /* since: priv-1.10.0 */
 #define RISCV_EXCP_LOAD_PAGE_FAULT               0xd /* since: priv-1.10.0 */
 #define RISCV_EXCP_STORE_PAGE_FAULT              0xf /* since: priv-1.10.0 */
+#define RISCV_EXCP_SEMIHOST                      0x10
 #define RISCV_EXCP_INST_GUEST_PAGE_FAULT         0x14
 #define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT       0x15
 #define RISCV_EXCP_VIRT_INSTRUCTION_FAULT        0x16
diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 293791f721..5fcb8663c6 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -1,6 +1,6 @@
 /*
  *  Semihosting support for systems modeled on the Arm "Angel"
- *  semihosting syscalls design.
+ *  semihosting syscalls design. This includes Arm and RISC-V processors
  *
  *  Copyright (c) 2005, 2007 CodeSourcery.
  *  Copyright (c) 2019 Linaro
@@ -25,6 +25,10 @@
  *  ARM Semihosting is documented in:
  *     Semihosting for AArch32 and AArch64 Release 2.0
  *     https://static.docs.arm.com/100863/0200/semihosting.pdf
+ *
+ *  RISC-V Semihosting is documented in:
+ *     RISC-V Semihosting
+ *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
  */
 
 #include "qemu/osdep.h"
@@ -222,6 +226,42 @@ common_semi_rambase(CPUState *cs)
 
 #endif /* TARGET_ARM */
 
+#ifdef TARGET_RISCV
+static inline target_ulong
+common_semi_arg(CPUState *cs, int argno)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    return env->gpr[xA0 + argno];
+}
+
+static inline void
+common_semi_set_ret(CPUState *cs, target_ulong ret)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    env->gpr[xA0] = ret;
+}
+
+static inline bool
+common_semi_sys_exit_extended(CPUState *cs, int nr)
+{
+    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
+}
+
+#ifndef CONFIG_USER_ONLY
+
+static inline target_ulong
+common_semi_rambase(CPUState *cs)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    return common_semi_find_region_base(env->gpr[xSP]);
+}
+#endif
+
+#endif
+
 /*
  * Allocate a new guest file descriptor and return it; if we
  * couldn't allocate a new fd then return -1.
@@ -398,6 +438,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
         sp = env->regs[13];
     }
 #endif
+#ifdef TARGET_RISCV
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+
+    sp = env->gpr[xSP];
+#endif
 
     return sp - 64;
 }
@@ -741,6 +787,37 @@ static const GuestFDFunctions guestfd_fns[] = {
      put_user_u32(val, args + (n) * 4))
 #endif
 
+#ifdef TARGET_RISCV
+
+/*
+ * get_user_ual is defined as get_user_u32 in softmmu-semi.h,
+ * we need a macro that fetches a target_ulong
+ */
+#define get_user_utl(arg, p)                    \
+    ((sizeof(target_ulong) == 8) ?              \
+     get_user_u64(arg, p) :                     \
+     get_user_u32(arg, p))
+
+/*
+ * put_user_ual is defined as put_user_u32 in softmmu-semi.h,
+ * we need a macro that stores a target_ulong
+ */
+#define put_user_utl(arg, p)                    \
+    ((sizeof(target_ulong) == 8) ?              \
+     put_user_u64(arg, p) :                     \
+     put_user_u32(arg, p))
+
+#define GET_ARG(n) do {                                                 \
+        if (get_user_utl(arg ## n, args + (n) * sizeof(target_ulong))) { \
+            errno = EFAULT;                                             \
+            return set_swi_errno(cs, -1);                              \
+        }                                                               \
+    } while (0)
+
+#define SET_ARG(n, val)                                 \
+    put_user_utl(val, args + (n) * sizeof(target_ulong))
+#endif
+
 /*
  * Do a semihosting call.
  *
@@ -1179,6 +1256,9 @@ target_ulong do_common_semihosting(CPUState *cs)
         if (is_a64(cs->env_ptr)) {
             return 0;
         }
+#endif
+#ifdef TARGET_RISCV
+        return 0;
 #endif
         /* fall through -- invalid for A32/T32 */
     default:
diff --git a/linux-user/semihost.c b/linux-user/semihost.c
index a1f0f6050e..c0015ee7f6 100644
--- a/linux-user/semihost.c
+++ b/linux-user/semihost.c
@@ -1,11 +1,11 @@
 /*
- * ARM Semihosting Console Support
+ * ARM Compatible Semihosting Console Support.
  *
  * Copyright (c) 2019 Linaro Ltd
  *
- * Currently ARM is unique in having support for semihosting support
- * in linux-user. So for now we implement the common console API but
- * just for arm linux-user.
+ * Currently ARM and RISC-V are unique in having support for
+ * semihosting support in linux-user. So for now we implement the
+ * common console API but just for arm and risc-v linux-user.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a2afb95fa1..f8350f5f78 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "tcg/tcg-op.h"
 #include "trace.h"
+#include "hw/semihosting/common-semi.h"
 
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 {
@@ -847,6 +848,15 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     target_ulong htval = 0;
     target_ulong mtval2 = 0;
 
+    if  (cause == RISCV_EXCP_SEMIHOST) {
+        if (env->priv >= PRV_S) {
+            env->gpr[xA0] = do_common_semihosting(cs);
+            env->pc += 4;
+            return;
+        }
+        cause = RISCV_EXCP_BREAKPOINT;
+    }
+
     if (!async) {
         /* set tval to badaddr for traps with address information */
         switch (cause) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 554d52a4be..0f28b5f41e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -64,6 +64,7 @@ typedef struct DisasContext {
     uint16_t vlen;
     uint16_t mlen;
     bool vl_eq_vlmax;
+    CPUState *cs;
 } DisasContext;
 
 #ifdef TARGET_RISCV64
@@ -747,6 +748,15 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
     return true;
 }
 
+static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
+{
+    DisasContext *ctx = container_of(dcbase, DisasContext, base);
+    CPUState *cpu = ctx->cs;
+    CPURISCVState *env = cpu->env_ptr;
+
+    return cpu_ldl_code(env, pc);
+}
+
 /* Include insn module translation function */
 #include "insn_trans/trans_rvi.c.inc"
 #include "insn_trans/trans_rvm.c.inc"
@@ -814,6 +824,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
     ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
     ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
+    ctx->cs = cs;
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 2a61a853bf..32312be202 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -29,7 +29,42 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 
 static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
 {
-    generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
+    target_ulong    ebreak_addr = ctx->base.pc_next;
+    target_ulong    pre_addr = ebreak_addr - 4;
+    target_ulong    post_addr = ebreak_addr + 4;
+    uint32_t pre    = 0;
+    uint32_t ebreak = 0;
+    uint32_t post   = 0;
+
+    /*
+     * The RISC-V semihosting spec specifies the following
+     * three-instruction sequence to flag a semihosting call:
+     *
+     *      slli zero, zero, 0x1f       0x01f01013
+     *      ebreak                      0x00100073
+     *      srai zero, zero, 0x7        0x40705013
+     *
+     * The two shift operations on the zero register are no-ops, used
+     * here to signify a semihosting exception, rather than a breakpoint.
+     *
+     * Uncompressed instructions are required so that the sequence is easy
+     * to validate.
+     *
+     * The three instructions are required to lie in the same page so
+     * that no exception will be raised when fetching them.
+     */
+
+    if ((pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
+        pre    = opcode_at(&ctx->base, pre_addr);
+        ebreak = opcode_at(&ctx->base, ebreak_addr);
+        post   = opcode_at(&ctx->base, post_addr);
+    }
+
+    if  (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
+        generate_exception(ctx, RISCV_EXCP_SEMIHOST);
+    } else {
+        generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
+    }
     exit_tb(ctx); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
diff --git a/qemu-options.hx b/qemu-options.hx
index 1698a0c751..263caa21ec 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4197,10 +4197,10 @@ ERST
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
     "-semihosting    semihosting mode\n",
     QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
+    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
 SRST
 ``-semihosting``
-    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
+    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4212,10 +4212,10 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
     "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
     "                semihosting configuration\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
 SRST
 ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
-    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II
+    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
     only).
 
     Note that this allows guest direct access to the host filesystem, so
@@ -4230,6 +4230,8 @@ SRST
     open/read/write/seek/select. Tensilica baremetal libc for ISS and
     linux platform "sim" use this interface.
 
+    On RISC-V this implements the standard semihosting API, version 0.2.
+
     ``target=native|gdb|auto``
         Defines where the semihosting calls will be addressed, to QEMU
         (``native``) or to GDB (``gdb``). The default is ``auto``, which
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 17/20] riscv: Add semihosting support for user mode
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (15 preceding siblings ...)
  2021-01-08 22:42   ` Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 23:32   ` Alistair Francis
  2021-01-11 15:48   ` Philippe Mathieu-Daudé
  2021-01-08 22:42 ` [PATCH v1 18/20] semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ Alex Bennée
                   ` (2 subsequent siblings)
  19 siblings, 2 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kito Cheng, Alex Bennée, Keith Packard, Laurent Vivier

From: Kito Cheng <kito.cheng@sifive.com>

This could made testing more easier and ARM/AArch64 has supported on
their linux user mode too, so I think it should be reasonable.

Verified GCC testsuite with newlib/semihosting.

Signed-off-by: Kito Cheng <kito.cheng@sifive.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-7-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/riscv/cpu_loop.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
index aa9e437875..9665dabb09 100644
--- a/linux-user/riscv/cpu_loop.c
+++ b/linux-user/riscv/cpu_loop.c
@@ -23,6 +23,7 @@
 #include "qemu.h"
 #include "cpu_loop-common.h"
 #include "elf.h"
+#include "hw/semihosting/common-semi.h"
 
 void cpu_loop(CPURISCVState *env)
 {
@@ -91,6 +92,10 @@ void cpu_loop(CPURISCVState *env)
             sigcode = TARGET_SEGV_MAPERR;
             sigaddr = env->badaddr;
             break;
+        case RISCV_EXCP_SEMIHOST:
+            env->gpr[xA0] = do_common_semihosting(cs);
+            env->pc += 4;
+            break;
         case EXCP_DEBUG:
         gdbstep:
             signum = TARGET_SIGTRAP;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v1 18/20] semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (16 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 17/20] riscv: Add semihosting support for user mode Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 19/20] semihosting: Implement SYS_TMPNAM Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 20/20] semihosting: Implement SYS_ISERROR Alex Bennée
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Keith Packard, Alex Bennée

From: Keith Packard <keithp@keithp.com>

These are part of Semihosting for AArch32 and AArch64 Release 2.0

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-8-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/timer.h         |  2 ++
 hw/semihosting/common-semi.c | 16 ++++++++++++++++
 util/qemu-timer-common.c     |  4 ++++
 3 files changed, 22 insertions(+)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 61296ea980..1678238384 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -808,6 +808,8 @@ static inline int64_t get_clock_realtime(void)
     return tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000);
 }
 
+extern int64_t clock_start;
+
 /* Warning: don't insert tracepoints into these functions, they are
    also used by simpletrace backend and tracepoints would cause
    an infinite recursion! */
diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 5fcb8663c6..3d6604dcdd 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -38,6 +38,7 @@
 #include "hw/semihosting/console.h"
 #include "hw/semihosting/common-semi.h"
 #include "qemu/log.h"
+#include "qemu/timer.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 
@@ -73,6 +74,8 @@
 #define TARGET_SYS_EXIT        0x18
 #define TARGET_SYS_SYNCCACHE   0x19
 #define TARGET_SYS_EXIT_EXTENDED 0x20
+#define TARGET_SYS_ELAPSED     0x30
+#define TARGET_SYS_TICKFREQ    0x31
 
 /* ADP_Stopped_ApplicationExit is used for exit(0),
  * anything else is implemented as exit(1) */
@@ -837,6 +840,7 @@ target_ulong do_common_semihosting(CPUState *cs)
     uint32_t ret;
     uint32_t len;
     GuestFD *gf;
+    int64_t elapsed;
 
     (void) env; /* Used implicitly by arm lock_user macro */
     nr = common_semi_arg(cs, 0) & 0xffffffffU;
@@ -1246,6 +1250,18 @@ target_ulong do_common_semihosting(CPUState *cs)
         }
         gdb_exit(ret);
         exit(ret);
+    case TARGET_SYS_ELAPSED:
+        elapsed = get_clock() - clock_start;
+        if (sizeof(target_ulong) == 8) {
+            SET_ARG(0, elapsed);
+        } else {
+            SET_ARG(0, (uint32_t) elapsed);
+            SET_ARG(1, (uint32_t) (elapsed >> 32));
+        }
+        return 0;
+    case TARGET_SYS_TICKFREQ:
+        /* qemu always uses nsec */
+        return 1000000000;
     case TARGET_SYS_SYNCCACHE:
         /*
          * Clean the D-cache and invalidate the I-cache for the specified
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index baf3317f74..cc1326f726 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -27,6 +27,8 @@
 /***********************************************************/
 /* real time host monotonic timer */
 
+int64_t clock_start;
+
 #ifdef _WIN32
 
 int64_t clock_freq;
@@ -41,6 +43,7 @@ static void __attribute__((constructor)) init_get_clock(void)
         exit(1);
     }
     clock_freq = freq.QuadPart;
+    clock_start = get_clock();
 }
 
 #else
@@ -55,5 +58,6 @@ static void __attribute__((constructor)) init_get_clock(void)
     if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
         use_rt_clock = 1;
     }
+    clock_start = get_clock();
 }
 #endif
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 19/20] semihosting: Implement SYS_TMPNAM
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (17 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 18/20] semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  2021-01-08 22:42 ` [PATCH v1 20/20] semihosting: Implement SYS_ISERROR Alex Bennée
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Keith Packard, Alex Bennée

From: Keith Packard <keithp@keithp.com>

Part of Semihosting for AArch32 and AArch64 Release 2.0

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-9-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/semihosting/common-semi.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index 3d6604dcdd..a631904fb0 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -835,6 +835,7 @@ target_ulong do_common_semihosting(CPUState *cs)
     CPUArchState *env = cs->env_ptr;
     target_ulong args;
     target_ulong arg0, arg1, arg2, arg3;
+    target_ulong ul_ret;
     char * s;
     int nr;
     uint32_t ret;
@@ -998,8 +999,24 @@ target_ulong do_common_semihosting(CPUState *cs)
 
         return guestfd_fns[gf->type].flenfn(cs, gf);
     case TARGET_SYS_TMPNAM:
-        qemu_log_mask(LOG_UNIMP, "%s: SYS_TMPNAM not implemented", __func__);
-        return -1;
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
+        if (asprintf(&s, "/tmp/qemu-%x%02x", getpid(),
+                     (int) (arg1 & 0xff)) < 0) {
+            return -1;
+        }
+        ul_ret = (target_ulong) -1;
+
+        /* Make sure there's enough space in the buffer */
+        if (strlen(s) < arg2) {
+            char *output = lock_user(VERIFY_WRITE, arg0, arg2, 0);
+            strcpy(output, s);
+            unlock_user(output, arg0, arg2);
+            ul_ret = 0;
+        }
+        free(s);
+        return ul_ret;
     case TARGET_SYS_REMOVE:
         GET_ARG(0);
         GET_ARG(1);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH  v1 20/20] semihosting: Implement SYS_ISERROR
  2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
                   ` (18 preceding siblings ...)
  2021-01-08 22:42 ` [PATCH v1 19/20] semihosting: Implement SYS_TMPNAM Alex Bennée
@ 2021-01-08 22:42 ` Alex Bennée
  19 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2021-01-08 22:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Keith Packard, Alex Bennée

From: Keith Packard <keithp@keithp.com>

Part of Semihosting for AArch32 and AArch64 Release 2.0

Signed-off-by: Keith Packard <keithp@keithp.com>
Message-Id: <20210107170717.2098982-10-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/semihosting/common-semi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
index a631904fb0..23c6e3edcb 100644
--- a/hw/semihosting/common-semi.c
+++ b/hw/semihosting/common-semi.c
@@ -59,6 +59,7 @@
 #define TARGET_SYS_WRITE       0x05
 #define TARGET_SYS_READ        0x06
 #define TARGET_SYS_READC       0x07
+#define TARGET_SYS_ISERROR     0x08
 #define TARGET_SYS_ISTTY       0x09
 #define TARGET_SYS_SEEK        0x0a
 #define TARGET_SYS_FLEN        0x0c
@@ -967,6 +968,9 @@ target_ulong do_common_semihosting(CPUState *cs)
         return guestfd_fns[gf->type].readfn(cs, gf, arg1, len);
     case TARGET_SYS_READC:
         return qemu_semihosting_console_inc(cs->env_ptr);
+    case TARGET_SYS_ISERROR:
+        GET_ARG(0);
+        return (target_long) arg0 < 0 ? 1 : 0;
     case TARGET_SYS_ISTTY:
         GET_ARG(0);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 16/20] riscv: Add semihosting support
  2021-01-08 22:42   ` Alex Bennée
@ 2021-01-08 23:30     ` Alistair Francis
  -1 siblings, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2021-01-08 23:30 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Keith Packard, open list:RISC-V TCG CPUs, Sagar Karandikar,
	Bastian Koppelmann, qemu-devel@nongnu.org Developers,
	Laurent Vivier, Palmer Dabbelt, Alistair Francis

On Fri, Jan 8, 2021 at 3:06 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> From: Keith Packard <keithp@keithp.com>
>
> Adapt the arm semihosting support code for RISCV. This implementation
> is based on the standard for RISC-V semihosting version 0.2 as
> documented in
>
>    https://github.com/riscv/riscv-semihosting-spec/releases/tag/0.2
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Message-Id: <20210107170717.2098982-6-keithp@keithp.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Whoops, I thought I had already reviewed this commit.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  default-configs/devices/riscv32-softmmu.mak   |  2 +
>  default-configs/devices/riscv64-softmmu.mak   |  2 +
>  .../targets/riscv32-linux-user.mak            |  1 +
>  .../targets/riscv64-linux-user.mak            |  1 +
>  hw/semihosting/common-semi.h                  |  5 +-
>  linux-user/qemu.h                             |  4 +-
>  target/riscv/cpu_bits.h                       |  1 +
>  hw/semihosting/common-semi.c                  | 82 ++++++++++++++++++-
>  linux-user/semihost.c                         |  8 +-
>  target/riscv/cpu_helper.c                     | 10 +++
>  target/riscv/translate.c                      | 11 +++
>  .../riscv/insn_trans/trans_privileged.c.inc   | 37 ++++++++-
>  qemu-options.hx                               | 10 ++-
>  13 files changed, 162 insertions(+), 12 deletions(-)
>
> diff --git a/default-configs/devices/riscv32-softmmu.mak b/default-configs/devices/riscv32-softmmu.mak
> index 94a236c9c2..d847bd5692 100644
> --- a/default-configs/devices/riscv32-softmmu.mak
> +++ b/default-configs/devices/riscv32-softmmu.mak
> @@ -3,6 +3,8 @@
>  # Uncomment the following lines to disable these optional devices:
>  #
>  #CONFIG_PCI_DEVICES=n
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
>  # Boards:
>  #
> diff --git a/default-configs/devices/riscv64-softmmu.mak b/default-configs/devices/riscv64-softmmu.mak
> index 76b6195648..d5eec75f05 100644
> --- a/default-configs/devices/riscv64-softmmu.mak
> +++ b/default-configs/devices/riscv64-softmmu.mak
> @@ -3,6 +3,8 @@
>  # Uncomment the following lines to disable these optional devices:
>  #
>  #CONFIG_PCI_DEVICES=n
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
>  # Boards:
>  #
> diff --git a/default-configs/targets/riscv32-linux-user.mak b/default-configs/targets/riscv32-linux-user.mak
> index dfb259e8aa..6a9d1b1bc1 100644
> --- a/default-configs/targets/riscv32-linux-user.mak
> +++ b/default-configs/targets/riscv32-linux-user.mak
> @@ -2,3 +2,4 @@ TARGET_ARCH=riscv32
>  TARGET_BASE_ARCH=riscv
>  TARGET_ABI_DIR=riscv
>  TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-csr.xml gdb-xml/riscv-32bit-virtual.xml
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/default-configs/targets/riscv64-linux-user.mak b/default-configs/targets/riscv64-linux-user.mak
> index b13895f3b0..0a92849a1b 100644
> --- a/default-configs/targets/riscv64-linux-user.mak
> +++ b/default-configs/targets/riscv64-linux-user.mak
> @@ -2,3 +2,4 @@ TARGET_ARCH=riscv64
>  TARGET_BASE_ARCH=riscv
>  TARGET_ABI_DIR=riscv
>  TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-csr.xml gdb-xml/riscv-64bit-virtual.xml
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
> index bc53e92c79..0bfab1c669 100644
> --- a/hw/semihosting/common-semi.h
> +++ b/hw/semihosting/common-semi.h
> @@ -1,6 +1,6 @@
>  /*
>   *  Semihosting support for systems modeled on the Arm "Angel"
> - *  semihosting syscalls design.
> + *  semihosting syscalls design. This includes Arm and RISC-V processors
>   *
>   *  Copyright (c) 2005, 2007 CodeSourcery.
>   *  Copyright (c) 2019 Linaro
> @@ -26,6 +26,9 @@
>   *     Semihosting for AArch32 and AArch64 Release 2.0
>   *     https://static.docs.arm.com/100863/0200/semihosting.pdf
>   *
> + *  RISC-V Semihosting is documented in:
> + *     RISC-V Semihosting
> + *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
>   */
>
>  #ifndef COMMON_SEMI_H
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 534753ca12..17aa992165 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -109,6 +109,8 @@ typedef struct TaskState {
>      /* FPA state */
>      FPA11 fpa;
>  # endif
> +#endif
> +#if defined(TARGET_ARM) || defined(TARGET_RISCV)
>      int swi_errno;
>  #endif
>  #if defined(TARGET_I386) && !defined(TARGET_X86_64)
> @@ -122,7 +124,7 @@ typedef struct TaskState {
>  #ifdef TARGET_M68K
>      abi_ulong tp_value;
>  #endif
> -#if defined(TARGET_ARM) || defined(TARGET_M68K)
> +#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
>      /* Extra fields for semihosted binaries.  */
>      abi_ulong heap_base;
>      abi_ulong heap_limit;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index b41e8836c3..4196ef8b69 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -542,6 +542,7 @@
>  #define RISCV_EXCP_INST_PAGE_FAULT               0xc /* since: priv-1.10.0 */
>  #define RISCV_EXCP_LOAD_PAGE_FAULT               0xd /* since: priv-1.10.0 */
>  #define RISCV_EXCP_STORE_PAGE_FAULT              0xf /* since: priv-1.10.0 */
> +#define RISCV_EXCP_SEMIHOST                      0x10
>  #define RISCV_EXCP_INST_GUEST_PAGE_FAULT         0x14
>  #define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT       0x15
>  #define RISCV_EXCP_VIRT_INSTRUCTION_FAULT        0x16
> diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
> index 293791f721..5fcb8663c6 100644
> --- a/hw/semihosting/common-semi.c
> +++ b/hw/semihosting/common-semi.c
> @@ -1,6 +1,6 @@
>  /*
>   *  Semihosting support for systems modeled on the Arm "Angel"
> - *  semihosting syscalls design.
> + *  semihosting syscalls design. This includes Arm and RISC-V processors
>   *
>   *  Copyright (c) 2005, 2007 CodeSourcery.
>   *  Copyright (c) 2019 Linaro
> @@ -25,6 +25,10 @@
>   *  ARM Semihosting is documented in:
>   *     Semihosting for AArch32 and AArch64 Release 2.0
>   *     https://static.docs.arm.com/100863/0200/semihosting.pdf
> + *
> + *  RISC-V Semihosting is documented in:
> + *     RISC-V Semihosting
> + *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
>   */
>
>  #include "qemu/osdep.h"
> @@ -222,6 +226,42 @@ common_semi_rambase(CPUState *cs)
>
>  #endif /* TARGET_ARM */
>
> +#ifdef TARGET_RISCV
> +static inline target_ulong
> +common_semi_arg(CPUState *cs, int argno)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return env->gpr[xA0 + argno];
> +}
> +
> +static inline void
> +common_semi_set_ret(CPUState *cs, target_ulong ret)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    env->gpr[xA0] = ret;
> +}
> +
> +static inline bool
> +common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +
> +static inline target_ulong
> +common_semi_rambase(CPUState *cs)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return common_semi_find_region_base(env->gpr[xSP]);
> +}
> +#endif
> +
> +#endif
> +
>  /*
>   * Allocate a new guest file descriptor and return it; if we
>   * couldn't allocate a new fd then return -1.
> @@ -398,6 +438,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
>          sp = env->regs[13];
>      }
>  #endif
> +#ifdef TARGET_RISCV
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +
> +    sp = env->gpr[xSP];
> +#endif
>
>      return sp - 64;
>  }
> @@ -741,6 +787,37 @@ static const GuestFDFunctions guestfd_fns[] = {
>       put_user_u32(val, args + (n) * 4))
>  #endif
>
> +#ifdef TARGET_RISCV
> +
> +/*
> + * get_user_ual is defined as get_user_u32 in softmmu-semi.h,
> + * we need a macro that fetches a target_ulong
> + */
> +#define get_user_utl(arg, p)                    \
> +    ((sizeof(target_ulong) == 8) ?              \
> +     get_user_u64(arg, p) :                     \
> +     get_user_u32(arg, p))
> +
> +/*
> + * put_user_ual is defined as put_user_u32 in softmmu-semi.h,
> + * we need a macro that stores a target_ulong
> + */
> +#define put_user_utl(arg, p)                    \
> +    ((sizeof(target_ulong) == 8) ?              \
> +     put_user_u64(arg, p) :                     \
> +     put_user_u32(arg, p))
> +
> +#define GET_ARG(n) do {                                                 \
> +        if (get_user_utl(arg ## n, args + (n) * sizeof(target_ulong))) { \
> +            errno = EFAULT;                                             \
> +            return set_swi_errno(cs, -1);                              \
> +        }                                                               \
> +    } while (0)
> +
> +#define SET_ARG(n, val)                                 \
> +    put_user_utl(val, args + (n) * sizeof(target_ulong))
> +#endif
> +
>  /*
>   * Do a semihosting call.
>   *
> @@ -1179,6 +1256,9 @@ target_ulong do_common_semihosting(CPUState *cs)
>          if (is_a64(cs->env_ptr)) {
>              return 0;
>          }
> +#endif
> +#ifdef TARGET_RISCV
> +        return 0;
>  #endif
>          /* fall through -- invalid for A32/T32 */
>      default:
> diff --git a/linux-user/semihost.c b/linux-user/semihost.c
> index a1f0f6050e..c0015ee7f6 100644
> --- a/linux-user/semihost.c
> +++ b/linux-user/semihost.c
> @@ -1,11 +1,11 @@
>  /*
> - * ARM Semihosting Console Support
> + * ARM Compatible Semihosting Console Support.
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> - * Currently ARM is unique in having support for semihosting support
> - * in linux-user. So for now we implement the common console API but
> - * just for arm linux-user.
> + * Currently ARM and RISC-V are unique in having support for
> + * semihosting support in linux-user. So for now we implement the
> + * common console API but just for arm and risc-v linux-user.
>   *
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a2afb95fa1..f8350f5f78 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/exec-all.h"
>  #include "tcg/tcg-op.h"
>  #include "trace.h"
> +#include "hw/semihosting/common-semi.h"
>
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  {
> @@ -847,6 +848,15 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      target_ulong htval = 0;
>      target_ulong mtval2 = 0;
>
> +    if  (cause == RISCV_EXCP_SEMIHOST) {
> +        if (env->priv >= PRV_S) {
> +            env->gpr[xA0] = do_common_semihosting(cs);
> +            env->pc += 4;
> +            return;
> +        }
> +        cause = RISCV_EXCP_BREAKPOINT;
> +    }
> +
>      if (!async) {
>          /* set tval to badaddr for traps with address information */
>          switch (cause) {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 554d52a4be..0f28b5f41e 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -64,6 +64,7 @@ typedef struct DisasContext {
>      uint16_t vlen;
>      uint16_t mlen;
>      bool vl_eq_vlmax;
> +    CPUState *cs;
>  } DisasContext;
>
>  #ifdef TARGET_RISCV64
> @@ -747,6 +748,15 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
>      return true;
>  }
>
> +static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> +{
> +    DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +    CPUState *cpu = ctx->cs;
> +    CPURISCVState *env = cpu->env_ptr;
> +
> +    return cpu_ldl_code(env, pc);
> +}
> +
>  /* Include insn module translation function */
>  #include "insn_trans/trans_rvi.c.inc"
>  #include "insn_trans/trans_rvm.c.inc"
> @@ -814,6 +824,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>      ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
>      ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
>      ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
> +    ctx->cs = cs;
>  }
>
>  static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 2a61a853bf..32312be202 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -29,7 +29,42 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
>
>  static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
>  {
> -    generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
> +    target_ulong    ebreak_addr = ctx->base.pc_next;
> +    target_ulong    pre_addr = ebreak_addr - 4;
> +    target_ulong    post_addr = ebreak_addr + 4;
> +    uint32_t pre    = 0;
> +    uint32_t ebreak = 0;
> +    uint32_t post   = 0;
> +
> +    /*
> +     * The RISC-V semihosting spec specifies the following
> +     * three-instruction sequence to flag a semihosting call:
> +     *
> +     *      slli zero, zero, 0x1f       0x01f01013
> +     *      ebreak                      0x00100073
> +     *      srai zero, zero, 0x7        0x40705013
> +     *
> +     * The two shift operations on the zero register are no-ops, used
> +     * here to signify a semihosting exception, rather than a breakpoint.
> +     *
> +     * Uncompressed instructions are required so that the sequence is easy
> +     * to validate.
> +     *
> +     * The three instructions are required to lie in the same page so
> +     * that no exception will be raised when fetching them.
> +     */
> +
> +    if ((pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
> +        pre    = opcode_at(&ctx->base, pre_addr);
> +        ebreak = opcode_at(&ctx->base, ebreak_addr);
> +        post   = opcode_at(&ctx->base, post_addr);
> +    }
> +
> +    if  (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
> +        generate_exception(ctx, RISCV_EXCP_SEMIHOST);
> +    } else {
> +        generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
> +    }
>      exit_tb(ctx); /* no chaining */
>      ctx->base.is_jmp = DISAS_NORETURN;
>      return true;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1698a0c751..263caa21ec 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4197,10 +4197,10 @@ ERST
>  DEF("semihosting", 0, QEMU_OPTION_semihosting,
>      "-semihosting    semihosting mode\n",
>      QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
> -    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
> +    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
>  SRST
>  ``-semihosting``
> -    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
> +    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
>
>      Note that this allows guest direct access to the host filesystem, so
>      should only be used with a trusted guest OS.
> @@ -4212,10 +4212,10 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
>      "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
>      "                semihosting configuration\n",
>  QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
> -QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
> +QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
>  SRST
>  ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
> -    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II
> +    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
>      only).
>
>      Note that this allows guest direct access to the host filesystem, so
> @@ -4230,6 +4230,8 @@ SRST
>      open/read/write/seek/select. Tensilica baremetal libc for ISS and
>      linux platform "sim" use this interface.
>
> +    On RISC-V this implements the standard semihosting API, version 0.2.
> +
>      ``target=native|gdb|auto``
>          Defines where the semihosting calls will be addressed, to QEMU
>          (``native``) or to GDB (``gdb``). The default is ``auto``, which
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 16/20] riscv: Add semihosting support
@ 2021-01-08 23:30     ` Alistair Francis
  0 siblings, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2021-01-08 23:30 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel@nongnu.org Developers, Keith Packard,
	open list:RISC-V TCG CPUs, Sagar Karandikar, Bastian Koppelmann,
	Laurent Vivier, Palmer Dabbelt, Alistair Francis

On Fri, Jan 8, 2021 at 3:06 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> From: Keith Packard <keithp@keithp.com>
>
> Adapt the arm semihosting support code for RISCV. This implementation
> is based on the standard for RISC-V semihosting version 0.2 as
> documented in
>
>    https://github.com/riscv/riscv-semihosting-spec/releases/tag/0.2
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Message-Id: <20210107170717.2098982-6-keithp@keithp.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Whoops, I thought I had already reviewed this commit.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  default-configs/devices/riscv32-softmmu.mak   |  2 +
>  default-configs/devices/riscv64-softmmu.mak   |  2 +
>  .../targets/riscv32-linux-user.mak            |  1 +
>  .../targets/riscv64-linux-user.mak            |  1 +
>  hw/semihosting/common-semi.h                  |  5 +-
>  linux-user/qemu.h                             |  4 +-
>  target/riscv/cpu_bits.h                       |  1 +
>  hw/semihosting/common-semi.c                  | 82 ++++++++++++++++++-
>  linux-user/semihost.c                         |  8 +-
>  target/riscv/cpu_helper.c                     | 10 +++
>  target/riscv/translate.c                      | 11 +++
>  .../riscv/insn_trans/trans_privileged.c.inc   | 37 ++++++++-
>  qemu-options.hx                               | 10 ++-
>  13 files changed, 162 insertions(+), 12 deletions(-)
>
> diff --git a/default-configs/devices/riscv32-softmmu.mak b/default-configs/devices/riscv32-softmmu.mak
> index 94a236c9c2..d847bd5692 100644
> --- a/default-configs/devices/riscv32-softmmu.mak
> +++ b/default-configs/devices/riscv32-softmmu.mak
> @@ -3,6 +3,8 @@
>  # Uncomment the following lines to disable these optional devices:
>  #
>  #CONFIG_PCI_DEVICES=n
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
>  # Boards:
>  #
> diff --git a/default-configs/devices/riscv64-softmmu.mak b/default-configs/devices/riscv64-softmmu.mak
> index 76b6195648..d5eec75f05 100644
> --- a/default-configs/devices/riscv64-softmmu.mak
> +++ b/default-configs/devices/riscv64-softmmu.mak
> @@ -3,6 +3,8 @@
>  # Uncomment the following lines to disable these optional devices:
>  #
>  #CONFIG_PCI_DEVICES=n
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
>  # Boards:
>  #
> diff --git a/default-configs/targets/riscv32-linux-user.mak b/default-configs/targets/riscv32-linux-user.mak
> index dfb259e8aa..6a9d1b1bc1 100644
> --- a/default-configs/targets/riscv32-linux-user.mak
> +++ b/default-configs/targets/riscv32-linux-user.mak
> @@ -2,3 +2,4 @@ TARGET_ARCH=riscv32
>  TARGET_BASE_ARCH=riscv
>  TARGET_ABI_DIR=riscv
>  TARGET_XML_FILES= gdb-xml/riscv-32bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-32bit-csr.xml gdb-xml/riscv-32bit-virtual.xml
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/default-configs/targets/riscv64-linux-user.mak b/default-configs/targets/riscv64-linux-user.mak
> index b13895f3b0..0a92849a1b 100644
> --- a/default-configs/targets/riscv64-linux-user.mak
> +++ b/default-configs/targets/riscv64-linux-user.mak
> @@ -2,3 +2,4 @@ TARGET_ARCH=riscv64
>  TARGET_BASE_ARCH=riscv
>  TARGET_ABI_DIR=riscv
>  TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-csr.xml gdb-xml/riscv-64bit-virtual.xml
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
> index bc53e92c79..0bfab1c669 100644
> --- a/hw/semihosting/common-semi.h
> +++ b/hw/semihosting/common-semi.h
> @@ -1,6 +1,6 @@
>  /*
>   *  Semihosting support for systems modeled on the Arm "Angel"
> - *  semihosting syscalls design.
> + *  semihosting syscalls design. This includes Arm and RISC-V processors
>   *
>   *  Copyright (c) 2005, 2007 CodeSourcery.
>   *  Copyright (c) 2019 Linaro
> @@ -26,6 +26,9 @@
>   *     Semihosting for AArch32 and AArch64 Release 2.0
>   *     https://static.docs.arm.com/100863/0200/semihosting.pdf
>   *
> + *  RISC-V Semihosting is documented in:
> + *     RISC-V Semihosting
> + *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
>   */
>
>  #ifndef COMMON_SEMI_H
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 534753ca12..17aa992165 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -109,6 +109,8 @@ typedef struct TaskState {
>      /* FPA state */
>      FPA11 fpa;
>  # endif
> +#endif
> +#if defined(TARGET_ARM) || defined(TARGET_RISCV)
>      int swi_errno;
>  #endif
>  #if defined(TARGET_I386) && !defined(TARGET_X86_64)
> @@ -122,7 +124,7 @@ typedef struct TaskState {
>  #ifdef TARGET_M68K
>      abi_ulong tp_value;
>  #endif
> -#if defined(TARGET_ARM) || defined(TARGET_M68K)
> +#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
>      /* Extra fields for semihosted binaries.  */
>      abi_ulong heap_base;
>      abi_ulong heap_limit;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index b41e8836c3..4196ef8b69 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -542,6 +542,7 @@
>  #define RISCV_EXCP_INST_PAGE_FAULT               0xc /* since: priv-1.10.0 */
>  #define RISCV_EXCP_LOAD_PAGE_FAULT               0xd /* since: priv-1.10.0 */
>  #define RISCV_EXCP_STORE_PAGE_FAULT              0xf /* since: priv-1.10.0 */
> +#define RISCV_EXCP_SEMIHOST                      0x10
>  #define RISCV_EXCP_INST_GUEST_PAGE_FAULT         0x14
>  #define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT       0x15
>  #define RISCV_EXCP_VIRT_INSTRUCTION_FAULT        0x16
> diff --git a/hw/semihosting/common-semi.c b/hw/semihosting/common-semi.c
> index 293791f721..5fcb8663c6 100644
> --- a/hw/semihosting/common-semi.c
> +++ b/hw/semihosting/common-semi.c
> @@ -1,6 +1,6 @@
>  /*
>   *  Semihosting support for systems modeled on the Arm "Angel"
> - *  semihosting syscalls design.
> + *  semihosting syscalls design. This includes Arm and RISC-V processors
>   *
>   *  Copyright (c) 2005, 2007 CodeSourcery.
>   *  Copyright (c) 2019 Linaro
> @@ -25,6 +25,10 @@
>   *  ARM Semihosting is documented in:
>   *     Semihosting for AArch32 and AArch64 Release 2.0
>   *     https://static.docs.arm.com/100863/0200/semihosting.pdf
> + *
> + *  RISC-V Semihosting is documented in:
> + *     RISC-V Semihosting
> + *     https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
>   */
>
>  #include "qemu/osdep.h"
> @@ -222,6 +226,42 @@ common_semi_rambase(CPUState *cs)
>
>  #endif /* TARGET_ARM */
>
> +#ifdef TARGET_RISCV
> +static inline target_ulong
> +common_semi_arg(CPUState *cs, int argno)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return env->gpr[xA0 + argno];
> +}
> +
> +static inline void
> +common_semi_set_ret(CPUState *cs, target_ulong ret)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    env->gpr[xA0] = ret;
> +}
> +
> +static inline bool
> +common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +
> +static inline target_ulong
> +common_semi_rambase(CPUState *cs)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +    return common_semi_find_region_base(env->gpr[xSP]);
> +}
> +#endif
> +
> +#endif
> +
>  /*
>   * Allocate a new guest file descriptor and return it; if we
>   * couldn't allocate a new fd then return -1.
> @@ -398,6 +438,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
>          sp = env->regs[13];
>      }
>  #endif
> +#ifdef TARGET_RISCV
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +
> +    sp = env->gpr[xSP];
> +#endif
>
>      return sp - 64;
>  }
> @@ -741,6 +787,37 @@ static const GuestFDFunctions guestfd_fns[] = {
>       put_user_u32(val, args + (n) * 4))
>  #endif
>
> +#ifdef TARGET_RISCV
> +
> +/*
> + * get_user_ual is defined as get_user_u32 in softmmu-semi.h,
> + * we need a macro that fetches a target_ulong
> + */
> +#define get_user_utl(arg, p)                    \
> +    ((sizeof(target_ulong) == 8) ?              \
> +     get_user_u64(arg, p) :                     \
> +     get_user_u32(arg, p))
> +
> +/*
> + * put_user_ual is defined as put_user_u32 in softmmu-semi.h,
> + * we need a macro that stores a target_ulong
> + */
> +#define put_user_utl(arg, p)                    \
> +    ((sizeof(target_ulong) == 8) ?              \
> +     put_user_u64(arg, p) :                     \
> +     put_user_u32(arg, p))
> +
> +#define GET_ARG(n) do {                                                 \
> +        if (get_user_utl(arg ## n, args + (n) * sizeof(target_ulong))) { \
> +            errno = EFAULT;                                             \
> +            return set_swi_errno(cs, -1);                              \
> +        }                                                               \
> +    } while (0)
> +
> +#define SET_ARG(n, val)                                 \
> +    put_user_utl(val, args + (n) * sizeof(target_ulong))
> +#endif
> +
>  /*
>   * Do a semihosting call.
>   *
> @@ -1179,6 +1256,9 @@ target_ulong do_common_semihosting(CPUState *cs)
>          if (is_a64(cs->env_ptr)) {
>              return 0;
>          }
> +#endif
> +#ifdef TARGET_RISCV
> +        return 0;
>  #endif
>          /* fall through -- invalid for A32/T32 */
>      default:
> diff --git a/linux-user/semihost.c b/linux-user/semihost.c
> index a1f0f6050e..c0015ee7f6 100644
> --- a/linux-user/semihost.c
> +++ b/linux-user/semihost.c
> @@ -1,11 +1,11 @@
>  /*
> - * ARM Semihosting Console Support
> + * ARM Compatible Semihosting Console Support.
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> - * Currently ARM is unique in having support for semihosting support
> - * in linux-user. So for now we implement the common console API but
> - * just for arm linux-user.
> + * Currently ARM and RISC-V are unique in having support for
> + * semihosting support in linux-user. So for now we implement the
> + * common console API but just for arm and risc-v linux-user.
>   *
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a2afb95fa1..f8350f5f78 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/exec-all.h"
>  #include "tcg/tcg-op.h"
>  #include "trace.h"
> +#include "hw/semihosting/common-semi.h"
>
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  {
> @@ -847,6 +848,15 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      target_ulong htval = 0;
>      target_ulong mtval2 = 0;
>
> +    if  (cause == RISCV_EXCP_SEMIHOST) {
> +        if (env->priv >= PRV_S) {
> +            env->gpr[xA0] = do_common_semihosting(cs);
> +            env->pc += 4;
> +            return;
> +        }
> +        cause = RISCV_EXCP_BREAKPOINT;
> +    }
> +
>      if (!async) {
>          /* set tval to badaddr for traps with address information */
>          switch (cause) {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 554d52a4be..0f28b5f41e 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -64,6 +64,7 @@ typedef struct DisasContext {
>      uint16_t vlen;
>      uint16_t mlen;
>      bool vl_eq_vlmax;
> +    CPUState *cs;
>  } DisasContext;
>
>  #ifdef TARGET_RISCV64
> @@ -747,6 +748,15 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
>      return true;
>  }
>
> +static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> +{
> +    DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +    CPUState *cpu = ctx->cs;
> +    CPURISCVState *env = cpu->env_ptr;
> +
> +    return cpu_ldl_code(env, pc);
> +}
> +
>  /* Include insn module translation function */
>  #include "insn_trans/trans_rvi.c.inc"
>  #include "insn_trans/trans_rvm.c.inc"
> @@ -814,6 +824,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>      ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
>      ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
>      ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
> +    ctx->cs = cs;
>  }
>
>  static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 2a61a853bf..32312be202 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -29,7 +29,42 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
>
>  static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
>  {
> -    generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
> +    target_ulong    ebreak_addr = ctx->base.pc_next;
> +    target_ulong    pre_addr = ebreak_addr - 4;
> +    target_ulong    post_addr = ebreak_addr + 4;
> +    uint32_t pre    = 0;
> +    uint32_t ebreak = 0;
> +    uint32_t post   = 0;
> +
> +    /*
> +     * The RISC-V semihosting spec specifies the following
> +     * three-instruction sequence to flag a semihosting call:
> +     *
> +     *      slli zero, zero, 0x1f       0x01f01013
> +     *      ebreak                      0x00100073
> +     *      srai zero, zero, 0x7        0x40705013
> +     *
> +     * The two shift operations on the zero register are no-ops, used
> +     * here to signify a semihosting exception, rather than a breakpoint.
> +     *
> +     * Uncompressed instructions are required so that the sequence is easy
> +     * to validate.
> +     *
> +     * The three instructions are required to lie in the same page so
> +     * that no exception will be raised when fetching them.
> +     */
> +
> +    if ((pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
> +        pre    = opcode_at(&ctx->base, pre_addr);
> +        ebreak = opcode_at(&ctx->base, ebreak_addr);
> +        post   = opcode_at(&ctx->base, post_addr);
> +    }
> +
> +    if  (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
> +        generate_exception(ctx, RISCV_EXCP_SEMIHOST);
> +    } else {
> +        generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
> +    }
>      exit_tb(ctx); /* no chaining */
>      ctx->base.is_jmp = DISAS_NORETURN;
>      return true;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1698a0c751..263caa21ec 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4197,10 +4197,10 @@ ERST
>  DEF("semihosting", 0, QEMU_OPTION_semihosting,
>      "-semihosting    semihosting mode\n",
>      QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
> -    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
> +    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
>  SRST
>  ``-semihosting``
> -    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
> +    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
>
>      Note that this allows guest direct access to the host filesystem, so
>      should only be used with a trusted guest OS.
> @@ -4212,10 +4212,10 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
>      "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
>      "                semihosting configuration\n",
>  QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
> -QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
> +QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
>  SRST
>  ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
> -    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II
> +    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
>      only).
>
>      Note that this allows guest direct access to the host filesystem, so
> @@ -4230,6 +4230,8 @@ SRST
>      open/read/write/seek/select. Tensilica baremetal libc for ISS and
>      linux platform "sim" use this interface.
>
> +    On RISC-V this implements the standard semihosting API, version 0.2.
> +
>      ``target=native|gdb|auto``
>          Defines where the semihosting calls will be addressed, to QEMU
>          (``native``) or to GDB (``gdb``). The default is ``auto``, which
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 17/20] riscv: Add semihosting support for user mode
  2021-01-08 22:42 ` [PATCH v1 17/20] riscv: Add semihosting support for user mode Alex Bennée
@ 2021-01-08 23:32   ` Alistair Francis
  2021-01-11 15:48   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2021-01-08 23:32 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Keith Packard, Kito Cheng, qemu-devel@nongnu.org Developers,
	Laurent Vivier

On Fri, Jan 8, 2021 at 3:05 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> From: Kito Cheng <kito.cheng@sifive.com>
>
> This could made testing more easier and ARM/AArch64 has supported on
> their linux user mode too, so I think it should be reasonable.
>
> Verified GCC testsuite with newlib/semihosting.
>
> Signed-off-by: Kito Cheng <kito.cheng@sifive.com>
> Reviewed-by: Keith Packard <keithp@keithp.com>
> Message-Id: <20210107170717.2098982-7-keithp@keithp.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  linux-user/riscv/cpu_loop.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
> index aa9e437875..9665dabb09 100644
> --- a/linux-user/riscv/cpu_loop.c
> +++ b/linux-user/riscv/cpu_loop.c
> @@ -23,6 +23,7 @@
>  #include "qemu.h"
>  #include "cpu_loop-common.h"
>  #include "elf.h"
> +#include "hw/semihosting/common-semi.h"
>
>  void cpu_loop(CPURISCVState *env)
>  {
> @@ -91,6 +92,10 @@ void cpu_loop(CPURISCVState *env)
>              sigcode = TARGET_SEGV_MAPERR;
>              sigaddr = env->badaddr;
>              break;
> +        case RISCV_EXCP_SEMIHOST:
> +            env->gpr[xA0] = do_common_semihosting(cs);
> +            env->pc += 4;
> +            break;
>          case EXCP_DEBUG:
>          gdbstep:
>              signum = TARGET_SIGTRAP;
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 16/20] riscv: Add semihosting support
  2021-01-08 23:30     ` Alistair Francis
@ 2021-01-09  0:44       ` Keith Packard
  -1 siblings, 0 replies; 36+ messages in thread
From: Keith Packard via @ 2021-01-09  0:44 UTC (permalink / raw)
  To: Alistair Francis, Alex Bennée
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Laurent Vivier, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

Alistair Francis <alistair23@gmail.com> writes:

> Whoops, I thought I had already reviewed this commit.

You had provided quite extensive review with lots of useful comments,
but never added the magic tag for this commit :-)

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 16/20] riscv: Add semihosting support
@ 2021-01-09  0:44       ` Keith Packard
  0 siblings, 0 replies; 36+ messages in thread
From: Keith Packard @ 2021-01-09  0:44 UTC (permalink / raw)
  To: Alistair Francis, Alex Bennée
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V TCG CPUs,
	Sagar Karandikar, Bastian Koppelmann, Laurent Vivier,
	Palmer Dabbelt, Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

Alistair Francis <alistair23@gmail.com> writes:

> Whoops, I thought I had already reviewed this commit.

You had provided quite extensive review with lots of useful comments,
but never added the magic tag for this commit :-)

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers
  2021-01-08 22:42 ` [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers Alex Bennée
@ 2021-01-11 13:20   ` Luis Machado
  2021-01-11 14:36     ` Alex Bennée
  0 siblings, 1 reply; 36+ messages in thread
From: Luis Machado @ 2021-01-11 13:20 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs

For the record, the layout looks OK to me.

Just a reminder that GDB will soon support bfloat16 types. A patch may 
be pushed this month.

On 1/8/21 7:42 PM, Alex Bennée wrote:
> While GDB can work with any XML description given to it there is
> special handling for SVE registers on the GDB side which makes the
> users life a little better. The changes aren't that major and all the
> registers save the $vg reported the same. All that changes is:
> 
>    - report org.gnu.gdb.aarch64.sve
>    - use gdb nomenclature for names and types
>    - minor re-ordering of the types to match reference
>    - re-enable ieee_half (as we know gdb supports it now)
>    - $vg is now a 64 bit int
>    - check $vN and $zN aliasing in test
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Luis Machado <luis.machado@linaro.org>
> Message-Id: <20201218112707.28348-10-alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/arm/gdbstub.c                        | 75 ++++++++-------------
>   target/arm/helper.c                         |  2 +-
>   tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 11 +++
>   3 files changed, 41 insertions(+), 47 deletions(-)
> 
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 866595b4f1..a8fff2a3d0 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>       { "uint128", 128, 'q', 'u' },
>       { "int128", 128, 'q', 's' },
>       /* 64 bit */
> +    { "ieee_double", 64, 'd', 'f' },
>       { "uint64", 64, 'd', 'u' },
>       { "int64", 64, 'd', 's' },
> -    { "ieee_double", 64, 'd', 'f' },
>       /* 32 bit */
> +    { "ieee_single", 32, 's', 'f' },
>       { "uint32", 32, 's', 'u' },
>       { "int32", 32, 's', 's' },
> -    { "ieee_single", 32, 's', 'f' },
>       /* 16 bit */
> +    { "ieee_half", 16, 'h', 'f' },
>       { "uint16", 16, 'h', 'u' },
>       { "int16", 16, 'h', 's' },
> -    /*
> -     * TODO: currently there is no reliable way of telling
> -     * if the remote gdb actually understands ieee_half so
> -     * we don't expose it in the target description for now.
> -     * { "ieee_half", 16, 'h', 'f' },
> -     */
>       /* bytes */
>       { "uint8", 8, 'b', 'u' },
>       { "int8", 8, 'b', 's' },
> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>       GString *s = g_string_new(NULL);
>       DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>       g_autoptr(GString) ts = g_string_new("");
> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>       info->num = 0;
>       g_string_printf(s, "<?xml version=\"1.0\"?>");
>       g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> -    g_string_append_printf(s, "<feature name=\"org.qemu.gdb.aarch64.sve\">");
> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>   
>       /* First define types and totals in a whole VL */
>       for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>           int count = reg_width / vec_lanes[i].size;
> -        g_string_printf(ts, "vq%d%c%c", count,
> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>           g_string_append_printf(s,
>                                  "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>                                  ts->str, vec_lanes[i].gdb_type, count);
> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>        * signed and potentially float versions of each size from 128 to
>        * 8 bits.
>        */
> -    for (bits = 128; bits >= 8; bits /= 2) {
> -        int count = reg_width / bits;
> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> -            if (vec_lanes[i].size == bits) {
> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
> -                                       vec_lanes[i].suffix,
> -                                       count,
> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
> +            if (vec_lanes[j].size == bits) {
> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
> +                                       vec_lanes[j].suffix,
> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>               }
>           }
>           g_string_append(s, "</union>");
>       }
>       /* And now the final union of unions */
> -    g_string_append(s, "<union id=\"vq\">");
> -    for (bits = 128; bits >= 8; bits /= 2) {
> -        int count = reg_width / bits;
> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> -            if (vec_lanes[i].size == bits) {
> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
> -                                       vec_lanes[i].sz, count);
> -                break;
> -            }
> -        }
> +    g_string_append(s, "<union id=\"svev\">");
> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
> +                               suf[i], suf[i]);
>       }
>       g_string_append(s, "</union>");
>   
> +    /* Finally the sve prefix type */
> +    g_string_append_printf(s,
> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
> +                           reg_width / 8);
> +
>       /* Then define each register in parts for each vq */
>       for (i = 0; i < 32; i++) {
>           g_string_append_printf(s,
>                                  "<reg name=\"z%d\" bitsize=\"%d\""
> -                               " regnum=\"%d\" group=\"vector\""
> -                               " type=\"vq\"/>",
> +                               " regnum=\"%d\" type=\"svev\"/>",
>                                  i, reg_width, base_reg++);
>           info->num++;
>       }
> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>                              " regnum=\"%d\" group=\"float\""
>                              " type=\"int\"/>", base_reg++);
>       info->num += 2;
> -    /*
> -     * Predicate registers aren't so big they are worth splitting up
> -     * but we do need to define a type to hold the array of quad
> -     * references.
> -     */
> -    g_string_append_printf(s,
> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
> -                           cpu->sve_max_vq);
> +
>       for (i = 0; i < 16; i++) {
>           g_string_append_printf(s,
>                                  "<reg name=\"p%d\" bitsize=\"%d\""
> -                               " regnum=\"%d\" group=\"vector\""
> -                               " type=\"vqp\"/>",
> +                               " regnum=\"%d\" type=\"svep\"/>",
>                                  i, cpu->sve_max_vq * 16, base_reg++);
>           info->num++;
>       }
>       g_string_append_printf(s,
>                              "<reg name=\"ffr\" bitsize=\"%d\""
>                              " regnum=\"%d\" group=\"vector\""
> -                           " type=\"vqp\"/>",
> +                           " type=\"svep\"/>",
>                              cpu->sve_max_vq * 16, base_reg++);
>       g_string_append_printf(s,
>                              "<reg name=\"vg\" bitsize=\"64\""
> -                           " regnum=\"%d\" group=\"vector\""
> -                           " type=\"uint32\"/>",
> +                           " regnum=\"%d\" type=\"int\"/>",
>                              base_reg++);
>       info->num += 2;
>       g_string_append_printf(s, "</feature>");
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d077dd9ef5..d434044f07 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>            * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>            */
>           int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
> -        return gdb_get_reg32(buf, vq * 2);
> +        return gdb_get_reg64(buf, vq * 2);
>       }
>       default:
>           /* gdbstub asked for something out our range */
> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> index 972cf73c31..b9ef169c1a 100644
> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>           except gdb.error:
>               report(False, "checking zregs (out of range)")
>   
> +        # Check the aliased V registers are set and GDB has correctly
> +        # created them for us having recognised and handled SVE.
> +        try:
> +            for i in range(0, 16):
> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
> +                report(int(val_z) == int(val_v),
> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
> +        except gdb.error:
> +            report(False, "checking vregs (out of range)")
> +
>   
>   def run_test():
>       "Run through the tests one by one"
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers
  2021-01-11 13:20   ` Luis Machado
@ 2021-01-11 14:36     ` Alex Bennée
  2021-01-11 14:50       ` Luis Machado
  0 siblings, 1 reply; 36+ messages in thread
From: Alex Bennée @ 2021-01-11 14:36 UTC (permalink / raw)
  To: Luis Machado; +Cc: Peter Maydell, open list:ARM TCG CPUs, qemu-devel


Luis Machado <luis.machado@linaro.org> writes:

> For the record, the layout looks OK to me.

So a Reviewed-by?

> Just a reminder that GDB will soon support bfloat16 types. A patch may 
> be pushed this month.

Will we be able to probe for the support - or will an older GDB silently
accept and drop any bfloat16 fields?

>
> On 1/8/21 7:42 PM, Alex Bennée wrote:
>> While GDB can work with any XML description given to it there is
>> special handling for SVE registers on the GDB side which makes the
>> users life a little better. The changes aren't that major and all the
>> registers save the $vg reported the same. All that changes is:
>> 
>>    - report org.gnu.gdb.aarch64.sve
>>    - use gdb nomenclature for names and types
>>    - minor re-ordering of the types to match reference
>>    - re-enable ieee_half (as we know gdb supports it now)
>>    - $vg is now a 64 bit int
>>    - check $vN and $zN aliasing in test
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Luis Machado <luis.machado@linaro.org>
>> Message-Id: <20201218112707.28348-10-alex.bennee@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   target/arm/gdbstub.c                        | 75 ++++++++-------------
>>   target/arm/helper.c                         |  2 +-
>>   tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 11 +++
>>   3 files changed, 41 insertions(+), 47 deletions(-)
>> 
>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>> index 866595b4f1..a8fff2a3d0 100644
>> --- a/target/arm/gdbstub.c
>> +++ b/target/arm/gdbstub.c
>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>       { "uint128", 128, 'q', 'u' },
>>       { "int128", 128, 'q', 's' },
>>       /* 64 bit */
>> +    { "ieee_double", 64, 'd', 'f' },
>>       { "uint64", 64, 'd', 'u' },
>>       { "int64", 64, 'd', 's' },
>> -    { "ieee_double", 64, 'd', 'f' },
>>       /* 32 bit */
>> +    { "ieee_single", 32, 's', 'f' },
>>       { "uint32", 32, 's', 'u' },
>>       { "int32", 32, 's', 's' },
>> -    { "ieee_single", 32, 's', 'f' },
>>       /* 16 bit */
>> +    { "ieee_half", 16, 'h', 'f' },
>>       { "uint16", 16, 'h', 'u' },
>>       { "int16", 16, 'h', 's' },
>> -    /*
>> -     * TODO: currently there is no reliable way of telling
>> -     * if the remote gdb actually understands ieee_half so
>> -     * we don't expose it in the target description for now.
>> -     * { "ieee_half", 16, 'h', 'f' },
>> -     */
>>       /* bytes */
>>       { "uint8", 8, 'b', 'u' },
>>       { "int8", 8, 'b', 's' },
>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>       GString *s = g_string_new(NULL);
>>       DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>       g_autoptr(GString) ts = g_string_new("");
>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>       info->num = 0;
>>       g_string_printf(s, "<?xml version=\"1.0\"?>");
>>       g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
>> -    g_string_append_printf(s, "<feature name=\"org.qemu.gdb.aarch64.sve\">");
>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>   
>>       /* First define types and totals in a whole VL */
>>       for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>           int count = reg_width / vec_lanes[i].size;
>> -        g_string_printf(ts, "vq%d%c%c", count,
>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>           g_string_append_printf(s,
>>                                  "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>                                  ts->str, vec_lanes[i].gdb_type, count);
>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>        * signed and potentially float versions of each size from 128 to
>>        * 8 bits.
>>        */
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>> -                                       vec_lanes[i].suffix,
>> -                                       count,
>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>> +            if (vec_lanes[j].size == bits) {
>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>> +                                       vec_lanes[j].suffix,
>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>               }
>>           }
>>           g_string_append(s, "</union>");
>>       }
>>       /* And now the final union of unions */
>> -    g_string_append(s, "<union id=\"vq\">");
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>> -                                       vec_lanes[i].sz, count);
>> -                break;
>> -            }
>> -        }
>> +    g_string_append(s, "<union id=\"svev\">");
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>> +                               suf[i], suf[i]);
>>       }
>>       g_string_append(s, "</union>");
>>   
>> +    /* Finally the sve prefix type */
>> +    g_string_append_printf(s,
>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>> +                           reg_width / 8);
>> +
>>       /* Then define each register in parts for each vq */
>>       for (i = 0; i < 32; i++) {
>>           g_string_append_printf(s,
>>                                  "<reg name=\"z%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vq\"/>",
>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>                                  i, reg_width, base_reg++);
>>           info->num++;
>>       }
>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>                              " regnum=\"%d\" group=\"float\""
>>                              " type=\"int\"/>", base_reg++);
>>       info->num += 2;
>> -    /*
>> -     * Predicate registers aren't so big they are worth splitting up
>> -     * but we do need to define a type to hold the array of quad
>> -     * references.
>> -     */
>> -    g_string_append_printf(s,
>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>> -                           cpu->sve_max_vq);
>> +
>>       for (i = 0; i < 16; i++) {
>>           g_string_append_printf(s,
>>                                  "<reg name=\"p%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vqp\"/>",
>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>                                  i, cpu->sve_max_vq * 16, base_reg++);
>>           info->num++;
>>       }
>>       g_string_append_printf(s,
>>                              "<reg name=\"ffr\" bitsize=\"%d\""
>>                              " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"vqp\"/>",
>> +                           " type=\"svep\"/>",
>>                              cpu->sve_max_vq * 16, base_reg++);
>>       g_string_append_printf(s,
>>                              "<reg name=\"vg\" bitsize=\"64\""
>> -                           " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"uint32\"/>",
>> +                           " regnum=\"%d\" type=\"int\"/>",
>>                              base_reg++);
>>       info->num += 2;
>>       g_string_append_printf(s, "</feature>");
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index d077dd9ef5..d434044f07 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>            * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>            */
>>           int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>> -        return gdb_get_reg32(buf, vq * 2);
>> +        return gdb_get_reg64(buf, vq * 2);
>>       }
>>       default:
>>           /* gdbstub asked for something out our range */
>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> index 972cf73c31..b9ef169c1a 100644
>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>           except gdb.error:
>>               report(False, "checking zregs (out of range)")
>>   
>> +        # Check the aliased V registers are set and GDB has correctly
>> +        # created them for us having recognised and handled SVE.
>> +        try:
>> +            for i in range(0, 16):
>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>> +                report(int(val_z) == int(val_v),
>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>> +        except gdb.error:
>> +            report(False, "checking vregs (out of range)")
>> +
>>   
>>   def run_test():
>>       "Run through the tests one by one"
>> 


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers
  2021-01-11 14:36     ` Alex Bennée
@ 2021-01-11 14:50       ` Luis Machado
  0 siblings, 0 replies; 36+ messages in thread
From: Luis Machado @ 2021-01-11 14:50 UTC (permalink / raw)
  To: Alex Bennée; +Cc: Peter Maydell, open list:ARM TCG CPUs, qemu-devel

Hi,

On 1/11/21 11:36 AM, Alex Bennée wrote:
> 
> Luis Machado <luis.machado@linaro.org> writes:
> 
>> For the record, the layout looks OK to me.
> 
> So a Reviewed-by?
> 

Yes.

>> Just a reminder that GDB will soon support bfloat16 types. A patch may
>> be pushed this month.
> 
> Will we be able to probe for the support - or will an older GDB silently
> accept and drop any bfloat16 fields?
> 

No probing unfortunately. I think GDB wouldn't handle it nicely. Older 
GDB's not supporting bfloat16 may throw an internal error when they see 
an unknown type.

That may need to be corrected to make it more robust.

>>
>> On 1/8/21 7:42 PM, Alex Bennée wrote:
>>> While GDB can work with any XML description given to it there is
>>> special handling for SVE registers on the GDB side which makes the
>>> users life a little better. The changes aren't that major and all the
>>> registers save the $vg reported the same. All that changes is:
>>>
>>>     - report org.gnu.gdb.aarch64.sve
>>>     - use gdb nomenclature for names and types
>>>     - minor re-ordering of the types to match reference
>>>     - re-enable ieee_half (as we know gdb supports it now)
>>>     - $vg is now a 64 bit int
>>>     - check $vN and $zN aliasing in test
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Luis Machado <luis.machado@linaro.org>
>>> Message-Id: <20201218112707.28348-10-alex.bennee@linaro.org>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    target/arm/gdbstub.c                        | 75 ++++++++-------------
>>>    target/arm/helper.c                         |  2 +-
>>>    tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 11 +++
>>>    3 files changed, 41 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>>> index 866595b4f1..a8fff2a3d0 100644
>>> --- a/target/arm/gdbstub.c
>>> +++ b/target/arm/gdbstub.c
>>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>>        { "uint128", 128, 'q', 'u' },
>>>        { "int128", 128, 'q', 's' },
>>>        /* 64 bit */
>>> +    { "ieee_double", 64, 'd', 'f' },
>>>        { "uint64", 64, 'd', 'u' },
>>>        { "int64", 64, 'd', 's' },
>>> -    { "ieee_double", 64, 'd', 'f' },
>>>        /* 32 bit */
>>> +    { "ieee_single", 32, 's', 'f' },
>>>        { "uint32", 32, 's', 'u' },
>>>        { "int32", 32, 's', 's' },
>>> -    { "ieee_single", 32, 's', 'f' },
>>>        /* 16 bit */
>>> +    { "ieee_half", 16, 'h', 'f' },
>>>        { "uint16", 16, 'h', 'u' },
>>>        { "int16", 16, 'h', 's' },
>>> -    /*
>>> -     * TODO: currently there is no reliable way of telling
>>> -     * if the remote gdb actually understands ieee_half so
>>> -     * we don't expose it in the target description for now.
>>> -     * { "ieee_half", 16, 'h', 'f' },
>>> -     */
>>>        /* bytes */
>>>        { "uint8", 8, 'b', 'u' },
>>>        { "int8", 8, 'b', 's' },
>>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>        GString *s = g_string_new(NULL);
>>>        DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>>        g_autoptr(GString) ts = g_string_new("");
>>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>>        info->num = 0;
>>>        g_string_printf(s, "<?xml version=\"1.0\"?>");
>>>        g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
>>> -    g_string_append_printf(s, "<feature name=\"org.qemu.gdb.aarch64.sve\">");
>>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>>    
>>>        /* First define types and totals in a whole VL */
>>>        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>>            int count = reg_width / vec_lanes[i].size;
>>> -        g_string_printf(ts, "vq%d%c%c", count,
>>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>>            g_string_append_printf(s,
>>>                                   "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>>                                   ts->str, vec_lanes[i].gdb_type, count);
>>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>         * signed and potentially float versions of each size from 128 to
>>>         * 8 bits.
>>>         */
>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>> -        int count = reg_width / bits;
>>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>> -            if (vec_lanes[i].size == bits) {
>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>>> -                                       vec_lanes[i].suffix,
>>> -                                       count,
>>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>>> +            if (vec_lanes[j].size == bits) {
>>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>>> +                                       vec_lanes[j].suffix,
>>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>>                }
>>>            }
>>>            g_string_append(s, "</union>");
>>>        }
>>>        /* And now the final union of unions */
>>> -    g_string_append(s, "<union id=\"vq\">");
>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>> -        int count = reg_width / bits;
>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>> -            if (vec_lanes[i].size == bits) {
>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>>> -                                       vec_lanes[i].sz, count);
>>> -                break;
>>> -            }
>>> -        }
>>> +    g_string_append(s, "<union id=\"svev\">");
>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>>> +                               suf[i], suf[i]);
>>>        }
>>>        g_string_append(s, "</union>");
>>>    
>>> +    /* Finally the sve prefix type */
>>> +    g_string_append_printf(s,
>>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>>> +                           reg_width / 8);
>>> +
>>>        /* Then define each register in parts for each vq */
>>>        for (i = 0; i < 32; i++) {
>>>            g_string_append_printf(s,
>>>                                   "<reg name=\"z%d\" bitsize=\"%d\""
>>> -                               " regnum=\"%d\" group=\"vector\""
>>> -                               " type=\"vq\"/>",
>>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>>                                   i, reg_width, base_reg++);
>>>            info->num++;
>>>        }
>>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>                               " regnum=\"%d\" group=\"float\""
>>>                               " type=\"int\"/>", base_reg++);
>>>        info->num += 2;
>>> -    /*
>>> -     * Predicate registers aren't so big they are worth splitting up
>>> -     * but we do need to define a type to hold the array of quad
>>> -     * references.
>>> -     */
>>> -    g_string_append_printf(s,
>>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>>> -                           cpu->sve_max_vq);
>>> +
>>>        for (i = 0; i < 16; i++) {
>>>            g_string_append_printf(s,
>>>                                   "<reg name=\"p%d\" bitsize=\"%d\""
>>> -                               " regnum=\"%d\" group=\"vector\""
>>> -                               " type=\"vqp\"/>",
>>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>>                                   i, cpu->sve_max_vq * 16, base_reg++);
>>>            info->num++;
>>>        }
>>>        g_string_append_printf(s,
>>>                               "<reg name=\"ffr\" bitsize=\"%d\""
>>>                               " regnum=\"%d\" group=\"vector\""
>>> -                           " type=\"vqp\"/>",
>>> +                           " type=\"svep\"/>",
>>>                               cpu->sve_max_vq * 16, base_reg++);
>>>        g_string_append_printf(s,
>>>                               "<reg name=\"vg\" bitsize=\"64\""
>>> -                           " regnum=\"%d\" group=\"vector\""
>>> -                           " type=\"uint32\"/>",
>>> +                           " regnum=\"%d\" type=\"int\"/>",
>>>                               base_reg++);
>>>        info->num += 2;
>>>        g_string_append_printf(s, "</feature>");
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index d077dd9ef5..d434044f07 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>>             * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>>             */
>>>            int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>>> -        return gdb_get_reg32(buf, vq * 2);
>>> +        return gdb_get_reg64(buf, vq * 2);
>>>        }
>>>        default:
>>>            /* gdbstub asked for something out our range */
>>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> index 972cf73c31..b9ef169c1a 100644
>>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>>            except gdb.error:
>>>                report(False, "checking zregs (out of range)")
>>>    
>>> +        # Check the aliased V registers are set and GDB has correctly
>>> +        # created them for us having recognised and handled SVE.
>>> +        try:
>>> +            for i in range(0, 16):
>>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>>> +                report(int(val_z) == int(val_v),
>>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>>> +        except gdb.error:
>>> +            report(False, "checking vregs (out of range)")
>>> +
>>>    
>>>    def run_test():
>>>        "Run through the tests one by one"
>>>
> 
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 02/20] test/guest-debug: echo QEMU command as well
  2021-01-08 22:42 ` [PATCH v1 02/20] test/guest-debug: echo QEMU command as well Alex Bennée
@ 2021-01-11 15:27   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:27 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel

On 1/8/21 11:42 PM, Alex Bennée wrote:
> This helps with debugging.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20201214153012.12723-2-alex.bennee@linaro.org>
> Message-Id: <20201218112707.28348-2-alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/guest-debug/run-test.py | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories
  2021-01-08 22:42 ` [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories Alex Bennée
@ 2021-01-11 15:30   ` Philippe Mathieu-Daudé
  2021-01-11 15:32     ` Philippe Mathieu-Daudé
  2021-01-11 15:46   ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:30 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Maydell, Keith Packard, open list:ARM TCG CPUs,
	Alistair Francis, Laurent Vivier

On 1/8/21 11:42 PM, Alex Bennée wrote:
> From: Keith Packard <keithp@keithp.com>
> 
> This commit renames two files which provide ARM semihosting support so
> that they can be shared by other architectures:
> 
>  1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
>  2. linux-user/arm/semihost.c -> linux-user/semihost.c
> 
> The build system was modified use a new config variable,
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING, which has been added to the ARM
> softmmu and linux-user default configs. The contents of the source
> files has not been changed in this patch.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20210107170717.2098982-2-keithp@keithp.com>
> ---
>  default-configs/devices/arm-softmmu.mak               | 1 +
>  default-configs/targets/aarch64-linux-user.mak        | 1 +
>  default-configs/targets/aarch64_be-linux-user.mak     | 1 +
>  default-configs/targets/arm-linux-user.mak            | 1 +
>  default-configs/targets/armeb-linux-user.mak          | 1 +
>  target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
>  linux-user/{arm => }/semihost.c                       | 0
>  hw/semihosting/Kconfig                                | 3 +++
>  hw/semihosting/meson.build                            | 3 +++
>  linux-user/arm/meson.build                            | 3 ---
>  linux-user/meson.build                                | 1 +
>  target/arm/meson.build                                | 2 --
>  12 files changed, 12 insertions(+), 5 deletions(-)
>  rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
>  rename linux-user/{arm => }/semihost.c (100%)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories
  2021-01-11 15:30   ` Philippe Mathieu-Daudé
@ 2021-01-11 15:32     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:32 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Maydell, Keith Packard, open list:ARM TCG CPUs,
	Alistair Francis, Laurent Vivier

On 1/11/21 4:30 PM, Philippe Mathieu-Daudé wrote:
> On 1/8/21 11:42 PM, Alex Bennée wrote:
>> From: Keith Packard <keithp@keithp.com>
>>
>> This commit renames two files which provide ARM semihosting support so
>> that they can be shared by other architectures:
>>
>>  1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
>>  2. linux-user/arm/semihost.c -> linux-user/semihost.c
>>
>> The build system was modified use a new config variable,
>> CONFIG_ARM_COMPATIBLE_SEMIHOSTING, which has been added to the ARM
>> softmmu and linux-user default configs. The contents of the source
>> files has not been changed in this patch.
>>
>> Signed-off-by: Keith Packard <keithp@keithp.com>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20210107170717.2098982-2-keithp@keithp.com>
>> ---
>>  default-configs/devices/arm-softmmu.mak               | 1 +
>>  default-configs/targets/aarch64-linux-user.mak        | 1 +
>>  default-configs/targets/aarch64_be-linux-user.mak     | 1 +
>>  default-configs/targets/arm-linux-user.mak            | 1 +
>>  default-configs/targets/armeb-linux-user.mak          | 1 +
>>  target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
>>  linux-user/{arm => }/semihost.c                       | 0
>>  hw/semihosting/Kconfig                                | 3 +++
>>  hw/semihosting/meson.build                            | 3 +++
>>  linux-user/arm/meson.build                            | 3 ---
>>  linux-user/meson.build                                | 1 +
>>  target/arm/meson.build                                | 2 --
>>  12 files changed, 12 insertions(+), 5 deletions(-)
>>  rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
>>  rename linux-user/{arm => }/semihost.c (100%)

Can we name these file with some explicit "arm_compat_semihosting"?

> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent
  2021-01-08 22:42 ` [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent Alex Bennée
@ 2021-01-11 15:34   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:34 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Maydell, Keith Packard, open list:ARM TCG CPUs,
	Alistair Francis, Laurent Vivier

On 1/8/21 11:42 PM, Alex Bennée wrote:
> From: Keith Packard <keithp@keithp.com>
> 
> The public API is now defined in
> hw/semihosting/common-semi.h. do_common_semihosting takes CPUState *
> instead of CPUARMState *. All internal functions have been renamed
> common_semi_ instead of arm_semi_ or arm_. Aside from the API change,
> there are no functional changes in this patch.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-Id: <20210107170717.2098982-3-keithp@keithp.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/semihosting/common-semi.h  | 36 +++++++++++++++++++++++++++++++++++
>  target/arm/cpu.h              |  8 --------
>  hw/semihosting/common-semi.c  | 16 ++++++++++------
>  linux-user/aarch64/cpu_loop.c |  3 ++-
>  linux-user/arm/cpu_loop.c     |  3 ++-
>  target/arm/helper.c           |  5 +++--
>  target/arm/m_helper.c         |  7 ++++++-
>  7 files changed, 59 insertions(+), 19 deletions(-)
>  create mode 100644 hw/semihosting/common-semi.h
> 
> diff --git a/hw/semihosting/common-semi.h b/hw/semihosting/common-semi.h
> new file mode 100644
> index 0000000000..bc53e92c79
> --- /dev/null
> +++ b/hw/semihosting/common-semi.h
> @@ -0,0 +1,36 @@
> +/*
> + *  Semihosting support for systems modeled on the Arm "Angel"
> + *  semihosting syscalls design.
> + *
> + *  Copyright (c) 2005, 2007 CodeSourcery.
> + *  Copyright (c) 2019 Linaro
> + *  Written by Paul Brook.
> + *
> + *  Copyright © 2020 by Keith Packard <keithp@keithp.com>
> + *  Adapted for systems other than ARM, including RISC-V, by Keith Packard
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + *  ARM Semihosting is documented in:
> + *     Semihosting for AArch32 and AArch64 Release 2.0
> + *     https://static.docs.arm.com/100863/0200/semihosting.pdf
> + *
> + */
> +
> +#ifndef COMMON_SEMI_H
> +#define COMMON_SEMI_H
> +
> +target_ulong do_common_semihosting(CPUState *cs);

Can we name this do_arm_compat_semihosting(CPUState *cs)?



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories
  2021-01-08 22:42 ` [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories Alex Bennée
  2021-01-11 15:30   ` Philippe Mathieu-Daudé
@ 2021-01-11 15:46   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:46 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Maydell, Keith Packard, open list:ARM TCG CPUs,
	Alistair Francis, Laurent Vivier

On 1/8/21 11:42 PM, Alex Bennée wrote:
> From: Keith Packard <keithp@keithp.com>
> 
> This commit renames two files which provide ARM semihosting support so
> that they can be shared by other architectures:
> 
>  1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
>  2. linux-user/arm/semihost.c -> linux-user/semihost.c
> 
> The build system was modified use a new config variable,
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING, which has been added to the ARM
> softmmu and linux-user default configs. The contents of the source
> files has not been changed in this patch.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20210107170717.2098982-2-keithp@keithp.com>
> ---
>  default-configs/devices/arm-softmmu.mak               | 1 +
>  default-configs/targets/aarch64-linux-user.mak        | 1 +
>  default-configs/targets/aarch64_be-linux-user.mak     | 1 +
>  default-configs/targets/arm-linux-user.mak            | 1 +
>  default-configs/targets/armeb-linux-user.mak          | 1 +
>  target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
>  linux-user/{arm => }/semihost.c                       | 0
>  hw/semihosting/Kconfig                                | 3 +++
>  hw/semihosting/meson.build                            | 3 +++
>  linux-user/arm/meson.build                            | 3 ---
>  linux-user/meson.build                                | 1 +
>  target/arm/meson.build                                | 2 --
>  12 files changed, 12 insertions(+), 5 deletions(-)
>  rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
>  rename linux-user/{arm => }/semihost.c (100%)
...

> diff --git a/hw/semihosting/Kconfig b/hw/semihosting/Kconfig
> index efe0a30734..4c30dc6b16 100644
> --- a/hw/semihosting/Kconfig
> +++ b/hw/semihosting/Kconfig
> @@ -1,3 +1,6 @@
>  
>  config SEMIHOSTING
>         bool
> +
> +config ARM_COMPATIBLE_SEMIHOSTING
> +       bool

This misses:

          "select SEMIHOSTING"

'SEMIHOSTING' is a bit confusing. Wondering about better names,
maybe SEMIHOSTING_HOST_CONSOLE is clearer? (question not related
to this patch).



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v1 17/20] riscv: Add semihosting support for user mode
  2021-01-08 22:42 ` [PATCH v1 17/20] riscv: Add semihosting support for user mode Alex Bennée
  2021-01-08 23:32   ` Alistair Francis
@ 2021-01-11 15:48   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-11 15:48 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Kito Cheng, Keith Packard, Laurent Vivier

On 1/8/21 11:42 PM, Alex Bennée wrote:
> From: Kito Cheng <kito.cheng@sifive.com>
> 
> This could made testing more easier and ARM/AArch64 has supported on
> their linux user mode too, so I think it should be reasonable.
> 
> Verified GCC testsuite with newlib/semihosting.
> 
> Signed-off-by: Kito Cheng <kito.cheng@sifive.com>
> Reviewed-by: Keith Packard <keithp@keithp.com>
> Message-Id: <20210107170717.2098982-7-keithp@keithp.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/riscv/cpu_loop.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2021-01-11 15:50 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08 22:42 [PATCH v1 00/20] gdbstub, semihosting and test/tool updates (pre PR) Alex Bennée
2021-01-08 22:42 ` [PATCH v1 01/20] tests/docker: Remove Debian 9 remnant lines Alex Bennée
2021-01-08 22:42 ` [PATCH v1 02/20] test/guest-debug: echo QEMU command as well Alex Bennée
2021-01-11 15:27   ` Philippe Mathieu-Daudé
2021-01-08 22:42 ` [PATCH v1 03/20] configure: gate our use of GDB to 8.3.1 or above Alex Bennée
2021-01-08 22:42 ` [PATCH v1 04/20] Revert "tests/tcg/multiarch/Makefile.target: Disable run-gdbstub-sha1 test" Alex Bennée
2021-01-08 22:42 ` [PATCH v1 05/20] gdbstub: implement a softmmu based test Alex Bennée
2021-01-08 22:42 ` [PATCH v1 06/20] gdbstub: add support to Xfer:auxv:read: packet Alex Bennée
2021-01-08 22:42 ` [PATCH v1 07/20] gdbstub: drop CPUEnv from gdb_exit() Alex Bennée
2021-01-08 22:42 ` [PATCH v1 08/20] gdbstub: drop gdbserver_cleanup in favour of gdb_exit Alex Bennée
2021-01-08 22:42 ` [PATCH v1 09/20] gdbstub: ensure we clean-up when terminated Alex Bennée
2021-01-08 22:42 ` [PATCH v1 10/20] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers Alex Bennée
2021-01-11 13:20   ` Luis Machado
2021-01-11 14:36     ` Alex Bennée
2021-01-11 14:50       ` Luis Machado
2021-01-08 22:42 ` [PATCH v1 11/20] Makefile: add GNU global tags support Alex Bennée
2021-01-08 22:42 ` [PATCH v1 12/20] semihosting: Move ARM semihosting code to shared directories Alex Bennée
2021-01-11 15:30   ` Philippe Mathieu-Daudé
2021-01-11 15:32     ` Philippe Mathieu-Daudé
2021-01-11 15:46   ` Philippe Mathieu-Daudé
2021-01-08 22:42 ` [PATCH v1 13/20] semihosting: Change common-semi API to be architecture-independent Alex Bennée
2021-01-11 15:34   ` Philippe Mathieu-Daudé
2021-01-08 22:42 ` [PATCH v1 14/20] semihosting: Change internal common-semi interfaces to use CPUState * Alex Bennée
2021-01-08 22:42 ` [PATCH v1 15/20] semihosting: Support SYS_HEAPINFO when env->boot_info is not set Alex Bennée
2021-01-08 22:42 ` [PATCH v1 16/20] riscv: Add semihosting support Alex Bennée
2021-01-08 22:42   ` Alex Bennée
2021-01-08 23:30   ` Alistair Francis
2021-01-08 23:30     ` Alistair Francis
2021-01-09  0:44     ` Keith Packard via
2021-01-09  0:44       ` Keith Packard
2021-01-08 22:42 ` [PATCH v1 17/20] riscv: Add semihosting support for user mode Alex Bennée
2021-01-08 23:32   ` Alistair Francis
2021-01-11 15:48   ` Philippe Mathieu-Daudé
2021-01-08 22:42 ` [PATCH v1 18/20] semihosting: Implement SYS_ELAPSED and SYS_TICKFREQ Alex Bennée
2021-01-08 22:42 ` [PATCH v1 19/20] semihosting: Implement SYS_TMPNAM Alex Bennée
2021-01-08 22:42 ` [PATCH v1 20/20] semihosting: Implement SYS_ISERROR Alex Bennée

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.