From 9b109f566adff4d394806b046b395363e523bd80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 30 May 2019 10:40:28 +0100 Subject: [PATCH 01/40] editorconfig: add setting for shell scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- .editorconfig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.editorconfig b/.editorconfig index df6db65531..a001f340bd 100644 --- a/.editorconfig +++ b/.editorconfig @@ -26,6 +26,10 @@ file_type_emacs = makefile indent_style = space indent_size = 4 +[*.sh] +indent_style = space +indent_size = 4 + [*.{s,S}] indent_style = tab indent_size = 8 From 50290c002c045280f8defad911901e16bfb52884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 29 May 2019 17:16:32 +0100 Subject: [PATCH 02/40] qemu-io-cmds: use clock_gettime for benchmarking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous use of gettimeofday() ran into undefined behaviour when we ended up doing a div 0 for a very short operation. This is because gettimeofday only works at the microsecond level as well as being prone to discontinuous jumps in system time. Using clock_gettime with CLOCK_MONOTONIC gives greater precision and alleviates some of the potential problems with time jumping around. We could use CLOCK_MONOTONIC_RAW to avoid being tripped up by NTP and adjtime but that is Linux specific so I decided it would do for now. Signed-off-by: Alex Bennée --- qemu-io-cmds.c | 77 +++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 30a7d9a13b..8904733961 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -248,20 +248,21 @@ static void cvtstr(double value, char *str, size_t size) -static struct timeval tsub(struct timeval t1, struct timeval t2) +static struct timespec tsub(struct timespec t1, struct timespec t2) { - t1.tv_usec -= t2.tv_usec; - if (t1.tv_usec < 0) { - t1.tv_usec += 1000000; + t1.tv_nsec -= t2.tv_nsec; + if (t1.tv_nsec < 0) { + t1.tv_nsec += NANOSECONDS_PER_SECOND; t1.tv_sec--; } t1.tv_sec -= t2.tv_sec; return t1; } -static double tdiv(double value, struct timeval tv) +static double tdiv(double value, struct timespec tv) { - return value / ((double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0)); + double seconds = tv.tv_sec + (tv.tv_nsec / 1e9); + return value / seconds; } #define HOURS(sec) ((sec) / (60 * 60)) @@ -274,29 +275,27 @@ enum { VERBOSE_FIXED_TIME = 0x2, }; -static void timestr(struct timeval *tv, char *ts, size_t size, int format) +static void timestr(struct timespec *tv, char *ts, size_t size, int format) { - double usec = (double)tv->tv_usec / 1000000.0; + double frac_sec = tv->tv_nsec / 1e9; if (format & TERSE_FIXED_TIME) { if (!HOURS(tv->tv_sec)) { - snprintf(ts, size, "%u:%02u.%02u", - (unsigned int) MINUTES(tv->tv_sec), - (unsigned int) SECONDS(tv->tv_sec), - (unsigned int) (usec * 100)); + snprintf(ts, size, "%u:%05.2f", + (unsigned int) MINUTES(tv->tv_sec), + SECONDS(tv->tv_sec) + frac_sec); return; } format |= VERBOSE_FIXED_TIME; /* fallback if hours needed */ } if ((format & VERBOSE_FIXED_TIME) || tv->tv_sec) { - snprintf(ts, size, "%u:%02u:%02u.%02u", + snprintf(ts, size, "%u:%02u:%05.2f", (unsigned int) HOURS(tv->tv_sec), (unsigned int) MINUTES(tv->tv_sec), - (unsigned int) SECONDS(tv->tv_sec), - (unsigned int) (usec * 100)); + SECONDS(tv->tv_sec) + frac_sec); } else { - snprintf(ts, size, "0.%04u sec", (unsigned int) (usec * 10000)); + snprintf(ts, size, "%05.2f sec", frac_sec); } } @@ -376,7 +375,7 @@ static void dump_buffer(const void *buffer, int64_t offset, int64_t len) } } -static void print_report(const char *op, struct timeval *t, int64_t offset, +static void print_report(const char *op, struct timespec *t, int64_t offset, int64_t count, int64_t total, int cnt, bool Cflag) { char s1[64], s2[64], ts[64]; @@ -649,7 +648,7 @@ static const cmdinfo_t read_cmd = { static int read_f(BlockBackend *blk, int argc, char **argv) { - struct timeval t1, t2; + struct timespec t1, t2; bool Cflag = false, qflag = false, vflag = false; bool Pflag = false, sflag = false, lflag = false, bflag = false; int c, cnt, ret; @@ -758,13 +757,13 @@ static int read_f(BlockBackend *blk, int argc, char **argv) buf = qemu_io_alloc(blk, count, 0xab); - gettimeofday(&t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &t1); if (bflag) { ret = do_load_vmstate(blk, buf, offset, count, &total); } else { ret = do_pread(blk, buf, offset, count, &total); } - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("read failed: %s\n", strerror(-ret)); @@ -836,7 +835,7 @@ static const cmdinfo_t readv_cmd = { static int readv_f(BlockBackend *blk, int argc, char **argv) { - struct timeval t1, t2; + struct timespec t1, t2; bool Cflag = false, qflag = false, vflag = false; int c, cnt, ret; char *buf; @@ -891,9 +890,9 @@ static int readv_f(BlockBackend *blk, int argc, char **argv) return -EINVAL; } - gettimeofday(&t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &t1); ret = do_aio_readv(blk, &qiov, offset, &total); - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("readv failed: %s\n", strerror(-ret)); @@ -972,7 +971,7 @@ static const cmdinfo_t write_cmd = { static int write_f(BlockBackend *blk, int argc, char **argv) { - struct timeval t1, t2; + struct timespec t1, t2; bool Cflag = false, qflag = false, bflag = false; bool Pflag = false, zflag = false, cflag = false; int flags = 0; @@ -1091,7 +1090,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv) buf = qemu_io_alloc(blk, count, pattern); } - gettimeofday(&t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &t1); if (bflag) { ret = do_save_vmstate(blk, buf, offset, count, &total); } else if (zflag) { @@ -1101,7 +1100,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv) } else { ret = do_pwrite(blk, buf, offset, count, flags, &total); } - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("write failed: %s\n", strerror(-ret)); @@ -1160,7 +1159,7 @@ static const cmdinfo_t writev_cmd = { static int writev_f(BlockBackend *blk, int argc, char **argv) { - struct timeval t1, t2; + struct timespec t1, t2; bool Cflag = false, qflag = false; int flags = 0; int c, cnt, ret; @@ -1213,9 +1212,9 @@ static int writev_f(BlockBackend *blk, int argc, char **argv) return -EINVAL; } - gettimeofday(&t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &t1); ret = do_aio_writev(blk, &qiov, offset, flags, &total); - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("writev failed: %s\n", strerror(-ret)); @@ -1250,15 +1249,15 @@ struct aio_ctx { bool zflag; BlockAcctCookie acct; int pattern; - struct timeval t1; + struct timespec t1; }; static void aio_write_done(void *opaque, int ret) { struct aio_ctx *ctx = opaque; - struct timeval t2; + struct timespec t2; - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { @@ -1288,9 +1287,9 @@ out: static void aio_read_done(void *opaque, int ret) { struct aio_ctx *ctx = opaque; - struct timeval t2; + struct timespec t2; - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("readv failed: %s\n", strerror(-ret)); @@ -1425,7 +1424,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) return -EINVAL; } - gettimeofday(&ctx->t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &ctx->t1); block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size, BLOCK_ACCT_READ); blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx); @@ -1570,7 +1569,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) return -EINVAL; } - gettimeofday(&ctx->t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &ctx->t1); block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size, BLOCK_ACCT_WRITE); @@ -1746,7 +1745,7 @@ static const cmdinfo_t discard_cmd = { static int discard_f(BlockBackend *blk, int argc, char **argv) { - struct timeval t1, t2; + struct timespec t1, t2; bool Cflag = false, qflag = false; int c, ret; int64_t offset, bytes; @@ -1787,9 +1786,9 @@ static int discard_f(BlockBackend *blk, int argc, char **argv) return -EINVAL; } - gettimeofday(&t1, NULL); + clock_gettime(CLOCK_MONOTONIC, &t1); ret = blk_pdiscard(blk, offset, bytes); - gettimeofday(&t2, NULL); + clock_gettime(CLOCK_MONOTONIC, &t2); if (ret < 0) { printf("discard failed: %s\n", strerror(-ret)); From c87e38399cf4c9b30c8e558c586725a895c877f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 28 May 2019 17:33:04 +0200 Subject: [PATCH 03/40] tests/docker: Update the Fedora image to Fedora 30 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fedora 30 got released: https://fedoramagazine.org/announcing-fedora-30/ Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Marc-André Lureau Tested-by: Stefano Garzarella Reviewed-by: Stefano Garzarella Message-Id: <20190528153304.27157-1-philmd@redhat.com> Signed-off-by: Alex Bennée --- tests/docker/dockerfiles/fedora.docker | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index afbba29ada..12c460597e 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -1,4 +1,4 @@ -FROM fedora:29 +FROM fedora:30 ENV PACKAGES \ bc \ bison \ From acc5c5061d084ba00b5dad35d35e85fe97acbdf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 30 May 2019 10:52:53 +0100 Subject: [PATCH 04/40] tests/docker: Update the Fedora cross compile images to 30 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While at it remove the bogus :latest tag for cris cross compiler. It tends to break caching and cause confusion. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/docker/dockerfiles/fedora-cris-cross.docker | 2 +- tests/docker/dockerfiles/fedora-i386-cross.docker | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora-cris-cross.docker b/tests/docker/dockerfiles/fedora-cris-cross.docker index b168ada615..09e7e449f9 100644 --- a/tests/docker/dockerfiles/fedora-cris-cross.docker +++ b/tests/docker/dockerfiles/fedora-cris-cross.docker @@ -2,7 +2,7 @@ # Cross compiler for cris system tests # -FROM fedora:latest +FROM fedora:30 ENV PACKAGES gcc-cris-linux-gnu RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index eb8108d118..9106cf9ebe 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -1,4 +1,4 @@ -FROM fedora:29 +FROM fedora:30 ENV PACKAGES \ gcc \ glib2-devel.i686 \ From 248cf06cf2b47cda15324480cfc2bd407b4a7e15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 29 May 2019 21:03:01 +0100 Subject: [PATCH 05/40] tests/docker: Update the Ubuntu image to 19.04 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This has aged a little and we have a separate LTS image for testing on the older distros. Update it to a more recent release like its Fedora cousin. Besides it is useful to have something with gcc-9 on it for squashing those stringop truncation errors. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- tests/docker/dockerfiles/ubuntu.docker | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/docker/dockerfiles/ubuntu.docker b/tests/docker/dockerfiles/ubuntu.docker index 36e2b17de5..8d256961f0 100644 --- a/tests/docker/dockerfiles/ubuntu.docker +++ b/tests/docker/dockerfiles/ubuntu.docker @@ -1,6 +1,15 @@ -FROM ubuntu:16.04 -RUN echo "deb http://archive.ubuntu.com/ubuntu/ trusty universe multiverse" >> \ - /etc/apt/sources.list +# +# Latest Ubuntu Release +# +# Useful for testing against relatively bleeding edge libraries and +# compilers. We also have seperate recipe for the most recent LTS +# release. +# +# When updating use the full tag not :latest otherwise the build +# system won't pick up that it has changed. +# + +FROM ubuntu:19.04 ENV PACKAGES flex bison \ ccache \ clang \ @@ -21,7 +30,7 @@ ENV PACKAGES flex bison \ libepoxy-dev \ libfdt-dev \ libgbm-dev \ - libgnutls-dev \ + libgnutls28-dev \ libgtk-3-dev \ libibverbs-dev \ libiscsi-dev \ @@ -34,7 +43,7 @@ ENV PACKAGES flex bison \ libnss3-dev \ libnuma-dev \ libpixman-1-dev \ - libpng12-dev \ + libpng-dev \ librados-dev \ librbd-dev \ librdmacm-dev \ From 3998c25e5368275257e2a44c483500a55510b3d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 28 May 2019 19:13:08 +0100 Subject: [PATCH 06/40] .travis.yml: bump gcc sanitiser job to gcc-9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toolchain PPA has it so we might as well use it. We currently have to add: -Wno-error=stringop-truncation as there are still strncpy operations in the tree operating on things that haven't been annotated with QEMU_NONSTRING. Signed-off-by: Alex Bennée --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index a08a7b7278..a6bb1f9f98 100644 --- a/.travis.yml +++ b/.travis.yml @@ -240,8 +240,8 @@ matrix: - ubuntu-toolchain-r-test packages: # Extra toolchains - - gcc-7 - - g++-7 + - gcc-9 + - g++-9 # Build dependencies - libaio-dev - libattr1-dev @@ -270,11 +270,11 @@ matrix: language: generic compiler: none env: - - COMPILER_NAME=gcc CXX=g++-7 CC=gcc-7 - - CONFIG="--cc=gcc-7 --cxx=g++-7 --disable-pie --disable-linux-user" + - COMPILER_NAME=gcc CXX=g++-9 CC=gcc-9 + - CONFIG="--cc=gcc-9 --cxx=g++-9 --disable-pie --disable-linux-user" - TEST_CMD="" before_script: - - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread -fuse-ld=gold" || { cat config.log && exit 1; } + - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -Wno-error=stringop-truncation -fsanitize=thread -fuse-ld=gold" || { cat config.log && exit 1; } # Run check-tcg against linux-user From 7831147edfeb3b0d54a7bff411bf41cc2398f924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 28 May 2019 19:21:19 +0100 Subject: [PATCH 07/40] .travis.yml: add clang ubsan job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We document this on our wiki and we might as well catch it in our CI rather than waiting for it to be picked up on merge: https://wiki.qemu.org/Testing#clang_UBSan Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- .travis.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.travis.yml b/.travis.yml index a6bb1f9f98..08502c0aa2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -152,6 +152,13 @@ matrix: compiler: clang + - env: + - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} " + compiler: clang + before_script: + - ./configure ${CONFIG} --extra-cflags="-fsanitize=undefined -Werror" || { cat config.log && exit 1; } + + - env: - CONFIG="--disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS}" compiler: clang From b0040fa17bcf0e51d4a599ee82108ace830d4eac Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Fri, 29 Mar 2019 17:08:00 -0400 Subject: [PATCH 08/40] tests/vm: Use python configured on build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed the vm-test makefile to execute python scripts with the interpreter configured on build. This allows to run vm-test targets properly in Linux distros with Python 3 only support. Signed-off-by: Wainer dos Santos Moschetta Message-Id: <20190329210804.22121-2-wainersm@redhat.com> Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/vm/Makefile.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include index 992d823f6b..6f82676306 100644 --- a/tests/vm/Makefile.include +++ b/tests/vm/Makefile.include @@ -35,7 +35,7 @@ $(IMAGES_DIR)/%.img: $(SRC_PATH)/tests/vm/% \ $(SRC_PATH)/tests/vm/Makefile.include @mkdir -p $(IMAGES_DIR) $(call quiet-command, \ - $< \ + $(PYTHON) $< \ $(if $(V)$(DEBUG), --debug) \ --image "$@" \ --force \ @@ -46,7 +46,7 @@ $(IMAGES_DIR)/%.img: $(SRC_PATH)/tests/vm/% \ # Build in VM $(IMAGE) vm-build-%: $(IMAGES_DIR)/%.img $(call quiet-command, \ - $(SRC_PATH)/tests/vm/$* \ + $(PYTHON) $(SRC_PATH)/tests/vm/$* \ $(if $(V)$(DEBUG), --debug) \ $(if $(DEBUG), --interactive) \ $(if $(J),--jobs $(J)) \ From 3ad3e36e1469a37740b5ec41d8d917c5ecd40c16 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Fri, 29 Mar 2019 17:08:01 -0400 Subject: [PATCH 09/40] tests/vm: Port basevm to Python 3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed tests/vm/basevm.py to run with Python 3: - hashlib.sha1() requires an binary encoded object. - uses floor division ("//") (PEP 238). - decode bytes to unicode when needed. Signed-off-by: Wainer dos Santos Moschetta Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20190329210804.22121-3-wainersm@redhat.com> Signed-off-by: Alex Bennée Tested-by: Philippe Mathieu-Daudé --- tests/vm/basevm.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 0556bdcf9e..083befce9f 100755 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -85,12 +85,12 @@ class BaseVM(object): if not sha256sum: return True checksum = subprocess.check_output(["sha256sum", fname]).split()[0] - return sha256sum == checksum + return sha256sum == checksum.decode() cache_dir = os.path.expanduser("~/.cache/qemu-vm/download") if not os.path.exists(cache_dir): os.makedirs(cache_dir) - fname = os.path.join(cache_dir, hashlib.sha1(url).hexdigest()) + fname = os.path.join(cache_dir, hashlib.sha1(url.encode()).hexdigest()) if os.path.exists(fname) and check_sha256sum(fname): return fname logging.debug("Downloading %s to %s...", url, fname) @@ -134,7 +134,7 @@ class BaseVM(object): raise NotImplementedError def add_source_dir(self, src_dir): - name = "data-" + hashlib.sha1(src_dir).hexdigest()[:5] + name = "data-" + hashlib.sha1(src_dir.encode()).hexdigest()[:5] tarfile = os.path.join(self._tmpdir, name + ".tar") logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir) subprocess.check_call(["./scripts/archive-source.sh", tarfile], @@ -204,7 +204,7 @@ def parse_args(vmcls): def get_default_jobs(): if kvm_available(vmcls.arch): - return multiprocessing.cpu_count() / 2 + return multiprocessing.cpu_count() // 2 else: return 1 From aea439138b97d5ab5032b2e00f5d99df9ef40f71 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Fri, 29 Mar 2019 17:08:03 -0400 Subject: [PATCH 10/40] tests/vm: Fix build-centos docker-based tests run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make vm-build-centos` run docker-based tests on CentOS. The created containers should have network otherwise some tests fail. Also fixed the BUILD_SCRIPT template to correctly evaluate "V=1" for verbose output. Signed-off-by: Wainer dos Santos Moschetta Message-Id: <20190329210804.22121-5-wainersm@redhat.com> Signed-off-by: Alex Bennée --- tests/vm/centos | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/vm/centos b/tests/vm/centos index ba133ea429..7417b50af4 100755 --- a/tests/vm/centos +++ b/tests/vm/centos @@ -26,9 +26,9 @@ class CentosVM(basevm.BaseVM): export SRC_ARCHIVE=/dev/vdb; sudo chmod a+r $SRC_ARCHIVE; tar -xf $SRC_ARCHIVE; - make docker-test-block@centos7 V={verbose} J={jobs}; - make docker-test-quick@centos7 V={verbose} J={jobs}; - make docker-test-mingw@fedora V={verbose} J={jobs}; + make docker-test-block@centos7 {verbose} J={jobs} NETWORK=1; + make docker-test-quick@centos7 {verbose} J={jobs} NETWORK=1; + make docker-test-mingw@fedora {verbose} J={jobs} NETWORK=1; """ def _gen_cloud_init_iso(self): From eec4b30ae6dff370599015792ed9f1c60e436c33 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Fri, 29 Mar 2019 17:08:04 -0400 Subject: [PATCH 11/40] tests/vm: Add missing variables on help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added description of variables missing on vm-test help. Signed-off-by: Wainer dos Santos Moschetta Message-Id: <20190329210804.22121-6-wainersm@redhat.com> Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- tests/vm/Makefile.include | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include index 6f82676306..c59411bee0 100644 --- a/tests/vm/Makefile.include +++ b/tests/vm/Makefile.include @@ -21,9 +21,13 @@ vm-test: @echo " vm-clean-all - Clean up VM images" @echo @echo "Special variables:" - @echo " BUILD_TARGET=foo - override the build target" - @echo " TARGET_LIST=a,b,c - Override target list in builds." + @echo " BUILD_TARGET=foo - Override the build target" + @echo " TARGET_LIST=a,b,c - Override target list in builds" @echo ' EXTRA_CONFIGURE_OPTS="..."' + @echo " J=[0..9]* - Override the -jN parameter for make commands" + @echo " DEBUG=1 - Enable verbose output on host and interactive debugging" + @echo " V=1 - Enable verbose ouput on host and guest commands" + @echo " QEMU=/path/to/qemu - Change path to QEMU binary" vm-build-all: $(addprefix vm-build-, $(IMAGES)) From 8fc76176f60d6758641f50f43c2c4bcceab8a68d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 20 May 2019 14:47:03 +0200 Subject: [PATCH 12/40] scripts: use git archive in archive-source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use git archive to create tarballs of qemu and submodules instead of cloning the repository and the submodules. This is a order of magnitude faster because it doesn't fetch the submodules from the internet each time the script runs. Signed-off-by: Gerd Hoffmann Tested-by: Thomas Huth Tested-by: Philippe Mathieu-Daudé Message-Id: <20190520124716.30472-2-kraxel@redhat.com> [AJB: fixed up tabs] Signed-off-by: Alex Bennée --- scripts/archive-source.sh | 74 +++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 42 deletions(-) diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh index 8b89948260..ca94e49978 100755 --- a/scripts/archive-source.sh +++ b/scripts/archive-source.sh @@ -19,16 +19,25 @@ if test $# -lt 1; then fi tar_file=$(realpath "$1") -list_file="${tar_file}.list" -vroot_dir="${tar_file}.vroot" +sub_tdir=$(mktemp -d "${tar_file%.tar}.sub.XXXXXXXX") +sub_file="${sub_tdir}/submodule.tar" # We want a predictable list of submodules for builds, that is # independent of what the developer currently has initialized # in their checkout, because the build environment is completely # different to the host OS. submodules="dtc slirp ui/keycodemapdb tests/fp/berkeley-softfloat-3 tests/fp/berkeley-testfloat-3" +sub_deinit="" -trap "status=$?; rm -rf \"$list_file\" \"$vroot_dir\"; exit \$status" 0 1 2 3 15 +function cleanup() { + local status=$? + rm -rf "$sub_tdir" + if test "$sub_deinit" != ""; then + git submodule deinit $sub_deinit + fi + exit $status +} +trap "cleanup" 0 1 2 3 15 if git diff-index --quiet HEAD -- &>/dev/null then @@ -36,45 +45,26 @@ then else HEAD=$(git stash create) fi -git clone --shared . "$vroot_dir" -test $? -ne 0 && error "failed to clone into '$vroot_dir'" + +git archive --format tar $HEAD > "$tar_file" +test $? -ne 0 && error "failed to archive qemu" for sm in $submodules; do - if test -d "$sm/.git" - then - git clone --shared "$sm" "$vroot_dir/$sm" - test $? -ne 0 && error "failed to clone submodule $sm" - fi + status="$(git submodule status "$sm")" + smhash="${status#[ +-]}" + smhash="${smhash%% *}" + case "$status" in + -*) + sub_deinit="$sub_deinit $sm" + git submodule update --init "$sm" + test $? -ne 0 && error "failed to update submodule $sm" + ;; + +*) + echo "WARNING: submodule $sm is out of sync" + ;; + esac + (cd $sm; git archive --format tar --prefix "$sm/" $smhash) > "$sub_file" + test $? -ne 0 && error "failed to archive submodule $sm ($smhash)" + tar --concatenate --file "$tar_file" "$sub_file" + test $? -ne 0 && error "failed append submodule $sm to $tar_file" done - -cd "$vroot_dir" -test $? -ne 0 && error "failed to change into '$vroot_dir'" - -git checkout $HEAD -test $? -ne 0 && error "failed to checkout $HEAD revision" - -for sm in $submodules; do - git submodule update --init $sm - test $? -ne 0 && error "failed to init submodule $sm" -done - -if test -n "$submodules"; then - { - git ls-files || error "git ls-files failed" - for sm in $submodules; do - (cd $sm; git ls-files) | sed "s:^:$sm/:" - if test "${PIPESTATUS[*]}" != "0 0"; then - error "git ls-files in submodule $sm failed" - fi - done - } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > "$list_file" -else - git ls-files > "$list_file" -fi - -if test $? -ne 0; then - error "failed to generate list file" -fi - -tar -cf "$tar_file" -T "$list_file" || error "failed to create tar file" - exit 0 From 3ace9be6d267b2f876ebb34096fe5d9b64a82d9a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 20 May 2019 14:47:04 +0200 Subject: [PATCH 13/40] tests/vm: python3 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add proper unicode handling when processing strings. Also need to explicitly say we want int not float. Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Message-Id: <20190520124716.30472-3-kraxel@redhat.com> [AJB: fix conflicts with tests/vm: Port basevm to Python 3] Signed-off-by: Alex Bennée --- tests/vm/basevm.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 083befce9f..4847549592 100755 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -73,7 +73,7 @@ class BaseVM(object): "-vnc", "127.0.0.1:0,to=20", "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")] if vcpus and vcpus > 1: - self._args += ["-smp", str(vcpus)] + self._args += ["-smp", "%d" % vcpus] if kvm_available(self.arch): self._args += ["-enable-kvm"] else: @@ -85,12 +85,13 @@ class BaseVM(object): if not sha256sum: return True checksum = subprocess.check_output(["sha256sum", fname]).split()[0] - return sha256sum == checksum.decode() + return sha256sum == checksum.decode("utf-8") cache_dir = os.path.expanduser("~/.cache/qemu-vm/download") if not os.path.exists(cache_dir): os.makedirs(cache_dir) - fname = os.path.join(cache_dir, hashlib.sha1(url.encode()).hexdigest()) + fname = os.path.join(cache_dir, + hashlib.sha1(url.encode("utf-8")).hexdigest()) if os.path.exists(fname) and check_sha256sum(fname): return fname logging.debug("Downloading %s to %s...", url, fname) @@ -134,7 +135,7 @@ class BaseVM(object): raise NotImplementedError def add_source_dir(self, src_dir): - name = "data-" + hashlib.sha1(src_dir.encode()).hexdigest()[:5] + name = "data-" + hashlib.sha1(src_dir.encode("utf-8")).hexdigest()[:5] tarfile = os.path.join(self._tmpdir, name + ".tar") logging.debug("Creating archive %s for src_dir dir: %s", tarfile, src_dir) subprocess.check_call(["./scripts/archive-source.sh", tarfile], @@ -256,7 +257,7 @@ def main(vmcls): vm.add_source_dir(args.build_qemu) cmd = [vm.BUILD_SCRIPT.format( configure_opts = " ".join(argv), - jobs=args.jobs, + jobs=int(args.jobs), target=args.build_target, verbose = "V=1" if args.verbose else "")] else: From 78e24848f6a2923f356d15d8751c644f94a39fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 30 May 2019 15:35:14 +0100 Subject: [PATCH 14/40] semihosting: split console_out into string and char versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is ostensibly to avoid the weirdness of len looking like it might come from a guest and sometimes being used. While we are at it fix up the error checking for the arm-linux-user implementation of the API which got flagged up by Coverity (CID 1401700). Signed-off-by: Alex Bennée --- hw/semihosting/console.c | 34 +++++++++++++++++++++++--------- include/hw/semihosting/console.h | 25 +++++++++++++++++------ linux-user/arm/semihost.c | 31 ++++++++++++++++++++++++++--- target/arm/arm-semi.c | 4 ++-- 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c index 4ab7533bb8..b4b17c8afb 100644 --- a/hw/semihosting/console.c +++ b/hw/semihosting/console.c @@ -36,26 +36,24 @@ int qemu_semihosting_log_out(const char *s, int len) /* * A re-implementation of lock_user_string that we can use locally * instead of relying on softmmu-semi. Hopefully we can deprecate that - * in time. We either copy len bytes if specified or until we find a NULL. + * in time. Copy string until we find a 0 or address error. */ -static GString *copy_user_string(CPUArchState *env, target_ulong addr, int len) +static GString *copy_user_string(CPUArchState *env, target_ulong addr) { CPUState *cpu = env_cpu(env); - GString *s = g_string_sized_new(len ? len : 128); + GString *s = g_string_sized_new(128); uint8_t c; - bool done; do { if (cpu_memory_rw_debug(cpu, addr++, &c, 1, 0) == 0) { s = g_string_append_c(s, c); - done = len ? s->len == len : c == 0; } else { qemu_log_mask(LOG_GUEST_ERROR, "%s: passed inaccessible address " TARGET_FMT_lx, __func__, addr); - done = true; + break; } - } while (!done); + } while (c!=0); return s; } @@ -68,9 +66,9 @@ static void semihosting_cb(CPUState *cs, target_ulong ret, target_ulong err) } } -int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len) +int qemu_semihosting_console_outs(CPUArchState *env, target_ulong addr) { - GString *s = copy_user_string(env, addr, len); + GString *s = copy_user_string(env, addr); int out = s->len; if (use_gdb_syscalls()) { @@ -82,3 +80,21 @@ int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len) g_string_free(s, true); return out; } + +void qemu_semihosting_console_outc(CPUArchState *env, target_ulong addr) +{ + CPUState *cpu = env_cpu(env); + uint8_t c; + + if (cpu_memory_rw_debug(cpu, addr, &c, 1, 0) == 0) { + if (use_gdb_syscalls()) { + gdb_do_syscall(semihosting_cb, "write,2,%x,%x", addr, 1); + } else { + qemu_semihosting_log_out((const char *) &c, 1); + } + } else { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: passed inaccessible address " TARGET_FMT_lx, + __func__, addr); + } +} diff --git a/include/hw/semihosting/console.h b/include/hw/semihosting/console.h index 9eb45b7c53..cfab572c0c 100644 --- a/include/hw/semihosting/console.h +++ b/include/hw/semihosting/console.h @@ -10,17 +10,30 @@ #define SEMIHOST_CONSOLE_H /** - * qemu_semihosting_console_out: + * qemu_semihosting_console_outs: * @env: CPUArchState - * @s: host address of guest string - * @len: length of string or 0 (string is null terminated) + * @s: host address of null terminated guest string * - * Send a guest string to the debug console. This may be the remote - * gdb session if a softmmu guest is currently being debugged. + * Send a null terminated guest string to the debug console. This may + * be the remote gdb session if a softmmu guest is currently being + * debugged. * * Returns: number of bytes written. */ -int qemu_semihosting_console_out(CPUArchState *env, target_ulong s, int len); +int qemu_semihosting_console_outs(CPUArchState *env, target_ulong s); + +/** + * qemu_semihosting_console_outc: + * @env: CPUArchState + * @s: host address of null terminated guest string + * + * Send single character from guest memory to the debug console. This + * may be the remote gdb session if a softmmu guest is currently being + * debugged. + * + * Returns: nothing + */ +void qemu_semihosting_console_outc(CPUArchState *env, target_ulong c); /** * qemu_semihosting_log_out: diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c index 9554102a85..a16b525eec 100644 --- a/linux-user/arm/semihost.c +++ b/linux-user/arm/semihost.c @@ -15,10 +15,35 @@ #include "hw/semihosting/console.h" #include "qemu.h" -int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len) +int qemu_semihosting_console_outs(CPUArchState *env, target_ulong addr) { - void *s = lock_user_string(addr); - len = write(STDERR_FILENO, s, len ? len : strlen(s)); + int len = target_strlen(addr); + void *s; + if (len < 0){ + qemu_log_mask(LOG_GUEST_ERROR, + "%s: passed inaccessible address " TARGET_FMT_lx, + __func__, addr); + return 0; + } + s = lock_user(VERIFY_READ, addr, (long)(len + 1), 1); + g_assert(s); /* target_strlen has already verified this will work */ + len = write(STDERR_FILENO, s, len); unlock_user(s, addr, 0); return len; } + +void qemu_semihosting_console_outc(CPUArchState *env, target_ulong addr) +{ + char c; + + if (get_user_u8(c, addr)) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: passed inaccessible address " TARGET_FMT_lx, + __func__, addr); + } else { + if (write(STDERR_FILENO, &c, 1) != 1) { + qemu_log_mask(LOG_UNIMP, "%s: unexpected write to stdout failure", + __func__); + } + } +} diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index bca9a25910..90423a35de 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -314,10 +314,10 @@ target_ulong do_arm_semihosting(CPUARMState *env) return set_swi_errno(ts, close(arg0)); } case TARGET_SYS_WRITEC: - qemu_semihosting_console_out(env, args, 1); + qemu_semihosting_console_outc(env, args); return 0xdeadbeef; case TARGET_SYS_WRITE0: - return qemu_semihosting_console_out(env, args, 0); + return qemu_semihosting_console_outs(env, args); case TARGET_SYS_WRITE: GET_ARG(0); GET_ARG(1); From 8c79b288513587e960b6b7257a9d955d5592f209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Mon, 3 Jun 2019 15:56:32 +0100 Subject: [PATCH 15/40] cputlb: use uint64_t for interim values for unaligned load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running on 32 bit TCG backends a wide unaligned load ends up truncating data before returning to the guest. We specifically have the return type as uint64_t to avoid any premature truncation so we should use the same for the interim types. Fixes: https://bugs.launchpad.net/qemu/+bug/1830872 Fixes: eed5664238e Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Tested-by: Laszlo Ersek Tested-by: Igor Mammedov --- accel/tcg/cputlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index baa3eb8f92..8d6891931e 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1315,7 +1315,7 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi, && unlikely((addr & ~TARGET_PAGE_MASK) + size - 1 >= TARGET_PAGE_SIZE)) { target_ulong addr1, addr2; - tcg_target_ulong r1, r2; + uint64_t r1, r2; unsigned shift; do_unaligned_access: addr1 = addr & ~(size - 1); From fcf112317c6c5eaa816a010a701efeaebc5baf60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Mon, 3 Jun 2019 16:14:24 +0100 Subject: [PATCH 16/40] tests/tcg: better detect truncated reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we've truncated a wider read we can detect the condition earlier by looking at the number of zeros we've read. So we don't trip up on cases where we have written zeros to the start of the buffer we also ensure we only start each offset read from the right address. Signed-off-by: Alex Bennée --- tests/tcg/multiarch/system/memory.c | 36 +++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c index dc1d8a98ff..d124502d73 100644 --- a/tests/tcg/multiarch/system/memory.c +++ b/tests/tcg/multiarch/system/memory.c @@ -208,6 +208,7 @@ static bool read_test_data_u32(int offset) for (i = 0; i < max; i++) { uint8_t b1, b2, b3, b4; + int zeros = 0; word = *ptr++; b1 = word >> 24 & 0xff; @@ -215,6 +216,16 @@ static bool read_test_data_u32(int offset) b3 = word >> 8 & 0xff; b4 = word & 0xff; + zeros += (b1 == 0 ? 1 : 0); + zeros += (b2 == 0 ? 1 : 0); + zeros += (b3 == 0 ? 1 : 0); + zeros += (b4 == 0 ? 1 : 0); + if (zeros > 1) { + ml_printf("Error @ %p, more zeros than expected: %d, %d, %d, %d", + ptr - 1, b1, b2, b3, b4); + return false; + } + if ((b1 < b2 && b1 != 0) || (b2 < b3 && b2 != 0) || (b3 < b4 && b3 != 0)) { @@ -238,6 +249,7 @@ static bool read_test_data_u64(int offset) for (i = 0; i < max; i++) { uint8_t b1, b2, b3, b4, b5, b6, b7, b8; + int zeros = 0; word = *ptr++; b1 = ((uint64_t) (word >> 56)) & 0xff; @@ -249,6 +261,20 @@ static bool read_test_data_u64(int offset) b7 = (word >> 8) & 0xff; b8 = (word >> 0) & 0xff; + zeros += (b1 == 0 ? 1 : 0); + zeros += (b2 == 0 ? 1 : 0); + zeros += (b3 == 0 ? 1 : 0); + zeros += (b4 == 0 ? 1 : 0); + zeros += (b5 == 0 ? 1 : 0); + zeros += (b6 == 0 ? 1 : 0); + zeros += (b7 == 0 ? 1 : 0); + zeros += (b8 == 0 ? 1 : 0); + if (zeros > 1) { + ml_printf("Error @ %p, more zeros than expected: %d, %d, %d, %d, %d, %d, %d, %d", + ptr - 1, b1, b2, b3, b4, b5, b6, b7, b8); + return false; + } + if ((b1 < b2 && b1 != 0) || (b2 < b3 && b2 != 0) || (b3 < b4 && b3 != 0) || @@ -272,7 +298,7 @@ read_ufn read_ufns[] = { read_test_data_u16, read_test_data_u32, read_test_data_u64 }; -bool do_unsigned_reads(void) +bool do_unsigned_reads(int start_off) { int i; bool ok = true; @@ -280,11 +306,11 @@ bool do_unsigned_reads(void) for (i = 0; i < ARRAY_SIZE(read_ufns) && ok; i++) { #if CHECK_UNALIGNED int off; - for (off = 0; off < 8 && ok; off++) { + for (off = start_off; off < 8 && ok; off++) { ok = read_ufns[i](off); } #else - ok = read_ufns[i](0); + ok = read_ufns[i](start_off); #endif } @@ -298,11 +324,11 @@ static bool do_unsigned_test(init_ufn fn) int i; for (i = 0; i < 8 && ok; i++) { fn(i); - ok = do_unsigned_reads(); + ok = do_unsigned_reads(i); } #else fn(0); - return do_unsigned_reads(); + return do_unsigned_reads(0); #endif } From 2736b5cbee8aaf1af668693edacf261851565cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 4 Jun 2019 16:30:05 +0100 Subject: [PATCH 17/40] tests/tcg: clean-up VPATH/TESTS for i386 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we only run build the multiarch tests and we use a fully resolved path for the crt object we don't need the wildcard or VPATH messing about. Signed-off-by: Alex Bennée --- tests/tcg/i386/Makefile.softmmu-target | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target index e1f98177aa..e1d880f9b5 100644 --- a/tests/tcg/i386/Makefile.softmmu-target +++ b/tests/tcg/i386/Makefile.softmmu-target @@ -8,15 +8,10 @@ I386_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/i386/system X64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system -# Set search path for all sources -VPATH+=$(I386_SYSTEM_SRC) # These objects provide the basic boot code and helper functions for all tests CRT_OBJS=boot.o -X86_TEST_SRCS=$(wildcard $(I386_SYSTEM_SRC)/*.c) -X86_TESTS = $(patsubst $(I386_SYSTEM_SRC)/%.c, %, $(X86_TEST_SRCS)) - ifeq ($(TARGET_X86_64), y) CRT_PATH=$(X64_SYSTEM_SRC) LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld @@ -26,12 +21,12 @@ CRT_PATH=$(I386_SYSTEM_SRC) CFLAGS+=-m32 LINK_SCRIPT=$(I386_SYSTEM_SRC)/kernel.ld LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_i386 -# FIXME: move to common once x86_64 is bootstrapped -TESTS+=$(X86_TESTS) $(MULTIARCH_TESTS) endif CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC) LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc +TESTS+=$(MULTIARCH_TESTS) + # building head blobs .PRECIOUS: $(CRT_OBJS) From db61edad7ab9d1741922d6c62b5ff6d070e980df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 6 Mar 2019 16:44:47 +0000 Subject: [PATCH 18/40] tests/tcg/x86_64: add a PVH crt.o for x86_64 system tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of doing the full real to 64 bit dance we are attempting to leverage Xen's PVH boot spec to go from 32 bit to 64 bit. Signed-off-by: Alex Bennée --- tests/tcg/i386/Makefile.softmmu-target | 1 + tests/tcg/x86_64/system/boot.S | 277 +++++++++++++++++++++++++ tests/tcg/x86_64/system/kernel.ld | 33 +++ 3 files changed, 311 insertions(+) create mode 100644 tests/tcg/x86_64/system/boot.S create mode 100644 tests/tcg/x86_64/system/kernel.ld diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target index e1d880f9b5..0a4364868c 100644 --- a/tests/tcg/i386/Makefile.softmmu-target +++ b/tests/tcg/i386/Makefile.softmmu-target @@ -14,6 +14,7 @@ CRT_OBJS=boot.o ifeq ($(TARGET_X86_64), y) CRT_PATH=$(X64_SYSTEM_SRC) +CFLAGS=-march=x86-64 LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64 else diff --git a/tests/tcg/x86_64/system/boot.S b/tests/tcg/x86_64/system/boot.S new file mode 100644 index 0000000000..205cfbd398 --- /dev/null +++ b/tests/tcg/x86_64/system/boot.S @@ -0,0 +1,277 @@ +/* + * x86_64 boot and support code + * + * Copyright 2019 Linaro + * + * This work is licensed under the terms of the GNU GPL, version 3 or later. + * See the COPYING file in the top-level directory. + * + * Unlike the i386 version we instead use Xen's PVHVM booting header + * which should drop us automatically into 32 bit mode ready to go. I've + * nabbed bits of the Linux kernel setup to achieve this. + * + * SPDX-License-Identifier: GPL-3.0-or-later + */ + + .section .head + +#define ELFNOTE_START(name, type, flags) \ +.pushsection .note.name, flags,@note ; \ + .balign 4 ; \ + .long 2f - 1f /* namesz */ ; \ + .long 4484f - 3f /* descsz */ ; \ + .long type ; \ +1:.asciz #name ; \ +2:.balign 4 ; \ +3: + +#define ELFNOTE_END \ +4484:.balign 4 ; \ +.popsection ; + +#define ELFNOTE(name, type, desc) \ + ELFNOTE_START(name, type, "") \ + desc ; \ + ELFNOTE_END + +#define XEN_ELFNOTE_ENTRY 1 +#define XEN_ELFNOTE_HYPERCALL_PAGE 2 +#define XEN_ELFNOTE_VIRT_BASE 3 +#define XEN_ELFNOTE_PADDR_OFFSET 4 +#define XEN_ELFNOTE_PHYS32_ENTRY 18 + +#define __ASM_FORM(x) x +#define __ASM_FORM_RAW(x) x +#define __ASM_FORM_COMMA(x) x, +#define __ASM_SEL(a,b) __ASM_FORM(b) +#define __ASM_SEL_RAW(a,b) __ASM_FORM_RAW(b) +#define _ASM_PTR __ASM_SEL(.long, .quad) + + ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE, _ASM_PTR 0x100000) + ELFNOTE(Xen, XEN_ELFNOTE_ENTRY, _ASM_PTR _start) + ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY, _ASM_PTR _start) /* entry == virtbase */ + ELFNOTE(Xen, XEN_ELFNOTE_PADDR_OFFSET, _ASM_PTR 0) + + /* + * Entry point for PVH guests. + * + * Xen ABI specifies the following register state when we come here: + * + * - `ebx`: contains the physical memory address where the loader has placed + * the boot start info structure. + * - `cr0`: bit 0 (PE) must be set. All the other writeable bits are cleared. + * - `cr4`: all bits are cleared. + * - `cs `: must be a 32-bit read/execute code segment with a base of ‘0’ + * and a limit of ‘0xFFFFFFFF’. The selector value is unspecified. + * - `ds`, `es`: must be a 32-bit read/write data segment with a base of + * ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all + * unspecified. + * - `tr`: must be a 32-bit TSS (active) with a base of '0' and a limit + * of '0x67'. + * - `eflags`: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared. + * Bit 8 (TF) must be cleared. Other bits are all unspecified. + * + * All other processor registers and flag bits are unspecified. The OS is in + * charge of setting up it's own stack, GDT and IDT. + */ + .code32 + .section .text + +.global _start +_start: + cld + lgdt gdtr + + ljmp $0x8,$.Lloadcs +.Lloadcs: + mov $0x10,%eax + mov %eax,%ds + mov %eax,%es + mov %eax,%fs + mov %eax,%gs + mov %eax,%ss + + /* Enable PAE mode (bit 5). */ + mov %cr4, %eax + btsl $5, %eax + mov %eax, %cr4 + +#define MSR_EFER 0xc0000080 /* extended feature register */ + + /* Enable Long mode. */ + mov $MSR_EFER, %ecx + rdmsr + btsl $8, %eax + wrmsr + + /* Enable paging */ + mov $.Lpml4, %ecx + mov %ecx, %cr3 + + mov %cr0, %eax + btsl $31, %eax + mov %eax, %cr0 + + /* Jump to 64-bit mode. */ + lgdt gdtr64 + ljmp $0x8,$.Lenter64 + + .code64 + .section .text +.Lenter64: + + + // Setup stack ASAP + movq $stack_end,%rsp + + /* 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 */ + test %al, %al + jz 1f + out %ax, $0xf4 + +1: /* QEMU ACPI poweroff */ + mov $0x604,%edx + mov $0x2000,%eax + out %ax,%dx + hlt + jmp 1b + + /* + * Helper Functions + * + * x86_64 calling convention is rdi, rsi, rdx, rcx, r8, r9 + */ + + /* Output a single character to serial port */ + .global __sys_outc +__sys_outc: + pushq %rax + mov %rax, %rdx + out %al,$0xE9 + popq %rax + ret + + /* Interrupt Descriptor Table */ + + .section .data + .align 16 + +idt_00: .int 0, 0 +idt_01: .int 0, 0 +idt_02: .int 0, 0 +idt_03: .int 0, 0 +idt_04: .int 0, 0 +idt_05: .int 0, 0 +idt_06: .int 0, 0 /* intr_6_opcode, Invalid Opcode */ +idt_07: .int 0, 0 +idt_08: .int 0, 0 +idt_09: .int 0, 0 +idt_0A: .int 0, 0 +idt_0B: .int 0, 0 +idt_0C: .int 0, 0 +idt_0D: .int 0, 0 +idt_0E: .int 0, 0 +idt_0F: .int 0, 0 +idt_10: .int 0, 0 +idt_11: .int 0, 0 +idt_12: .int 0, 0 +idt_13: .int 0, 0 +idt_14: .int 0, 0 +idt_15: .int 0, 0 +idt_16: .int 0, 0 +idt_17: .int 0, 0 +idt_18: .int 0, 0 +idt_19: .int 0, 0 +idt_1A: .int 0, 0 +idt_1B: .int 0, 0 +idt_1C: .int 0, 0 +idt_1D: .int 0, 0 +idt_1E: .int 0, 0 +idt_1F: .int 0, 0 + + + /* + * Global Descriptor Table (GDT) + * + * This describes various memory areas (segments) through + * segment descriptors. In 32 bit mode each segment each + * segement is associated with segment registers which are + * implicitly (or explicitly) referenced depending on the + * instruction. However in 64 bit mode selectors are flat and + * segmented addressing isn't used. + */ +gdt: + .short 0 +gdtr: + .short gdt_en - gdt - 1 + .int gdt + + // Code cs: + .short 0xFFFF + .short 0 + .byte 0 + .byte 0x9b + .byte 0xCF + .byte 0 + + // Data ds:, ss:, es:, fs:, and gs: + .short 0xFFFF + .short 0 + .byte 0 + .byte 0x93 + .byte 0xCF + .byte 0 +gdt_en: + +gdt64: + .short 0 +gdtr64: + .short gdt64_en - gdt64 - 1 + .int gdt64 + + // Code + .short 0xFFFF + .short 0 + .byte 0 + .byte 0x9b + .byte 0xAF + .byte 0 + + // Data + .short 0xFFFF + .short 0 + .byte 0 + .byte 0x93 + .byte 0xCF + .byte 0 +gdt64_en: + + .section .bss + .align 16 + +stack: .space 65536 +stack_end: + + .section .data + +.align 4096 +.Lpd: +i = 0 + .rept 512 * 4 + .quad 0x1e7 | (i << 21) + i = i + 1 + .endr + +.align 4096 +.Lpdp: + .quad .Lpd + 7 + 0 * 4096 /* 0-1 GB */ + .quad .Lpd + 7 + 1 * 4096 /* 1-2 GB */ + .quad .Lpd + 7 + 2 * 4096 /* 2-3 GB */ + .quad .Lpd + 7 + 3 * 4096 /* 3-4 GB */ + +.align 4096 +.Lpml4: + .quad .Lpdp + 7 /* 0-512 GB */ diff --git a/tests/tcg/x86_64/system/kernel.ld b/tests/tcg/x86_64/system/kernel.ld new file mode 100644 index 0000000000..49c12b04ae --- /dev/null +++ b/tests/tcg/x86_64/system/kernel.ld @@ -0,0 +1,33 @@ +PHDRS { + text PT_LOAD FLAGS(5); /* R_E */ + note PT_NOTE FLAGS(0); /* ___ */ +} + +SECTIONS { + . = 0x100000; + + .text : { + __load_st = .; + *(.head) + *(.text) + } :text + + .rodata : { + *(.rodata) + } :text + + /* Keep build ID and PVH notes in same section */ + .notes : { + *(.note.*) + } :note + + .data : { + *(.data) + __load_en = .; + } :text + + .bss : { + *(.bss) + __bss_en = .; + } +} From c7b3e866bbe361a8637cb20efea8a6839a719ae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 6 Jun 2019 09:27:53 +0100 Subject: [PATCH 19/40] MAINTAINERS: put myself forward for gdbstub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As I've been reviewing a lot of this recently and I'm going to put together a pull request I'd better keep an eye on it. Philippe has also volunteered to be a reviewer. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé --- MAINTAINERS | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 588c8d947a..acbad134ec 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1865,7 +1865,9 @@ F: util/error.c F: util/qemu-error.c GDB stub -S: Orphan +M: Alex Bennée +R: Philippe Mathieu-Daudé +S: Maintained F: gdbstub* F: gdb-xml/ From ab7a2009df66241a3742cbdfe8f9a1f66c6af21f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 6 Jun 2019 16:38:19 +0100 Subject: [PATCH 20/40] cputlb: cast size_t to target_ulong before using for address masks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While size_t is defined to happily access the biggest host object this isn't the case when generating masks for 64 bit guests on 32 bit hosts. Otherwise we end up truncating the address when we fall back to our unaligned helper. Fixes: https://bugs.launchpad.net/qemu/+bug/1831545 Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Tested-by: Andrew Randrianasulu Reviewed-by: Philippe Mathieu-Daudé --- accel/tcg/cputlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 8d6891931e..bb9897b25a 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1318,7 +1318,7 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi, uint64_t r1, r2; unsigned shift; do_unaligned_access: - addr1 = addr & ~(size - 1); + addr1 = addr & ~((target_ulong)size - 1); addr2 = addr1 + size; r1 = full_load(env, addr1, oi, retaddr); r2 = full_load(env, addr2, oi, retaddr); From d14055dc69dfe4f53e3af9a93126382a40da3366 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:29 +0300 Subject: [PATCH 21/40] gdbstub: Add infrastructure to parse cmd packets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-2-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) diff --git a/gdbstub.c b/gdbstub.c index 17dcd9186d..22ee2ded6f 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1272,6 +1272,201 @@ out: return res; } +typedef union GdbCmdVariant { + const char *data; + uint8_t opcode; + unsigned long val_ul; + unsigned long long val_ull; + struct { + GDBThreadIdKind kind; + uint32_t pid; + uint32_t tid; + } thread_id; +} GdbCmdVariant; + +static const char *cmd_next_param(const char *param, const char delimiter) +{ + static const char all_delimiters[] = ",;:="; + char curr_delimiters[2] = {0}; + const char *delimiters; + + if (delimiter == '?') { + delimiters = all_delimiters; + } else if (delimiter == '0') { + return strchr(param, '\0'); + } else if (delimiter == '.' && *param) { + return param + 1; + } else { + curr_delimiters[0] = delimiter; + delimiters = curr_delimiters; + } + + param += strcspn(param, delimiters); + if (*param) { + param++; + } + return param; +} + +static int cmd_parse_params(const char *data, const char *schema, + GdbCmdVariant *params, int *num_params) +{ + int curr_param; + const char *curr_schema, *curr_data; + + *num_params = 0; + + if (!schema) { + return 0; + } + + curr_schema = schema; + curr_param = 0; + curr_data = data; + while (curr_schema[0] && curr_schema[1] && *curr_data) { + switch (curr_schema[0]) { + case 'l': + if (qemu_strtoul(curr_data, &curr_data, 16, + ¶ms[curr_param].val_ul)) { + return -EINVAL; + } + curr_param++; + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + case 'L': + if (qemu_strtou64(curr_data, &curr_data, 16, + (uint64_t *)¶ms[curr_param].val_ull)) { + return -EINVAL; + } + curr_param++; + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + case 's': + params[curr_param].data = curr_data; + curr_param++; + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + case 'o': + params[curr_param].opcode = *(uint8_t *)curr_data; + curr_param++; + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + case 't': + params[curr_param].thread_id.kind = + read_thread_id(curr_data, &curr_data, + ¶ms[curr_param].thread_id.pid, + ¶ms[curr_param].thread_id.tid); + curr_param++; + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + case '?': + curr_data = cmd_next_param(curr_data, curr_schema[1]); + break; + default: + return -EINVAL; + } + curr_schema += 2; + } + + *num_params = curr_param; + return 0; +} + +typedef struct GdbCmdContext { + GDBState *s; + GdbCmdVariant *params; + int num_params; + uint8_t mem_buf[MAX_PACKET_LENGTH]; + char str_buf[MAX_PACKET_LENGTH + 1]; +} GdbCmdContext; + +typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx); + +/* + * cmd_startswith -> cmd is compared using startswith + * + * + * schema definitions: + * Each schema parameter entry consists of 2 chars, + * the first char represents the parameter type handling + * the second char represents the delimiter for the next parameter + * + * Currently supported schema types: + * 'l' -> unsigned long (stored in .val_ul) + * 'L' -> unsigned long long (stored in .val_ull) + * 's' -> string (stored in .data) + * 'o' -> single char (stored in .opcode) + * 't' -> thread id (stored in .thread_id) + * '?' -> skip according to delimiter + * + * Currently supported delimiters: + * '?' -> Stop at any delimiter (",;:=\0") + * '0' -> Stop at "\0" + * '.' -> Skip 1 char unless reached "\0" + * Any other value is treated as the delimiter value itself + */ +typedef struct GdbCmdParseEntry { + GdbCmdHandler handler; + const char *cmd; + bool cmd_startswith; + const char *schema; +} GdbCmdParseEntry; + +static inline int startswith(const char *string, const char *pattern) +{ + return !strncmp(string, pattern, strlen(pattern)); +} + +static int process_string_cmd( + GDBState *s, void *user_ctx, const char *data, + const GdbCmdParseEntry *cmds, int num_cmds) + __attribute__((unused)); + +static int process_string_cmd(GDBState *s, void *user_ctx, const char *data, + const GdbCmdParseEntry *cmds, int num_cmds) +{ + int i, schema_len, max_num_params = 0; + GdbCmdContext gdb_ctx; + + if (!cmds) { + return -1; + } + + for (i = 0; i < num_cmds; i++) { + const GdbCmdParseEntry *cmd = &cmds[i]; + g_assert(cmd->handler && cmd->cmd); + + if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) || + (!cmd->cmd_startswith && strcmp(cmd->cmd, data))) { + continue; + } + + if (cmd->schema) { + schema_len = strlen(cmd->schema); + if (schema_len % 2) { + return -2; + } + + max_num_params = schema_len / 2; + } + + gdb_ctx.params = + (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params); + memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params); + + if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema, + gdb_ctx.params, &gdb_ctx.num_params)) { + return -1; + } + + gdb_ctx.s = s; + cmd->handler(&gdb_ctx, user_ctx); + return 0; + } + + return -1; +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; From 3e2c12615b52eb5e1cae7f448a02da622179fc43 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:30 +0300 Subject: [PATCH 22/40] gdbstub: Implement deatch (D pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-3-arilou@gmail.com> Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée --- gdbstub.c | 101 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 22ee2ded6f..907bff6c07 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1417,11 +1417,6 @@ static inline int startswith(const char *string, const char *pattern) return !strncmp(string, pattern, strlen(pattern)); } -static int process_string_cmd( - GDBState *s, void *user_ctx, const char *data, - const GdbCmdParseEntry *cmds, int num_cmds) - __attribute__((unused)); - static int process_string_cmd(GDBState *s, void *user_ctx, const char *data, const GdbCmdParseEntry *cmds, int num_cmds) { @@ -1467,6 +1462,55 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data, return -1; } +static void run_cmd_parser(GDBState *s, const char *data, + const GdbCmdParseEntry *cmd) +{ + if (!data) { + return; + } + + /* In case there was an error during the command parsing we must + * send a NULL packet to indicate the command is not supported */ + if (process_string_cmd(s, NULL, data, cmd, 1)) { + put_packet(s, ""); + } +} + +static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + GDBProcess *process; + GDBState *s = gdb_ctx->s; + uint32_t pid = 1; + + if (s->multiprocess) { + if (!gdb_ctx->num_params) { + put_packet(s, "E22"); + return; + } + + pid = gdb_ctx->params[0].val_ul; + } + + process = gdb_get_process(s, pid); + gdb_process_breakpoint_remove_all(s, process); + process->attached = false; + + if (pid == gdb_get_cpu_pid(s, s->c_cpu)) { + s->c_cpu = gdb_first_attached_cpu(s); + } + + if (pid == gdb_get_cpu_pid(s, s->g_cpu)) { + s->g_cpu = gdb_first_attached_cpu(s); + } + + if (!s->c_cpu) { + /* No more process attached */ + gdb_syscall_mode = GDB_SYS_DISABLED; + gdb_continue(s); + } + put_packet(s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1481,6 +1525,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) uint8_t *registers; target_ulong addr, len; GDBThreadIdKind thread_kind; + const GdbCmdParseEntry *cmd_parser = NULL; trace_gdbstub_io_command(line_buf); @@ -1581,42 +1626,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) error_report("QEMU: Terminated via GDBstub"); exit(0); case 'D': - /* Detach packet */ - pid = 1; - - if (s->multiprocess) { - unsigned long lpid; - if (*p != ';') { - put_packet(s, "E22"); - break; - } - - if (qemu_strtoul(p + 1, &p, 16, &lpid)) { - put_packet(s, "E22"); - break; - } - - pid = lpid; + { + static const GdbCmdParseEntry detach_cmd_desc = { + .handler = handle_detach, + .cmd = "D", + .cmd_startswith = 1, + .schema = "?.l0" + }; + cmd_parser = &detach_cmd_desc; } - - process = gdb_get_process(s, pid); - gdb_process_breakpoint_remove_all(s, process); - process->attached = false; - - if (pid == gdb_get_cpu_pid(s, s->c_cpu)) { - s->c_cpu = gdb_first_attached_cpu(s); - } - - if (pid == gdb_get_cpu_pid(s, s->g_cpu)) { - s->g_cpu = gdb_first_attached_cpu(s); - } - - if (s->c_cpu == NULL) { - /* No more process attached */ - gdb_syscall_mode = GDB_SYS_DISABLED; - gdb_continue(s); - } - put_packet(s, "OK"); break; case 's': if (*p != '\0') { @@ -1989,6 +2007,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, buf); break; } + + run_cmd_parser(s, line_buf, cmd_parser); + return RS_IDLE; } From 44ffded013b193414f45bc0cb5df74ab7174b8bc Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:31 +0300 Subject: [PATCH 23/40] gdbstub: Implement thread_alive (T pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-4-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 907bff6c07..89d5dcf07a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1511,6 +1511,30 @@ static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(s, "OK"); } +static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + CPUState *cpu; + + if (!gdb_ctx->num_params) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + if (gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid, + gdb_ctx->params[0].thread_id.tid); + if (!cpu) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + put_packet(gdb_ctx->s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1811,17 +1835,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'T': - thread_kind = read_thread_id(p, &p, &pid, &tid); - if (thread_kind == GDB_READ_THREAD_ERR) { - put_packet(s, "E22"); - break; - } - cpu = gdb_get_cpu(s, pid, tid); - - if (cpu != NULL) { - put_packet(s, "OK"); - } else { - put_packet(s, "E22"); + { + static const GdbCmdParseEntry thread_alive_cmd_desc = { + .handler = handle_thread_alive, + .cmd = "T", + .cmd_startswith = 1, + .schema = "t0" + }; + cmd_parser = &thread_alive_cmd_desc; } break; case 'q': From 4d6e3fe2794f6b586d59dfe26ce3969e7a7cf36a Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:32 +0300 Subject: [PATCH 24/40] gdbstub: Implement continue (c pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-5-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 89d5dcf07a..53149c565a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1535,6 +1535,16 @@ static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "OK"); } +static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (gdb_ctx->num_params) { + gdb_set_cpu_pc(gdb_ctx->s, gdb_ctx->params[0].val_ull); + } + + gdb_ctx->s->signal = 0; + gdb_continue(gdb_ctx->s); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1571,13 +1581,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) gdb_breakpoint_remove_all(); break; case 'c': - if (*p != '\0') { - addr = strtoull(p, (char **)&p, 16); - gdb_set_cpu_pc(s, addr); + { + static const GdbCmdParseEntry continue_cmd_desc = { + .handler = handle_continue, + .cmd = "c", + .cmd_startswith = 1, + .schema = "L0" + }; + cmd_parser = &continue_cmd_desc; } - s->signal = 0; - gdb_continue(s); - return RS_IDLE; + break; case 'C': s->signal = gdb_signal_to_target (strtoul(p, (char **)&p, 16)); if (s->signal == -1) From ccc47d5d01a99d2eaa7fc4f10f78dde844c7d573 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:33 +0300 Subject: [PATCH 25/40] gdbstub: Implement continue with signal (C pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-6-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 53149c565a..25ee936a8c 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1545,6 +1545,25 @@ static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(gdb_ctx->s); } +static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + unsigned long signal = 0; + + /* + * Note: C sig;[addr] is currently unsupported and we simply + * omit the addr parameter + */ + if (gdb_ctx->num_params) { + signal = gdb_ctx->params[0].val_ul; + } + + gdb_ctx->s->signal = gdb_signal_to_target(signal); + if (gdb_ctx->s->signal == -1) { + gdb_ctx->s->signal = 0; + } + gdb_continue(gdb_ctx->s); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1592,11 +1611,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'C': - s->signal = gdb_signal_to_target (strtoul(p, (char **)&p, 16)); - if (s->signal == -1) - s->signal = 0; - gdb_continue(s); - return RS_IDLE; + { + static const GdbCmdParseEntry cont_with_sig_cmd_desc = { + .handler = handle_cont_with_sig, + .cmd = "C", + .cmd_startswith = 1, + .schema = "l0" + }; + cmd_parser = &cont_with_sig_cmd_desc; + } + break; case 'v': if (strncmp(p, "Cont", 4) == 0) { p += 4; From 3a9651d6743778a6d0968d2a56688c98769fbda9 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:34 +0300 Subject: [PATCH 26/40] gdbstub: Implement set_thread (H pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-7-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 83 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 25ee936a8c..aad43c075b 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1564,6 +1564,51 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(gdb_ctx->s); } +static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + CPUState *cpu; + + if (gdb_ctx->num_params != 2) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + if (gdb_ctx->params[1].thread_id.kind == GDB_READ_THREAD_ERR) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + if (gdb_ctx->params[1].thread_id.kind != GDB_ONE_THREAD) { + put_packet(gdb_ctx->s, "OK"); + return; + } + + cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[1].thread_id.pid, + gdb_ctx->params[1].thread_id.tid); + if (!cpu) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + /* + * Note: This command is deprecated and modern gdb's will be using the + * vCont command instead. + */ + switch (gdb_ctx->params[0].opcode) { + case 'c': + gdb_ctx->s->c_cpu = cpu; + put_packet(gdb_ctx->s, "OK"); + break; + case 'g': + gdb_ctx->s->g_cpu = cpu; + put_packet(gdb_ctx->s, "OK"); + break; + default: + put_packet(gdb_ctx->s, "E22"); + break; + } +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1577,7 +1622,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) char thread_id[16]; uint8_t *registers; target_ulong addr, len; - GDBThreadIdKind thread_kind; const GdbCmdParseEntry *cmd_parser = NULL; trace_gdbstub_io_command(line_buf); @@ -1840,35 +1884,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "E22"); break; case 'H': - type = *p++; - - thread_kind = read_thread_id(p, &p, &pid, &tid); - if (thread_kind == GDB_READ_THREAD_ERR) { - put_packet(s, "E22"); - break; - } - - if (thread_kind != GDB_ONE_THREAD) { - put_packet(s, "OK"); - break; - } - cpu = gdb_get_cpu(s, pid, tid); - if (cpu == NULL) { - put_packet(s, "E22"); - break; - } - switch (type) { - case 'c': - s->c_cpu = cpu; - put_packet(s, "OK"); - break; - case 'g': - s->g_cpu = cpu; - put_packet(s, "OK"); - break; - default: - put_packet(s, "E22"); - break; + { + static const GdbCmdParseEntry set_thread_cmd_desc = { + .handler = handle_set_thread, + .cmd = "H", + .cmd_startswith = 1, + .schema = "o.t0" + }; + cmd_parser = &set_thread_cmd_desc; } break; case 'T': From 77f6ce500f1950b9e07851ce66cde2d068e1a41d Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:35 +0300 Subject: [PATCH 27/40] gdbstub: Implement breakpoint commands (Z/z pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-8-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 86 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index aad43c075b..50e95a5663 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -954,7 +954,7 @@ static inline int xlat_gdb_type(CPUState *cpu, int gdbtype) } #endif -static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) +static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong len) { CPUState *cpu; int err = 0; @@ -991,7 +991,7 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) } } -static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type) +static int gdb_breakpoint_remove(int type, target_ulong addr, target_ulong len) { CPUState *cpu; int err = 0; @@ -1609,6 +1609,52 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx) } } +static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int res; + + if (gdb_ctx->num_params != 3) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + res = gdb_breakpoint_insert(gdb_ctx->params[0].val_ul, + gdb_ctx->params[1].val_ull, + gdb_ctx->params[2].val_ull); + if (res >= 0) { + put_packet(gdb_ctx->s, "OK"); + return; + } else if (res == -ENOSYS) { + put_packet(gdb_ctx->s, ""); + return; + } + + put_packet(gdb_ctx->s, "E22"); +} + +static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int res; + + if (gdb_ctx->num_params != 3) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + res = gdb_breakpoint_remove(gdb_ctx->params[0].val_ul, + gdb_ctx->params[1].val_ull, + gdb_ctx->params[2].val_ull); + if (res >= 0) { + put_packet(gdb_ctx->s, "OK"); + return; + } else if (res == -ENOSYS) { + put_packet(gdb_ctx->s, ""); + return; + } + + put_packet(gdb_ctx->s, "E22"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1864,24 +1910,26 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "OK"); break; case 'Z': + { + static const GdbCmdParseEntry insert_bp_cmd_desc = { + .handler = handle_insert_bp, + .cmd = "Z", + .cmd_startswith = 1, + .schema = "l?L?L0" + }; + cmd_parser = &insert_bp_cmd_desc; + } + break; case 'z': - type = strtoul(p, (char **)&p, 16); - if (*p == ',') - p++; - addr = strtoull(p, (char **)&p, 16); - if (*p == ',') - p++; - len = strtoull(p, (char **)&p, 16); - if (ch == 'Z') - res = gdb_breakpoint_insert(addr, len, type); - else - res = gdb_breakpoint_remove(addr, len, type); - if (res >= 0) - put_packet(s, "OK"); - else if (res == -ENOSYS) - put_packet(s, ""); - else - put_packet(s, "E22"); + { + static const GdbCmdParseEntry remove_bp_cmd_desc = { + .handler = handle_remove_bp, + .cmd = "z", + .cmd_startswith = 1, + .schema = "l?L?L0" + }; + cmd_parser = &remove_bp_cmd_desc; + } break; case 'H': { From 62b3320bddd79c050553ea7f81f20c6d3b401ce3 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:36 +0300 Subject: [PATCH 28/40] gdbstub: Implement set register (P pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-9-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 50e95a5663..6ec87e169a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1655,6 +1655,27 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "E22"); } +static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int reg_size; + + if (!gdb_has_xml) { + put_packet(gdb_ctx->s, "E00"); + return; + } + + if (gdb_ctx->num_params != 2) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + reg_size = strlen(gdb_ctx->params[1].data) / 2; + hextomem(gdb_ctx->mem_buf, gdb_ctx->params[1].data, reg_size); + gdb_write_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf, + gdb_ctx->params[0].val_ull); + put_packet(gdb_ctx->s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1899,15 +1920,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'P': - if (!gdb_has_xml) - goto unknown_command; - addr = strtoull(p, (char **)&p, 16); - if (*p == '=') - p++; - reg_size = strlen(p) / 2; - hextomem(mem_buf, p, reg_size); - gdb_write_register(s->g_cpu, mem_buf, addr); - put_packet(s, "OK"); + { + static const GdbCmdParseEntry set_reg_cmd_desc = { + .handler = handle_set_reg, + .cmd = "P", + .cmd_startswith = 1, + .schema = "L?s0" + }; + cmd_parser = &set_reg_cmd_desc; + } break; case 'Z': { From 5d0e57bd6889af947027da25ef6dd7b772ebe5b9 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:37 +0300 Subject: [PATCH 29/40] gdbstub: Implement get register (p pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-10-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 50 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 6ec87e169a..e9b23ff3f9 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1676,6 +1676,36 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "OK"); } +static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int reg_size; + + /* + * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. + * This works, but can be very slow. Anything new enough to + * understand XML also knows how to use this properly. + */ + if (!gdb_has_xml) { + put_packet(gdb_ctx->s, ""); + return; + } + + if (!gdb_ctx->num_params) { + put_packet(gdb_ctx->s, "E14"); + return; + } + + reg_size = gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf, + gdb_ctx->params[0].val_ull); + if (!reg_size) { + put_packet(gdb_ctx->s, "E14"); + return; + } + + memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1905,18 +1935,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'p': - /* Older gdb are really dumb, and don't use 'g' if 'p' is avaialable. - This works, but can be very slow. Anything new enough to - understand XML also knows how to use this properly. */ - if (!gdb_has_xml) - goto unknown_command; - addr = strtoull(p, (char **)&p, 16); - reg_size = gdb_read_register(s->g_cpu, mem_buf, addr); - if (reg_size) { - memtohex(buf, mem_buf, reg_size); - put_packet(s, buf); - } else { - put_packet(s, "E14"); + { + static const GdbCmdParseEntry get_reg_cmd_desc = { + .handler = handle_get_reg, + .cmd = "p", + .cmd_startswith = 1, + .schema = "L0" + }; + cmd_parser = &get_reg_cmd_desc; } break; case 'P': From cc0ecc7890b7b7ec8cf1f2f9e5a21f04d18aa0f9 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:38 +0300 Subject: [PATCH 30/40] gdbstub: Implement write memory (M pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-11-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index e9b23ff3f9..230a0c7294 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1706,6 +1706,31 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, gdb_ctx->str_buf); } +static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (gdb_ctx->num_params != 3) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + /* hextomem() reads 2*len bytes */ + if (gdb_ctx->params[1].val_ull > strlen(gdb_ctx->params[2].data) / 2) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + hextomem(gdb_ctx->mem_buf, gdb_ctx->params[2].data, + gdb_ctx->params[1].val_ull); + if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull, + gdb_ctx->mem_buf, + gdb_ctx->params[1].val_ull, true)) { + put_packet(gdb_ctx->s, "E14"); + return; + } + + put_packet(gdb_ctx->s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1914,24 +1939,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'M': - addr = strtoull(p, (char **)&p, 16); - if (*p == ',') - p++; - len = strtoull(p, (char **)&p, 16); - if (*p == ':') - p++; - - /* hextomem() reads 2*len bytes */ - if (len > strlen(p) / 2) { - put_packet (s, "E22"); - break; - } - hextomem(mem_buf, p, len); - if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, - true) != 0) { - put_packet(s, "E14"); - } else { - put_packet(s, "OK"); + { + static const GdbCmdParseEntry write_mem_cmd_desc = { + .handler = handle_write_mem, + .cmd = "M", + .cmd_startswith = 1, + .schema = "L,L:s0" + }; + cmd_parser = &write_mem_cmd_desc; } break; case 'p': From da92e2360e42ef8fd9dadcd2dc2a3b846d883800 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:39 +0300 Subject: [PATCH 31/40] gdbstub: Implement read memory (m pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-12-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 48 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 230a0c7294..291349854a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1731,6 +1731,30 @@ static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "OK"); } +static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (gdb_ctx->num_params != 2) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + /* memtohex() doubles the required space */ + if (gdb_ctx->params[1].val_ull > MAX_PACKET_LENGTH / 2) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull, + gdb_ctx->mem_buf, + gdb_ctx->params[1].val_ull, false)) { + put_packet(gdb_ctx->s, "E14"); + return; + } + + memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1920,22 +1944,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "OK"); break; case 'm': - addr = strtoull(p, (char **)&p, 16); - if (*p == ',') - p++; - len = strtoull(p, NULL, 16); - - /* memtohex() doubles the required space */ - if (len > MAX_PACKET_LENGTH / 2) { - put_packet (s, "E22"); - break; - } - - if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, false) != 0) { - put_packet (s, "E14"); - } else { - memtohex(buf, mem_buf, len); - put_packet(s, buf); + { + static const GdbCmdParseEntry read_mem_cmd_desc = { + .handler = handle_read_mem, + .cmd = "m", + .cmd_startswith = 1, + .schema = "L,L0" + }; + cmd_parser = &read_mem_cmd_desc; } break; case 'M': From 287ca120bdf918b324b1fc374363ed400f143806 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:40 +0300 Subject: [PATCH 32/40] gdbstub: Implement write all registers (G pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-13-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 291349854a..bfa4177d45 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1755,6 +1755,29 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, gdb_ctx->str_buf); } +static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + target_ulong addr, len; + uint8_t *registers; + int reg_size; + + if (!gdb_ctx->num_params) { + return; + } + + cpu_synchronize_state(gdb_ctx->s->g_cpu); + registers = gdb_ctx->mem_buf; + len = strlen(gdb_ctx->params[0].data) / 2; + hextomem(registers, gdb_ctx->params[0].data, len); + for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs && len > 0; + addr++) { + reg_size = gdb_write_register(gdb_ctx->s->g_cpu, registers, addr); + len -= reg_size; + registers += reg_size; + } + put_packet(gdb_ctx->s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1766,7 +1789,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; char thread_id[16]; - uint8_t *registers; target_ulong addr, len; const GdbCmdParseEntry *cmd_parser = NULL; @@ -1932,16 +1954,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, buf); break; case 'G': - cpu_synchronize_state(s->g_cpu); - registers = mem_buf; - len = strlen(p) / 2; - hextomem((uint8_t *)registers, p, len); - for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) { - reg_size = gdb_write_register(s->g_cpu, registers, addr); - len -= reg_size; - registers += reg_size; + { + static const GdbCmdParseEntry write_all_regs_cmd_desc = { + .handler = handle_write_all_regs, + .cmd = "G", + .cmd_startswith = 1, + .schema = "s0" + }; + cmd_parser = &write_all_regs_cmd_desc; } - put_packet(s, "OK"); break; case 'm': { From 397d137046401920ee61050d21fada3600f0a14f Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:41 +0300 Subject: [PATCH 33/40] gdbstub: Implement read all registers (g pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-14-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index bfa4177d45..d9b74be920 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1778,6 +1778,21 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, "OK"); } +static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + target_ulong addr, len; + + cpu_synchronize_state(gdb_ctx->s->g_cpu); + len = 0; + for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs; addr++) { + len += gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf + len, + addr); + } + + memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1785,7 +1800,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) CPUClass *cc; const char *p; uint32_t pid, tid; - int ch, reg_size, type, res; + int ch, type, res; uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; char thread_id[16]; @@ -1944,14 +1959,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'g': - cpu_synchronize_state(s->g_cpu); - len = 0; - for (addr = 0; addr < s->g_cpu->gdb_num_g_regs; addr++) { - reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr); - len += reg_size; + { + static const GdbCmdParseEntry read_all_regs_cmd_desc = { + .handler = handle_read_all_regs, + .cmd = "g", + .cmd_startswith = 1 + }; + cmd_parser = &read_all_regs_cmd_desc; } - memtohex(buf, mem_buf, len); - put_packet(s, buf); break; case 'G': { From 4b20fab101b9e2d0fb47454209637a17fc7a13d5 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:42 +0300 Subject: [PATCH 34/40] gdbstub: Implement file io (F pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-15-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 48 ++++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index d9b74be920..d1969d5977 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1793,6 +1793,25 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, gdb_ctx->str_buf); } +static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) { + target_ulong ret, err; + + ret = (target_ulong)gdb_ctx->params[0].val_ull; + err = (target_ulong)gdb_ctx->params[1].val_ull; + gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err); + gdb_ctx->s->current_syscall_cb = NULL; + } + + if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') { + put_packet(gdb_ctx->s, "T02"); + return; + } + + gdb_continue(gdb_ctx->s); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1934,28 +1953,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) return RS_IDLE; case 'F': { - target_ulong ret; - target_ulong err; - - ret = strtoull(p, (char **)&p, 16); - if (*p == ',') { - p++; - err = strtoull(p, (char **)&p, 16); - } else { - err = 0; - } - if (*p == ',') - p++; - type = *p; - if (s->current_syscall_cb) { - s->current_syscall_cb(s->c_cpu, ret, err); - s->current_syscall_cb = NULL; - } - if (type == 'C') { - put_packet(s, "T02"); - } else { - gdb_continue(s); - } + static const GdbCmdParseEntry file_io_cmd_desc = { + .handler = handle_file_io, + .cmd = "F", + .cmd_startswith = 1, + .schema = "L,L,o0" + }; + cmd_parser = &file_io_cmd_desc; } break; case 'g': From 933f80dd4226155e263635b0e909836151a795b4 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:43 +0300 Subject: [PATCH 35/40] gdbstub: Implement step (s pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-16-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index d1969d5977..b457f2841d 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1812,6 +1812,16 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(gdb_ctx->s); } +static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (gdb_ctx->num_params) { + gdb_set_cpu_pc(gdb_ctx->s, (target_ulong)gdb_ctx->params[0].val_ull); + } + + cpu_single_step(gdb_ctx->s->c_cpu, sstep_flags); + gdb_continue(gdb_ctx->s); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1944,13 +1954,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 's': - if (*p != '\0') { - addr = strtoull(p, (char **)&p, 16); - gdb_set_cpu_pc(s, addr); + { + static const GdbCmdParseEntry step_cmd_desc = { + .handler = handle_step, + .cmd = "s", + .cmd_startswith = 1, + .schema = "L0" + }; + cmd_parser = &step_cmd_desc; } - cpu_single_step(s->c_cpu, sstep_flags); - gdb_continue(s); - return RS_IDLE; + break; case 'F': { static const GdbCmdParseEntry file_io_cmd_desc = { From 8536ec02fe0659bfa943ede9cbdb9ea54b66f674 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:44 +0300 Subject: [PATCH 36/40] gdbstub: Implement v commands with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-17-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 170 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 110 insertions(+), 60 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index b457f2841d..8864c3a392 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1822,6 +1822,106 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(gdb_ctx->s); } +static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + put_packet(gdb_ctx->s, "vCont;c;C;s;S"); +} + +static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int res; + + if (!gdb_ctx->num_params) { + return; + } + + res = gdb_handle_vcont(gdb_ctx->s, gdb_ctx->params[0].data); + if ((res == -EINVAL) || (res == -ERANGE)) { + put_packet(gdb_ctx->s, "E22"); + } else if (res) { + put_packet(gdb_ctx->s, ""); + } +} + +static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + GDBProcess *process; + CPUState *cpu; + char thread_id[16]; + + pstrcpy(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "E22"); + if (!gdb_ctx->num_params) { + goto cleanup; + } + + process = gdb_get_process(gdb_ctx->s, gdb_ctx->params[0].val_ul); + if (!process) { + goto cleanup; + } + + cpu = get_first_cpu_in_process(gdb_ctx->s, process); + if (!cpu) { + goto cleanup; + } + + process->attached = true; + gdb_ctx->s->g_cpu = cpu; + gdb_ctx->s->c_cpu = cpu; + + gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id)); + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;", + GDB_SIGNAL_TRAP, thread_id); +cleanup: + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + /* Kill the target */ + put_packet(gdb_ctx->s, "OK"); + error_report("QEMU: Terminated via GDBstub"); + exit(0); +} + +static GdbCmdParseEntry gdb_v_commands_table[] = { + /* Order is important if has same prefix */ + { + .handler = handle_v_cont_query, + .cmd = "Cont?", + .cmd_startswith = 1 + }, + { + .handler = handle_v_cont, + .cmd = "Cont", + .cmd_startswith = 1, + .schema = "s0" + }, + { + .handler = handle_v_attach, + .cmd = "Attach;", + .cmd_startswith = 1, + .schema = "l0" + }, + { + .handler = handle_v_kill, + .cmd = "Kill;", + .cmd_startswith = 1 + }, +}; + +static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (!gdb_ctx->num_params) { + return; + } + + if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data, + gdb_v_commands_table, + ARRAY_SIZE(gdb_v_commands_table))) { + put_packet(gdb_ctx->s, ""); + } +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1829,7 +1929,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) CPUClass *cc; const char *p; uint32_t pid, tid; - int ch, type, res; + int ch, type; uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; char thread_id[16]; @@ -1878,66 +1978,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'v': - if (strncmp(p, "Cont", 4) == 0) { - p += 4; - if (*p == '?') { - put_packet(s, "vCont;c;C;s;S"); - break; - } - - res = gdb_handle_vcont(s, p); - - if (res) { - if ((res == -EINVAL) || (res == -ERANGE)) { - put_packet(s, "E22"); - break; - } - goto unknown_command; - } - break; - } else if (strncmp(p, "Attach;", 7) == 0) { - unsigned long pid; - - p += 7; - - if (qemu_strtoul(p, &p, 16, &pid)) { - put_packet(s, "E22"); - break; - } - - process = gdb_get_process(s, pid); - - if (process == NULL) { - put_packet(s, "E22"); - break; - } - - cpu = get_first_cpu_in_process(s, process); - - if (cpu == NULL) { - /* Refuse to attach an empty process */ - put_packet(s, "E22"); - break; - } - - process->attached = true; - - s->g_cpu = cpu; - s->c_cpu = cpu; - - snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP, - gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id))); - - put_packet(s, buf); - break; - } else if (strncmp(p, "Kill;", 5) == 0) { - /* Kill the target */ - put_packet(s, "OK"); - error_report("QEMU: Terminated via GDBstub"); - exit(0); - } else { - goto unknown_command; + { + static const GdbCmdParseEntry v_cmd_desc = { + .handler = handle_v_commands, + .cmd = "v", + .cmd_startswith = 1, + .schema = "s0" + }; + cmd_parser = &v_cmd_desc; } + break; case 'k': /* Kill the target */ error_report("QEMU: Terminated via GDBstub"); From 2704efad514da6600481bef8686ddb786c10c62b Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:45 +0300 Subject: [PATCH 37/40] gdbstub: Implement generic set/query (Q/q pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic set/query packets contains implementation for varioius sub-commands which are required for GDB and also additional commands which are QEMU specific. To see which QEMU specific commands are available use the command gdb> maintenance packet qqemu.Supported Currently the only implemented QEMU specific command is the command that sets the single step behavior. gdb> maintenance packet qqemu.sstepbits Will display the MASK bits used to control the single stepping. gdb> maintenance packet qqemu.sstep Will display the current value of the mask used when single stepping. gdb> maintenance packet Qqemu.sstep:HEX_VALUE Will change the single step mask. Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-18-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 559 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 373 insertions(+), 186 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 8864c3a392..6ccfec9ef3 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1134,14 +1134,6 @@ static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf, return GDB_ONE_THREAD; } -static int is_query_packet(const char *p, const char *query, char separator) -{ - unsigned int query_len = strlen(query); - - return strncmp(p, query, query_len) == 0 && - (p[query_len] == '\0' || p[query_len] == separator); -} - /** * gdb_handle_vcont - Parses and handles a vCont packet. * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is @@ -1922,18 +1914,368 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) } } -static int gdb_handle_packet(GDBState *s, const char *line_buf) +static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), + "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE, + SSTEP_NOIRQ, SSTEP_NOTIMER); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (!gdb_ctx->num_params) { + return; + } + + sstep_flags = gdb_ctx->params[0].val_ul; + put_packet(gdb_ctx->s, "OK"); +} + +static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx) { CPUState *cpu; GDBProcess *process; + char thread_id[16]; + + /* + * "Current thread" remains vague in the spec, so always return + * the first thread of the current process (gdb returns the + * first thread). + */ + process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu); + cpu = get_first_cpu_in_process(gdb_ctx->s, process); + gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id)); + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + char thread_id[16]; + + if (!gdb_ctx->s->query_cpu) { + put_packet(gdb_ctx->s, "l"); + return; + } + + gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->query_cpu, thread_id, + sizeof(thread_id)); + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); + gdb_ctx->s->query_cpu = + gdb_next_attached_cpu(gdb_ctx->s, gdb_ctx->s->query_cpu); +} + +static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + gdb_ctx->s->query_cpu = gdb_first_attached_cpu(gdb_ctx->s); + handle_query_threads(gdb_ctx, user_ctx); +} + +static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + CPUState *cpu; + int len; + + if (!gdb_ctx->num_params || + gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid, + gdb_ctx->params[0].thread_id.tid); + if (!cpu) { + return; + } + + cpu_synchronize_state(cpu); + + if (gdb_ctx->s->multiprocess && (gdb_ctx->s->process_num > 1)) { + /* Print the CPU model and name in multiprocess mode */ + ObjectClass *oc = object_get_class(OBJECT(cpu)); + const char *cpu_model = object_class_get_name(oc); + char *cpu_name = object_get_canonical_path_component(OBJECT(cpu)); + len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2, + "%s %s [%s]", cpu_model, cpu_name, + cpu->halted ? "halted " : "running"); + g_free(cpu_name); + } else { + /* memtohex() doubles the required space */ + len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2, + "CPU#%d [%s]", cpu->cpu_index, + cpu->halted ? "halted " : "running"); + } + trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf); + memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +#ifdef CONFIG_USER_ONLY +static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + TaskState *ts; + + ts = gdb_ctx->s->c_cpu->opaque; + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), + "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx + ";Bss=" TARGET_ABI_FMT_lx, + ts->info->code_offset, + ts->info->data_offset, + ts->info->data_offset); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} +#else +static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + int len; + + if (!gdb_ctx->num_params) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + len = strlen(gdb_ctx->params[0].data); + if (len % 2) { + put_packet(gdb_ctx->s, "E01"); + return; + } + + len = len / 2; + hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len); + gdb_ctx->mem_buf[len++] = 0; + qemu_chr_be_write(gdb_ctx->s->mon_chr, gdb_ctx->mem_buf, len); + put_packet(gdb_ctx->s, "OK"); + +} +#endif + +static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) +{ CPUClass *cc; + + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "PacketSize=%x", + MAX_PACKET_LENGTH); + cc = CPU_GET_CLASS(first_cpu); + if (cc->gdb_core_xml_file) { + pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), + ";qXfer:features:read+"); + } + + if (gdb_ctx->num_params && + strstr(gdb_ctx->params[0].data, "multiprocess+")) { + gdb_ctx->s->multiprocess = true; + } + + pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+"); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + GDBProcess *process; + CPUClass *cc; + unsigned long len, total_len, addr; + const char *xml; const char *p; - uint32_t pid, tid; - int ch, type; + + if (gdb_ctx->num_params < 3) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu); + cc = CPU_GET_CLASS(gdb_ctx->s->g_cpu); + if (!cc->gdb_core_xml_file) { + put_packet(gdb_ctx->s, ""); + return; + } + + gdb_has_xml = true; + p = gdb_ctx->params[0].data; + xml = get_feature_xml(gdb_ctx->s, p, &p, process); + if (!xml) { + put_packet(gdb_ctx->s, "E00"); + return; + } + + addr = gdb_ctx->params[1].val_ul; + len = gdb_ctx->params[2].val_ul; + total_len = strlen(xml); + if (addr > total_len) { + put_packet(gdb_ctx->s, "E00"); + return; + } + + if (len > (MAX_PACKET_LENGTH - 5) / 2) { + len = (MAX_PACKET_LENGTH - 5) / 2; + } + + if (len < total_len - addr) { + gdb_ctx->str_buf[0] = 'm'; + len = memtox(gdb_ctx->str_buf + 1, xml + addr, len); + } else { + gdb_ctx->str_buf[0] = 'l'; + len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr); + } + + put_packet_binary(gdb_ctx->s, gdb_ctx->str_buf, len + 1, true); +} + +static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + put_packet(gdb_ctx->s, GDB_ATTACHED); +} + +static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + put_packet(gdb_ctx->s, "sstepbits;sstep"); +} + +static GdbCmdParseEntry gdb_gen_query_set_common_table[] = { + /* Order is important if has same prefix */ + { + .handler = handle_query_qemu_sstepbits, + .cmd = "qemu.sstepbits", + }, + { + .handler = handle_query_qemu_sstep, + .cmd = "qemu.sstep", + }, + { + .handler = handle_set_qemu_sstep, + .cmd = "qemu.sstep=", + .cmd_startswith = 1, + .schema = "l0" + }, +}; + +static GdbCmdParseEntry gdb_gen_query_table[] = { + { + .handler = handle_query_curr_tid, + .cmd = "C", + }, + { + .handler = handle_query_threads, + .cmd = "sThreadInfo", + }, + { + .handler = handle_query_first_threads, + .cmd = "fThreadInfo", + }, + { + .handler = handle_query_thread_extra, + .cmd = "ThreadExtraInfo,", + .cmd_startswith = 1, + .schema = "t0" + }, +#ifdef CONFIG_USER_ONLY + { + .handler = handle_query_offsets, + .cmd = "Offsets", + }, +#else + { + .handler = handle_query_rcmd, + .cmd = "Rcmd,", + .cmd_startswith = 1, + .schema = "s0" + }, +#endif + { + .handler = handle_query_supported, + .cmd = "Supported:", + .cmd_startswith = 1, + .schema = "s0" + }, + { + .handler = handle_query_supported, + .cmd = "Supported", + .schema = "s0" + }, + { + .handler = handle_query_xfer_features, + .cmd = "Xfer:features:read:", + .cmd_startswith = 1, + .schema = "s:l,l0" + }, + { + .handler = handle_query_attached, + .cmd = "Attached:", + .cmd_startswith = 1 + }, + { + .handler = handle_query_attached, + .cmd = "Attached", + }, + { + .handler = handle_query_qemu_supported, + .cmd = "qemu.Supported", + }, +}; + +static GdbCmdParseEntry gdb_gen_set_table[] = { + /* Order is important if has same prefix */ + { + .handler = handle_set_qemu_sstep, + .cmd = "qemu.sstep:", + .cmd_startswith = 1, + .schema = "l0" + }, +}; + +static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (!gdb_ctx->num_params) { + return; + } + + if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data, + gdb_gen_query_set_common_table, + ARRAY_SIZE(gdb_gen_query_set_common_table))) { + return; + } + + if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data, + gdb_gen_query_table, + ARRAY_SIZE(gdb_gen_query_table))) { + put_packet(gdb_ctx->s, ""); + } +} + +static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (!gdb_ctx->num_params) { + return; + } + + if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data, + gdb_gen_query_set_common_table, + ARRAY_SIZE(gdb_gen_query_set_common_table))) { + return; + } + + if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data, + gdb_gen_set_table, + ARRAY_SIZE(gdb_gen_set_table))) { + put_packet(gdb_ctx->s, ""); + } +} + +static int gdb_handle_packet(GDBState *s, const char *line_buf) +{ + const char *p; + int ch; uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; char thread_id[16]; - target_ulong addr, len; const GdbCmdParseEntry *cmd_parser = NULL; trace_gdbstub_io_command(line_buf); @@ -2135,183 +2477,28 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } break; case 'q': + { + static const GdbCmdParseEntry gen_query_cmd_desc = { + .handler = handle_gen_query, + .cmd = "q", + .cmd_startswith = 1, + .schema = "s0" + }; + cmd_parser = &gen_query_cmd_desc; + } + break; case 'Q': - /* parse any 'q' packets here */ - if (!strcmp(p,"qemu.sstepbits")) { - /* Query Breakpoint bit definitions */ - snprintf(buf, sizeof(buf), "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", - SSTEP_ENABLE, - SSTEP_NOIRQ, - SSTEP_NOTIMER); - put_packet(s, buf); - break; - } else if (is_query_packet(p, "qemu.sstep", '=')) { - /* Display or change the sstep_flags */ - p += 10; - if (*p != '=') { - /* Display current setting */ - snprintf(buf, sizeof(buf), "0x%x", sstep_flags); - put_packet(s, buf); - break; - } - p++; - type = strtoul(p, (char **)&p, 16); - sstep_flags = type; - put_packet(s, "OK"); - break; - } else if (strcmp(p,"C") == 0) { - /* - * "Current thread" remains vague in the spec, so always return - * the first thread of the current process (gdb returns the - * first thread). - */ - cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, s->g_cpu)); - snprintf(buf, sizeof(buf), "QC%s", - gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id))); - put_packet(s, buf); - break; - } else if (strcmp(p,"fThreadInfo") == 0) { - s->query_cpu = gdb_first_attached_cpu(s); - goto report_cpuinfo; - } else if (strcmp(p,"sThreadInfo") == 0) { - report_cpuinfo: - if (s->query_cpu) { - snprintf(buf, sizeof(buf), "m%s", - gdb_fmt_thread_id(s, s->query_cpu, - thread_id, sizeof(thread_id))); - put_packet(s, buf); - s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu); - } else - put_packet(s, "l"); - break; - } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) { - if (read_thread_id(p + 16, &p, &pid, &tid) == GDB_READ_THREAD_ERR) { - put_packet(s, "E22"); - break; - } - cpu = gdb_get_cpu(s, pid, tid); - if (cpu != NULL) { - cpu_synchronize_state(cpu); - - if (s->multiprocess && (s->process_num > 1)) { - /* Print the CPU model and name in multiprocess mode */ - ObjectClass *oc = object_get_class(OBJECT(cpu)); - const char *cpu_model = object_class_get_name(oc); - char *cpu_name = - object_get_canonical_path_component(OBJECT(cpu)); - len = snprintf((char *)mem_buf, sizeof(buf) / 2, - "%s %s [%s]", cpu_model, cpu_name, - cpu->halted ? "halted " : "running"); - g_free(cpu_name); - } else { - /* memtohex() doubles the required space */ - len = snprintf((char *)mem_buf, sizeof(buf) / 2, - "CPU#%d [%s]", cpu->cpu_index, - cpu->halted ? "halted " : "running"); - } - trace_gdbstub_op_extra_info((char *)mem_buf); - memtohex(buf, mem_buf, len); - put_packet(s, buf); - } - break; + { + static const GdbCmdParseEntry gen_set_cmd_desc = { + .handler = handle_gen_set, + .cmd = "Q", + .cmd_startswith = 1, + .schema = "s0" + }; + cmd_parser = &gen_set_cmd_desc; } -#ifdef CONFIG_USER_ONLY - else if (strcmp(p, "Offsets") == 0) { - TaskState *ts = s->c_cpu->opaque; - - snprintf(buf, sizeof(buf), - "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx - ";Bss=" TARGET_ABI_FMT_lx, - ts->info->code_offset, - ts->info->data_offset, - ts->info->data_offset); - put_packet(s, buf); - break; - } -#else /* !CONFIG_USER_ONLY */ - else if (strncmp(p, "Rcmd,", 5) == 0) { - int len = strlen(p + 5); - - if ((len % 2) != 0) { - put_packet(s, "E01"); - break; - } - len = len / 2; - hextomem(mem_buf, p + 5, len); - mem_buf[len++] = 0; - qemu_chr_be_write(s->mon_chr, mem_buf, len); - put_packet(s, "OK"); - break; - } -#endif /* !CONFIG_USER_ONLY */ - if (is_query_packet(p, "Supported", ':')) { - snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH); - cc = CPU_GET_CLASS(first_cpu); - if (cc->gdb_core_xml_file != NULL) { - pstrcat(buf, sizeof(buf), ";qXfer:features:read+"); - } - - if (strstr(p, "multiprocess+")) { - s->multiprocess = true; - } - pstrcat(buf, sizeof(buf), ";multiprocess+"); - - put_packet(s, buf); - break; - } - if (strncmp(p, "Xfer:features:read:", 19) == 0) { - const char *xml; - target_ulong total_len; - - process = gdb_get_cpu_process(s, s->g_cpu); - cc = CPU_GET_CLASS(s->g_cpu); - if (cc->gdb_core_xml_file == NULL) { - goto unknown_command; - } - - gdb_has_xml = true; - p += 19; - xml = get_feature_xml(s, p, &p, process); - if (!xml) { - snprintf(buf, sizeof(buf), "E00"); - put_packet(s, buf); - break; - } - - if (*p == ':') - p++; - addr = strtoul(p, (char **)&p, 16); - if (*p == ',') - p++; - len = strtoul(p, (char **)&p, 16); - - total_len = strlen(xml); - if (addr > total_len) { - snprintf(buf, sizeof(buf), "E00"); - put_packet(s, buf); - break; - } - if (len > (MAX_PACKET_LENGTH - 5) / 2) - len = (MAX_PACKET_LENGTH - 5) / 2; - if (len < total_len - addr) { - buf[0] = 'm'; - len = memtox(buf + 1, xml + addr, len); - } else { - buf[0] = 'l'; - len = memtox(buf + 1, xml + addr, total_len - addr); - } - put_packet_binary(s, buf, len + 1, true); - break; - } - if (is_query_packet(p, "Attached", ':')) { - put_packet(s, GDB_ATTACHED); - break; - } - /* Unrecognised 'q' command. */ - goto unknown_command; - + break; default: - unknown_command: /* put empty packet */ buf[0] = '\0'; put_packet(s, buf); From 7009d579240035d9ba981940bf945d8b633625dd Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:46 +0300 Subject: [PATCH 38/40] gdbstub: Implement target halted (? pkt) with new infra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: The user-mode thread-id has been correctly reported since bd88c780e6 Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-19-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 6ccfec9ef3..31addf317e 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2269,13 +2269,29 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx) } } +static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + char thread_id[16]; + + gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->c_cpu, thread_id, + sizeof(thread_id)); + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;", + GDB_SIGNAL_TRAP, thread_id); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); + /* + * Remove all the breakpoints when this query is issued, + * because gdb is doing an initial connect and the state + * should be cleaned up. + */ + gdb_breakpoint_remove_all(); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { const char *p; int ch; uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; - char thread_id[16]; const GdbCmdParseEntry *cmd_parser = NULL; trace_gdbstub_io_command(line_buf); @@ -2287,15 +2303,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "OK"); break; case '?': - /* TODO: Make this return the correct value for user-mode. */ - snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP, - gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id))); - put_packet(s, buf); - /* Remove all the breakpoints when this query is issued, - * because gdb is doing and initial connect and the state - * should be cleaned up. - */ - gdb_breakpoint_remove_all(); + { + static const GdbCmdParseEntry target_halted_cmd_desc = { + .handler = handle_target_halt, + .cmd = "?", + .cmd_startswith = 1 + }; + cmd_parser = &target_halted_cmd_desc; + } break; case 'c': { From 3f1cbac73a441c518e27184bf24bc56796f4ab5a Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:47 +0300 Subject: [PATCH 39/40] gdbstub: Clear unused variables in gdb_handle_packet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jon Doron Reviewed-by: Alex Bennée Message-Id: <20190529064148.19856-20-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 31addf317e..4af94d8f78 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2288,17 +2288,11 @@ static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx) static int gdb_handle_packet(GDBState *s, const char *line_buf) { - const char *p; - int ch; - uint8_t mem_buf[MAX_PACKET_LENGTH]; - char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; const GdbCmdParseEntry *cmd_parser = NULL; trace_gdbstub_io_command(line_buf); - p = line_buf; - ch = *p++; - switch(ch) { + switch (line_buf[0]) { case '!': put_packet(s, "OK"); break; @@ -2515,8 +2509,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) break; default: /* put empty packet */ - buf[0] = '\0'; - put_packet(s, buf); + put_packet(s, ""); break; } From ab4752ec8d9b0b19ab80915016b739350418a078 Mon Sep 17 00:00:00 2001 From: Jon Doron Date: Wed, 29 May 2019 09:41:48 +0300 Subject: [PATCH 40/40] gdbstub: Implement qemu physical memory mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new query/set which changes the memory GDB sees to physical memory only. gdb> maint packet qqemu.PhyMemMode will reply the current phy_mem_mode state (1 for enabled, 0 for disabled) gdb> maint packet Qqemu.PhyMemMode:1 Will make GDB read/write only to physical memory, set to 0 to disable Signed-off-by: Jon Doron Message-Id: <20190529064148.19856-21-arilou@gmail.com> Signed-off-by: Alex Bennée --- gdbstub.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 4af94d8f78..d614a1f3c0 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -50,11 +50,27 @@ #define GDB_ATTACHED "1" #endif +#ifndef CONFIG_USER_ONLY +static int phy_memory_mode; +#endif + static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr, uint8_t *buf, int len, bool is_write) { - CPUClass *cc = CPU_GET_CLASS(cpu); + CPUClass *cc; +#ifndef CONFIG_USER_ONLY + if (phy_memory_mode) { + if (is_write) { + cpu_physical_memory_write(addr, buf, len); + } else { + cpu_physical_memory_read(addr, buf, len); + } + return 0; + } +#endif + + cc = CPU_GET_CLASS(cpu); if (cc->memory_rw_debug) { return cc->memory_rw_debug(cpu, addr, buf, len, is_write); } @@ -2136,9 +2152,37 @@ static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx) static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx) { - put_packet(gdb_ctx->s, "sstepbits;sstep"); + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "sstepbits;sstep"); +#ifndef CONFIG_USER_ONLY + pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";PhyMemMode"); +#endif + put_packet(gdb_ctx->s, gdb_ctx->str_buf); } +#ifndef CONFIG_USER_ONLY +static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, + void *user_ctx) +{ + snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "%d", phy_memory_mode); + put_packet(gdb_ctx->s, gdb_ctx->str_buf); +} + +static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + if (!gdb_ctx->num_params) { + put_packet(gdb_ctx->s, "E22"); + return; + } + + if (!gdb_ctx->params[0].val_ul) { + phy_memory_mode = 0; + } else { + phy_memory_mode = 1; + } + put_packet(gdb_ctx->s, "OK"); +} +#endif + static GdbCmdParseEntry gdb_gen_query_set_common_table[] = { /* Order is important if has same prefix */ { @@ -2219,6 +2263,12 @@ static GdbCmdParseEntry gdb_gen_query_table[] = { .handler = handle_query_qemu_supported, .cmd = "qemu.Supported", }, +#ifndef CONFIG_USER_ONLY + { + .handler = handle_query_qemu_phy_mem_mode, + .cmd = "qemu.PhyMemMode", + }, +#endif }; static GdbCmdParseEntry gdb_gen_set_table[] = { @@ -2229,6 +2279,14 @@ static GdbCmdParseEntry gdb_gen_set_table[] = { .cmd_startswith = 1, .schema = "l0" }, +#ifndef CONFIG_USER_ONLY + { + .handler = handle_set_qemu_phy_mem_mode, + .cmd = "qemu.PhyMemMode:", + .cmd_startswith = 1, + .schema = "l0" + }, +#endif }; static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)