From cef63308e6df3b4ab61f4ebdc1d4674c9083eb8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 30 Jun 2023 19:03:46 +0100 Subject: gitlab: explicit set artifacts publishing criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If not set explicitly, gitlab assumes 'when: on_success" as the publishing criteria for artifacts. This is reasonable if the artifact is an output deliverable of the job. This is useless if the artifact is a log file to be used for debugging job failures. This change makes the desired criteria explicit for every job that publishes artifacts. Signed-off-by: Daniel P. Berrangé Reviewed-by: Richard Henderson Message-Id: <20230503145535.91325-2-berrange@redhat.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-2-alex.bennee@linaro.org> --- .gitlab-ci.d/buildtest-template.yml | 4 +++- .gitlab-ci.d/buildtest.yml | 2 ++ .gitlab-ci.d/crossbuild-template.yml | 1 + .gitlab-ci.d/crossbuilds.yml | 2 ++ .gitlab-ci.d/opensbi.yml | 1 + 5 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml index 5da61f4..f3e39b7 100644 --- a/.gitlab-ci.d/buildtest-template.yml +++ b/.gitlab-ci.d/buildtest-template.yml @@ -25,6 +25,7 @@ # rebuilding all the object files we skip in the artifacts .native_build_artifact_template: artifacts: + when: on_success expire_in: 2 days paths: - build @@ -53,6 +54,7 @@ extends: .common_test_job_template artifacts: name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG" + when: always expire_in: 7 days paths: - build/meson-logs/testlog.txt @@ -68,7 +70,7 @@ policy: pull-push artifacts: name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG" - when: on_failure + when: always expire_in: 7 days paths: - build/tests/results/latest/results.xml diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index aa833b6..24bba06 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -462,6 +462,7 @@ gcov: coverage: /^\s*lines:\s*\d+.\d+\%/ artifacts: name: ${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA} + when: on_success expire_in: 2 days reports: coverage_report: @@ -587,6 +588,7 @@ pages: - make -C build install DESTDIR=$(pwd)/temp-install - mv temp-install/usr/local/share/doc/qemu/* public/ artifacts: + when: on_success paths: - public variables: diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml index 6efb0d2..d976110 100644 --- a/.gitlab-ci.d/crossbuild-template.yml +++ b/.gitlab-ci.d/crossbuild-template.yml @@ -55,6 +55,7 @@ .cross_test_artifacts: artifacts: name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG" + when: always expire_in: 7 days paths: - build/meson-logs/testlog.txt diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml index b6ec99e..84ff2f6 100644 --- a/.gitlab-ci.d/crossbuilds.yml +++ b/.gitlab-ci.d/crossbuilds.yml @@ -169,6 +169,7 @@ cross-win32-system: CROSS_SKIP_TARGETS: alpha-softmmu avr-softmmu hppa-softmmu m68k-softmmu microblazeel-softmmu mips64el-softmmu nios2-softmmu artifacts: + when: on_success paths: - build/qemu-setup*.exe @@ -184,6 +185,7 @@ cross-win64-system: or1k-softmmu rx-softmmu sh4eb-softmmu sparc64-softmmu tricore-softmmu xtensaeb-softmmu artifacts: + when: on_success paths: - build/qemu-setup*.exe diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml index b4d7eef..fd293e6 100644 --- a/.gitlab-ci.d/opensbi.yml +++ b/.gitlab-ci.d/opensbi.yml @@ -63,6 +63,7 @@ build-opensbi: stage: build needs: ['docker-opensbi'] artifacts: + when: on_success paths: # 'artifacts.zip' will contains the following files: - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin - pc-bios/opensbi-riscv64-generic-fw_dynamic.bin -- cgit v1.1 From dcb242c8495c04599a6405ed8fbd03d82bc33365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 30 Jun 2023 19:03:47 +0100 Subject: gitlab: ensure coverage job also publishes meson log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage job wants to publish a coverage report on success, but the tests might fail and in that case we need the meson logs for debugging. Signed-off-by: Daniel P. Berrangé Reviewed-by: Richard Henderson Message-Id: <20230503145535.91325-3-berrange@redhat.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-3-alex.bennee@linaro.org> --- .gitlab-ci.d/buildtest.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 24bba06..a8fd9a0 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -462,9 +462,12 @@ gcov: coverage: /^\s*lines:\s*\d+.\d+\%/ artifacts: name: ${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA} - when: on_success + when: always expire_in: 2 days + paths: + - build/meson-logs/testlog.txt reports: + junit: build/meson-logs/testlog.junit.xml coverage_report: coverage_format: cobertura path: build/coverage.xml -- cgit v1.1 From dc389ce1b30c7ca9305c849e02e349ff8296a831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:48 +0100 Subject: gitlab: reduce testing scope of check-gcov MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This keeps timing out on gitlab due to some qtests taking a long time. As this is just ensuring the gcov machinery is working and not attempting to be comprehensive lets skip qtest in this run. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-4-alex.bennee@linaro.org> --- .gitlab-ci.d/buildtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index a8fd9a0..77dc83a 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -454,7 +454,7 @@ gcov: IMAGE: ubuntu2204 CONFIGURE_ARGS: --enable-gcov TARGETS: aarch64-softmmu ppc64-softmmu s390x-softmmu x86_64-softmmu - MAKE_CHECK_ARGS: check + MAKE_CHECK_ARGS: check-unit check-softfloat after_script: - cd build - gcovr --xml-pretty --exclude-unreachable-branches --print-summary -- cgit v1.1 From d2ab529ec559df57dc142d24ea3ec0a629a23657 Mon Sep 17 00:00:00 2001 From: Ani Sinha Date: Fri, 30 Jun 2023 19:03:49 +0100 Subject: docs/devel: remind developers to run CI container pipeline when updating images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When new dependencies and packages are added to containers, its important to run CI container generation pipelines on gitlab to make sure that there are no obvious conflicts between packages that are being added and those that are already present. Running CI container pipelines will make sure that there are no such breakages before we commit the change updating the containers. Add a line in the documentation reminding developers to run the pipeline before submitting the change. It will also ease the life of the maintainers. Signed-off-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-Id: <20230506072012.10350-1-anisinha@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-5-alex.bennee@linaro.org> --- docs/devel/testing.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index e85e26c..fb651ea 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -485,6 +485,12 @@ first to contribute the mapping to the ``libvirt-ci`` project: `CI `__ documentation page on how to trigger gitlab CI pipelines on your change. + * Please also trigger gitlab container generation pipelines on your change + for as many OS distros as practical to make sure that there are no + obvious breakages when adding the new pre-requisite. Please see + `CI `__ documentation + page on how to trigger gitlab CI pipelines on your change. + For enterprise distros that default to old, end-of-life versions of the Python runtime, QEMU uses a separate set of mappings that work with more recent versions. These can be found in ``tests/lcitool/mappings.yml``. -- cgit v1.1 From af323aabe2a7152b1e857baa840485b79c06bbf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:50 +0100 Subject: tests/tcg: add mechanism to handle plugin arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We recently missed a regression that should have been picked up by check-tcg. This was because the libmem plugin is effectively a NOP if the user doesn't specify the type to use. Rather than changing the default behaviour add an additional expansion so we can take this into account in future. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-6-alex.bennee@linaro.org> --- tests/tcg/Makefile.target | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target index 72876cc..2462c26 100644 --- a/tests/tcg/Makefile.target +++ b/tests/tcg/Makefile.target @@ -169,13 +169,17 @@ extract-plugin = $(wordlist 2, 2, $(subst -with-, ,$1)) RUN_TESTS+=$(EXTRA_RUNS) +# Some plugins need additional arguments above the default to fully +# exercise things. We can define them on a per-test basis here. +run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true$(COMMA)callback=true + ifeq ($(filter %-softmmu, $(TARGET)),) run-%: % $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<) run-plugin-%: $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \ - -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@) \ + -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \ -d plugin -D $*.pout \ $(call strip-plugin,$<)) else @@ -189,7 +193,7 @@ run-plugin-%: $(call run-test, $@, \ $(QEMU) -monitor none -display none \ -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ - -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@) \ + -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \ -d plugin -D $*.pout \ $(QEMU_OPTS) $(call strip-plugin,$<)) endif -- cgit v1.1 From 58127176589c22d9fb8d60b67ce2c9bbec3dca67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:51 +0100 Subject: qemu-keymap: properly check return from xkb_keymap_mod_get_index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can return XKB_MOD_INVALID for AltGr which rightly gets flagged by sanitisers as an overly wide shift attempt. Properly check the return type and leave the bitmap as zero in that case. Tested output before and after is unchanged with the gb and ara keymaps. Reviewed-by: Juan Quintela Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-7-alex.bennee@linaro.org> --- qemu-keymap.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/qemu-keymap.c b/qemu-keymap.c index 229866e..8c80f7a 100644 --- a/qemu-keymap.c +++ b/qemu-keymap.c @@ -140,6 +140,18 @@ static void usage(FILE *out) names.options ?: "-"); } +static xkb_mod_mask_t get_mod(struct xkb_keymap *map, const char *name) +{ + xkb_mod_index_t mod; + xkb_mod_mask_t mask = 0; + + mod = xkb_keymap_mod_get_index(map, name); + if (mod != XKB_MOD_INVALID) { + mask = (1 << mod); + } + return mask; +} + int main(int argc, char *argv[]) { struct xkb_context *ctx; @@ -215,14 +227,10 @@ int main(int argc, char *argv[]) mod, xkb_keymap_mod_get_name(map, mod)); } - mod = xkb_keymap_mod_get_index(map, "Shift"); - shift = (1 << mod); - mod = xkb_keymap_mod_get_index(map, "Control"); - ctrl = (1 << mod); - mod = xkb_keymap_mod_get_index(map, "AltGr"); - altgr = (1 << mod); - mod = xkb_keymap_mod_get_index(map, "NumLock"); - numlock = (1 << mod); + shift = get_mod(map, "Shift"); + ctrl = get_mod(map, "Control"); + altgr = get_mod(map, "AltGr"); + numlock = get_mod(map, "NumLock"); state = xkb_state_new(map); xkb_keymap_key_for_each(map, walk_map, state); -- cgit v1.1 From 9ea2e69fb2df969332badd403f35b0e355f8a741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:52 +0100 Subject: scripts/oss-fuzz: add a suppression for keymap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When updating to the latest fedora the santizer found more leaks inside xkbmap: FAILED: pc-bios/keymaps/ar /builds/stsquad/qemu/build-oss-fuzz/qemu-keymap -f pc-bios/keymaps/ar -l ara ================================================================= ==3604==ERROR: LeakSanitizer: detected memory leaks Direct leak of 1424 byte(s) in 1 object(s) allocated from: #0 0x56316418ebec in __interceptor_calloc (/builds/stsquad/qemu/build-oss-fuzz/qemu-keymap+0x127bec) (BuildId: a2ad9da3190962acaa010fa8f44a9269f9081e1c) #1 0x7f60d4dc067e (/lib64/libxkbcommon.so.0+0x1c67e) (BuildId: b243a34e4e58e6a30b93771c256268b114d34b80) #2 0x7f60d4dc2137 in xkb_keymap_new_from_names (/lib64/libxkbcommon.so.0+0x1e137) (BuildId: b243a34e4e58e6a30b93771c256268b114d34b80) #3 0x5631641ca50f in main /builds/stsquad/qemu/build-oss-fuzz/../qemu-keymap.c:215:11 and many more. As we can't do anything about the library add a suppression to keep the CI going with what its meant to be doing. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-8-alex.bennee@linaro.org> --- scripts/oss-fuzz/lsan_suppressions.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/oss-fuzz/lsan_suppressions.txt b/scripts/oss-fuzz/lsan_suppressions.txt index 02ec0a6e..7d90c28 100644 --- a/scripts/oss-fuzz/lsan_suppressions.txt +++ b/scripts/oss-fuzz/lsan_suppressions.txt @@ -1,2 +1,5 @@ # The tcmalloc on Fedora37 confuses things leak:/lib64/libtcmalloc_minimal.so.4 + +# libxkbcommon also leaks in qemu-keymap +leak:/lib64/libxkbcommon.so.0 -- cgit v1.1 From 3176990fb0307233ae5619e36b2c63241134e326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:53 +0100 Subject: tests/qtests: clean-up and fix leak in generic_fuzz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An update to the clang tooling detects more issues with the code including a memory leak from the g_string_new() allocation. Clean up the code to avoid the allocation and use ARRAY_SIZE while we are at it. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-9-alex.bennee@linaro.org> --- tests/qtest/fuzz/generic_fuzz.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index c525d22..11256ab 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -954,17 +954,10 @@ static void register_generic_fuzz_targets(void) .crossover = generic_fuzz_crossover }); - GString *name; - const generic_fuzz_config *config; - - for (int i = 0; - i < sizeof(predefined_configs) / sizeof(generic_fuzz_config); - i++) { - config = predefined_configs + i; - name = g_string_new("generic-fuzz"); - g_string_append_printf(name, "-%s", config->name); + for (int i = 0; i < ARRAY_SIZE(predefined_configs); i++) { + const generic_fuzz_config *config = predefined_configs + i; fuzz_add_target(&(FuzzTarget){ - .name = name->str, + .name = g_strconcat("generic-fuzz-", config->name, NULL), .description = "Predefined generic-fuzz config.", .get_init_cmdline = generic_fuzz_predefined_config_cmdline, .pre_fuzz = generic_pre_fuzz, -- cgit v1.1 From 4ab20b513e7279c5441db2bba7d326484668d2cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:54 +0100 Subject: tests/docker: add test-fuzz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the fuzzer requires some hoop jumping and some problems only show up in containers. This basically replicates the build-oss-fuzz job from our CI so we can run in the same containers we use in CI. Reviewed-by: Alexander Bulekov Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-10-alex.bennee@linaro.org> --- MAINTAINERS | 1 + tests/docker/test-fuzz | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100755 tests/docker/test-fuzz diff --git a/MAINTAINERS b/MAINTAINERS index 4feea49..7164cf5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3106,6 +3106,7 @@ R: Qiuhao Li S: Maintained F: tests/qtest/fuzz/ F: tests/qtest/fuzz-*test.c +F: tests/docker/test-fuzz F: scripts/oss-fuzz/ F: hw/mem/sparse-mem.c F: docs/devel/fuzzing.rst diff --git a/tests/docker/test-fuzz b/tests/docker/test-fuzz new file mode 100755 index 0000000..7e506ae --- /dev/null +++ b/tests/docker/test-fuzz @@ -0,0 +1,28 @@ +#!/bin/bash -e +# +# Compile and check with oss-fuzz. +# +# Copyright (c) 2023 Linaro Ltd. +# +# Authors: +# Alex Bennée +# +# SPDX-License-Identifier: GPL-2.0-or-later + +. common.rc + +requires_binary clang + +# the build script runs out of $src so we need to copy across +cd "$BUILD_DIR" +cp -a $QEMU_SRC . +cd src +mkdir build-oss-fuzz +export LSAN_OPTIONS=suppressions=scripts/oss-fuzz/lsan_suppressions.txt +env CC="clang" CXX="clang++" CFLAGS="-fsanitize=address" ./scripts/oss-fuzz/build.sh +export ASAN_OPTIONS="fast_unwind_on_malloc=0" +for fuzzer in $(find ./build-oss-fuzz/DEST_DIR/ -executable -type f | grep -v slirp); do + grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ; + echo Testing ${fuzzer} ... ; + "${fuzzer}" -runs=1 -seed=1 || exit 1 ; +done -- cgit v1.1 From 369dbbe091e2e1b9e603167cd4d9da5e9edaca8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:55 +0100 Subject: Makefile: add lcitool-refresh to UNCHECKED_GOALS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is yet another make target you usually run in the top level of the source directory. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-11-alex.bennee@linaro.org> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 804a568..5d48dfa 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,7 @@ quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3) UNCHECKED_GOALS := TAGS gtags cscope ctags dist \ help check-help print-% \ - docker docker-% vm-help vm-test vm-build-% + docker docker-% lcitool-refresh vm-help vm-test vm-build-% all: .PHONY: all clean distclean recurse-all dist msi FORCE -- cgit v1.1 From c7374742d83a692fefcd64dc1030645c0fe931f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:56 +0100 Subject: tests/lcitool: update to latest version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need this for the riscv64 and gcc-native mappings. As the older alpine release has been dropped from the mappings we also need to bump the version of alpine we use. Acked-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-12-alex.bennee@linaro.org> --- tests/docker/dockerfiles/alpine.docker | 4 ++-- tests/lcitool/libvirt-ci | 2 +- tests/lcitool/refresh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker index 0097637..43370f7 100644 --- a/tests/docker/dockerfiles/alpine.docker +++ b/tests/docker/dockerfiles/alpine.docker @@ -1,10 +1,10 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all alpine-316 qemu +# $ lcitool dockerfile --layers all alpine-318 qemu # # https://gitlab.com/libvirt/libvirt-ci -FROM docker.io/library/alpine:3.16 +FROM docker.io/library/alpine:3.18 RUN apk update && \ apk upgrade && \ diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci index c8971e9..b0f44f9 160000 --- a/tests/lcitool/libvirt-ci +++ b/tests/lcitool/libvirt-ci @@ -1 +1 @@ -Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9 +Subproject commit b0f44f929a81c0a604fb7fbf8afc34d37ab0eae9 diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh index f1570b5..5d36a62 100755 --- a/tests/lcitool/refresh +++ b/tests/lcitool/refresh @@ -115,7 +115,7 @@ try: # # Standard native builds # - generate_dockerfile("alpine", "alpine-316") + generate_dockerfile("alpine", "alpine-318") generate_dockerfile("centos8", "centos-stream-8") generate_dockerfile("debian-amd64", "debian-11", trailer="".join(debian11_extras)) -- cgit v1.1 From 76eb439c0f2c53b461b9d2e62850068aee2dd70c Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 30 Jun 2023 19:03:57 +0100 Subject: tests/lcitool: Bump fedora container versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fedora 37 -> 38 Signed-off-by: Erik Skultety Acked-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: [AJB: Dropped alpine (in prev commit), reflow commit msg] Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-13-alex.bennee@linaro.org> --- tests/docker/dockerfiles/fedora-win32-cross.docker | 4 ++-- tests/docker/dockerfiles/fedora-win64-cross.docker | 4 ++-- tests/docker/dockerfiles/fedora.docker | 4 ++-- tests/lcitool/refresh | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/docker/dockerfiles/fedora-win32-cross.docker b/tests/docker/dockerfiles/fedora-win32-cross.docker index dc72ae9..a0a3cd9 100644 --- a/tests/docker/dockerfiles/fedora-win32-cross.docker +++ b/tests/docker/dockerfiles/fedora-win32-cross.docker @@ -1,10 +1,10 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross mingw32 fedora-37 qemu +# $ lcitool dockerfile --layers all --cross mingw32 fedora-38 qemu # # https://gitlab.com/libvirt/libvirt-ci -FROM registry.fedoraproject.org/fedora:37 +FROM registry.fedoraproject.org/fedora:38 RUN dnf install -y nosync && \ printf '#!/bin/sh\n\ diff --git a/tests/docker/dockerfiles/fedora-win64-cross.docker b/tests/docker/dockerfiles/fedora-win64-cross.docker index 7eb4a5d..b6c1a6a 100644 --- a/tests/docker/dockerfiles/fedora-win64-cross.docker +++ b/tests/docker/dockerfiles/fedora-win64-cross.docker @@ -1,10 +1,10 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all --cross mingw64 fedora-37 qemu +# $ lcitool dockerfile --layers all --cross mingw64 fedora-38 qemu # # https://gitlab.com/libvirt/libvirt-ci -FROM registry.fedoraproject.org/fedora:37 +FROM registry.fedoraproject.org/fedora:38 RUN dnf install -y nosync && \ printf '#!/bin/sh\n\ diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 3a69eef..8a35a17 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -1,10 +1,10 @@ # THIS FILE WAS AUTO-GENERATED # -# $ lcitool dockerfile --layers all fedora-37 qemu +# $ lcitool dockerfile --layers all fedora-38 qemu # # https://gitlab.com/libvirt/libvirt-ci -FROM registry.fedoraproject.org/fedora:37 +FROM registry.fedoraproject.org/fedora:38 RUN dnf install -y nosync && \ printf '#!/bin/sh\n\ diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh index 5d36a62..5e06fb2 100755 --- a/tests/lcitool/refresh +++ b/tests/lcitool/refresh @@ -119,7 +119,7 @@ try: generate_dockerfile("centos8", "centos-stream-8") generate_dockerfile("debian-amd64", "debian-11", trailer="".join(debian11_extras)) - generate_dockerfile("fedora", "fedora-37") + generate_dockerfile("fedora", "fedora-38") generate_dockerfile("opensuse-leap", "opensuse-leap-15") generate_dockerfile("ubuntu2004", "ubuntu-2004") generate_dockerfile("ubuntu2204", "ubuntu-2204") @@ -169,12 +169,12 @@ try: trailer=cross_build("s390x-linux-gnu-", "s390x-softmmu,s390x-linux-user")) - generate_dockerfile("fedora-win32-cross", "fedora-37", + generate_dockerfile("fedora-win32-cross", "fedora-38", cross="mingw32", trailer=cross_build("i686-w64-mingw32-", "i386-softmmu")) - generate_dockerfile("fedora-win64-cross", "fedora-37", + generate_dockerfile("fedora-win64-cross", "fedora-38", cross="mingw64", trailer=cross_build("x86_64-w64-mingw32-", "x86_64-softmmu")) -- cgit v1.1 From 690be80412dc5ae761112dbe679efbe231edcacd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:58 +0100 Subject: tests/lcitool: add an explicit gcc-native package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need a native compiler to build the hexagon codegen tools. In our current images we already have a gcc as a side effect of a broken dependency between gcovr and lcov but this will be fixed when we move to bookworm. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=987818 for details. Update the packages while we are at it. Acked-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-14-alex.bennee@linaro.org> --- tests/docker/dockerfiles/debian-amd64-cross.docker | 1 + tests/docker/dockerfiles/debian-arm64-cross.docker | 1 + tests/docker/dockerfiles/debian-armel-cross.docker | 1 + tests/docker/dockerfiles/debian-armhf-cross.docker | 1 + tests/docker/dockerfiles/debian-mips64el-cross.docker | 1 + tests/docker/dockerfiles/debian-mipsel-cross.docker | 1 + tests/docker/dockerfiles/debian-ppc64el-cross.docker | 1 + tests/docker/dockerfiles/debian-s390x-cross.docker | 1 + tests/docker/dockerfiles/fedora-win32-cross.docker | 1 + tests/docker/dockerfiles/fedora-win64-cross.docker | 1 + tests/lcitool/projects/qemu.yml | 1 + 11 files changed, 11 insertions(+) diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker b/tests/docker/dockerfiles/debian-amd64-cross.docker index 40a2b6a..016c232 100644 --- a/tests/docker/dockerfiles/debian-amd64-cross.docker +++ b/tests/docker/dockerfiles/debian-amd64-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-arm64-cross.docker b/tests/docker/dockerfiles/debian-arm64-cross.docker index c99300b..3c114ef 100644 --- a/tests/docker/dockerfiles/debian-arm64-cross.docker +++ b/tests/docker/dockerfiles/debian-arm64-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-armel-cross.docker b/tests/docker/dockerfiles/debian-armel-cross.docker index 5db5c78..dfbd47d 100644 --- a/tests/docker/dockerfiles/debian-armel-cross.docker +++ b/tests/docker/dockerfiles/debian-armel-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-armhf-cross.docker b/tests/docker/dockerfiles/debian-armhf-cross.docker index ae6600b..4e0084e 100644 --- a/tests/docker/dockerfiles/debian-armhf-cross.docker +++ b/tests/docker/dockerfiles/debian-armhf-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-mips64el-cross.docker b/tests/docker/dockerfiles/debian-mips64el-cross.docker index daa2d48..88adf33 100644 --- a/tests/docker/dockerfiles/debian-mips64el-cross.docker +++ b/tests/docker/dockerfiles/debian-mips64el-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-mipsel-cross.docker b/tests/docker/dockerfiles/debian-mipsel-cross.docker index 5af04e2..256e8b5 100644 --- a/tests/docker/dockerfiles/debian-mipsel-cross.docker +++ b/tests/docker/dockerfiles/debian-mipsel-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-ppc64el-cross.docker b/tests/docker/dockerfiles/debian-ppc64el-cross.docker index 1eeba7f..4d19cd2 100644 --- a/tests/docker/dockerfiles/debian-ppc64el-cross.docker +++ b/tests/docker/dockerfiles/debian-ppc64el-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/debian-s390x-cross.docker b/tests/docker/dockerfiles/debian-s390x-cross.docker index 52e89a6..642bbde 100644 --- a/tests/docker/dockerfiles/debian-s390x-cross.docker +++ b/tests/docker/dockerfiles/debian-s390x-cross.docker @@ -24,6 +24,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ exuberant-ctags \ findutils \ flex \ + gcc \ gcovr \ gettext \ git \ diff --git a/tests/docker/dockerfiles/fedora-win32-cross.docker b/tests/docker/dockerfiles/fedora-win32-cross.docker index a0a3cd9..e3dfd68 100644 --- a/tests/docker/dockerfiles/fedora-win32-cross.docker +++ b/tests/docker/dockerfiles/fedora-win32-cross.docker @@ -29,6 +29,7 @@ exec "$@"\n' > /usr/bin/nosync && \ diffutils \ findutils \ flex \ + gcc \ gcovr \ git \ glib2-devel \ diff --git a/tests/docker/dockerfiles/fedora-win64-cross.docker b/tests/docker/dockerfiles/fedora-win64-cross.docker index b6c1a6a..0e15c96 100644 --- a/tests/docker/dockerfiles/fedora-win64-cross.docker +++ b/tests/docker/dockerfiles/fedora-win64-cross.docker @@ -29,6 +29,7 @@ exec "$@"\n' > /usr/bin/nosync && \ diffutils \ findutils \ flex \ + gcc \ gcovr \ git \ glib2-devel \ diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml index 566db83..21fd3d2 100644 --- a/tests/lcitool/projects/qemu.yml +++ b/tests/lcitool/projects/qemu.yml @@ -24,6 +24,7 @@ packages: - fuse3 - g++ - gcc + - gcc-native - gcovr - gettext - glib2 -- cgit v1.1 From b911b9001e0ad77dc123230873e3b598ff8c819b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:03:59 +0100 Subject: tests/lcitool: introduce qemu-minimal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a very bare bones set of dependencies for a minimal build of QEMU. This will be useful for minimal cross-compile sanity check based on things like Debian Sid where stuff isn't always in sync. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-15-alex.bennee@linaro.org> --- tests/lcitool/projects/qemu-minimal.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/lcitool/projects/qemu-minimal.yml diff --git a/tests/lcitool/projects/qemu-minimal.yml b/tests/lcitool/projects/qemu-minimal.yml new file mode 100644 index 0000000..d44737d --- /dev/null +++ b/tests/lcitool/projects/qemu-minimal.yml @@ -0,0 +1,27 @@ +# Very minimal set of qemu packages, used for minimal cross-compile sanity checks +--- +packages: + - bash + - bc + - bison + - ccache + - findutils + - flex + - g++ + - gcc + - gcc-native + - glib2 + - glib2-native + - glib2-static + - libc-static + - libfdt + - libffi + - make + - meson + - ninja + - pixman + - pkg-config + - python3 + - python3-venv + - sed + - tar -- cgit v1.1 From 0101dd71b061c0369931214b1a4b82258ef7a1fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:00 +0100 Subject: tests/docker: convert riscv64-cross to lcitool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We still need to base this on Debian Sid until riscv64 is promoted to a release architecture (or another distro provides a full cross compile target). We use the new qemu-minimal project description to avoid bringing in all the extra dependencies because every extra package is another chance for sid to fail. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-16-alex.bennee@linaro.org> --- .../docker/dockerfiles/debian-riscv64-cross.docker | 119 +++++++++++++-------- tests/lcitool/refresh | 10 +- 2 files changed, 83 insertions(+), 46 deletions(-) diff --git a/tests/docker/dockerfiles/debian-riscv64-cross.docker b/tests/docker/dockerfiles/debian-riscv64-cross.docker index 081404e..a2d879e 100644 --- a/tests/docker/dockerfiles/debian-riscv64-cross.docker +++ b/tests/docker/dockerfiles/debian-riscv64-cross.docker @@ -1,54 +1,85 @@ +# THIS FILE WAS AUTO-GENERATED # -# Docker cross-compiler target for riscv64 +# $ lcitool dockerfile --layers all --cross riscv64 debian-sid qemu-minimal # -# Currently the only distro that gets close to cross compiling riscv64 -# images is Debian Sid (with unofficial ports). As this is a moving -# target we keep the library list minimal and are aiming to migrate -# from this hack as soon as we are able. -# -FROM docker.io/library/debian:sid-slim +# https://gitlab.com/libvirt/libvirt-ci -# Add ports -RUN apt update && \ - DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \ - DEBIAN_FRONTEND=noninteractive eatmydata apt update -yy && \ - DEBIAN_FRONTEND=noninteractive eatmydata apt upgrade -yy - -# Install common build utilities -RUN DEBIAN_FRONTEND=noninteractive eatmydata apt install -yy \ - bison \ - bc \ - build-essential \ - ca-certificates \ - debian-ports-archive-keyring \ - dpkg-dev \ - flex \ - gettext \ - git \ - libglib2.0-dev \ - ninja-build \ - pkg-config \ - python3 \ - python3-venv +FROM docker.io/library/debian:sid-slim -# Add ports and riscv64 architecture -RUN echo "deb http://ftp.ports.debian.org/debian-ports/ sid main" >> /etc/apt/sources.list -RUN dpkg --add-architecture riscv64 +RUN export DEBIAN_FRONTEND=noninteractive && \ + apt-get update && \ + apt-get install -y eatmydata && \ + eatmydata apt-get dist-upgrade -y && \ + eatmydata apt-get install --no-install-recommends -y \ + bash \ + bc \ + bison \ + ca-certificates \ + ccache \ + findutils \ + flex \ + gcc \ + git \ + libglib2.0-dev \ + locales \ + make \ + meson \ + ninja-build \ + pkgconf \ + python3 \ + python3-venv \ + sed \ + tar && \ + eatmydata apt-get autoremove -y && \ + eatmydata apt-get autoclean -y && \ + sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \ + dpkg-reconfigure locales -# Duplicate deb line as deb-src -RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" -RUN apt update && \ - DEBIAN_FRONTEND=noninteractive eatmydata \ - apt install -y --no-install-recommends \ - gcc-riscv64-linux-gnu \ - libc6-dev-riscv64-cross \ - libfdt-dev:riscv64 \ - libffi-dev:riscv64 \ - libglib2.0-dev:riscv64 \ - libpixman-1-dev:riscv64 +RUN export DEBIAN_FRONTEND=noninteractive && \ + dpkg --add-architecture riscv64 && \ + eatmydata apt-get install debian-ports-archive-keyring && \ + eatmydata echo 'deb http://ftp.ports.debian.org/debian-ports/ sid main' > /etc/apt/sources.list.d/ports.list && \ + eatmydata echo 'deb http://ftp.ports.debian.org/debian-ports/ unreleased main' >> /etc/apt/sources.list.d/ports.list && \ + eatmydata apt-get update && \ + eatmydata apt-get dist-upgrade -y && \ + eatmydata apt-get install --no-install-recommends -y dpkg-dev && \ + eatmydata apt-get install --no-install-recommends -y \ + g++-riscv64-linux-gnu \ + gcc-riscv64-linux-gnu \ + libc6-dev:riscv64 \ + libfdt-dev:riscv64 \ + libffi-dev:riscv64 \ + libglib2.0-dev:riscv64 \ + libpixman-1-dev:riscv64 && \ + eatmydata apt-get autoremove -y && \ + eatmydata apt-get autoclean -y && \ + mkdir -p /usr/local/share/meson/cross && \ + printf "[binaries]\n\ +c = '/usr/bin/riscv64-linux-gnu-gcc'\n\ +ar = '/usr/bin/riscv64-linux-gnu-gcc-ar'\n\ +strip = '/usr/bin/riscv64-linux-gnu-strip'\n\ +pkgconfig = '/usr/bin/riscv64-linux-gnu-pkg-config'\n\ +\n\ +[host_machine]\n\ +system = 'linux'\n\ +cpu_family = 'riscv64'\n\ +cpu = 'riscv64'\n\ +endian = 'little'\n" > /usr/local/share/meson/cross/riscv64-linux-gnu && \ + dpkg-query --showformat '${Package}_${Version}_${Architecture}\n' --show > /packages.txt && \ + mkdir -p /usr/libexec/ccache-wrappers && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/riscv64-linux-gnu-c++ && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/riscv64-linux-gnu-cc && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/riscv64-linux-gnu-g++ && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/riscv64-linux-gnu-gcc -# Specify the cross prefix for this image (see tests/docker/common.rc) +ENV ABI "riscv64-linux-gnu" +ENV MESON_OPTS "--cross-file=riscv64-linux-gnu" ENV QEMU_CONFIGURE_OPTS --cross-prefix=riscv64-linux-gnu- ENV DEF_TARGET_LIST riscv64-softmmu,riscv64-linux-user # As a final step configure the user (if env is defined) diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh index 5e06fb2..b54566e 100755 --- a/tests/lcitool/refresh +++ b/tests/lcitool/refresh @@ -63,12 +63,12 @@ add_user_mapping = [ " id ${USER} 2>/dev/null || useradd -u ${UID} -U ${USER}; fi\n" ] -def generate_dockerfile(host, target, cross=None, trailer=None): +def generate_dockerfile(host, target, project="qemu", cross=None, trailer=None): filename = Path(src_dir, "tests", "docker", "dockerfiles", host + ".docker") cmd = lcitool_cmd + ["dockerfile"] if cross is not None: cmd.extend(["--cross", cross]) - cmd.extend([target, "qemu"]) + cmd.extend([target, project]) if trailer is not None: trailer += "\n".join(add_user_mapping) @@ -164,6 +164,12 @@ try: trailer=cross_build("powerpc64le-linux-gnu-", "ppc64-softmmu,ppc64-linux-user")) + generate_dockerfile("debian-riscv64-cross", "debian-sid", + project="qemu-minimal", + cross="riscv64", + trailer=cross_build("riscv64-linux-gnu-", + "riscv64-softmmu,riscv64-linux-user")) + generate_dockerfile("debian-s390x-cross", "debian-11", cross="s390x", trailer=cross_build("s390x-linux-gnu-", -- cgit v1.1 From 9fe8fa02cda26a203b1844433c62df64eab41244 Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz Date: Fri, 30 Jun 2023 19:04:01 +0100 Subject: tests/avocado: update firmware to enable sbsa-ref/max MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update prebuilt firmware images to have TF-A with FEAT_FGT support enabled. This allowed us to enable test for "max" cpu in sbsa-ref machine. Signed-off-by: Marcin Juszkiewicz Message-Id: <20230530152240.79160-1-marcin.juszkiewicz@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-17-alex.bennee@linaro.org> --- tests/avocado/machine_aarch64_sbsaref.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/avocado/machine_aarch64_sbsaref.py b/tests/avocado/machine_aarch64_sbsaref.py index 0a79fa7..cce6ef9 100644 --- a/tests/avocado/machine_aarch64_sbsaref.py +++ b/tests/avocado/machine_aarch64_sbsaref.py @@ -29,23 +29,23 @@ class Aarch64SbsarefMachine(QemuSystemTest): """ Flash volumes generated using: - - Fedora GNU Toolchain version 12.2.1 20220819 (Red Hat Cross 12.2.1-2) + - Fedora GNU Toolchain version 13.1.1 20230511 (Red Hat 13.1.1-2) - Trusted Firmware-A - https://github.com/ARM-software/arm-trusted-firmware/tree/5fdb2e54 + https://github.com/ARM-software/arm-trusted-firmware/tree/c0d8ee38 - Tianocore EDK II - https://github.com/tianocore/edk2/tree/494127613b - https://github.com/tianocore/edk2-non-osi/tree/41876073 - https://github.com/tianocore/edk2-platforms/tree/8efa4f42 + https://github.com/tianocore/edk2/tree/0f9283429dd4 + https://github.com/tianocore/edk2-non-osi/tree/f0bb00937ad6 + https://github.com/tianocore/edk2-platforms/tree/7880b92e2a04 """ # Secure BootRom (TF-A code) fs0_xz_url = ( - "https://fileserver.linaro.org/s/ATnSmq6k8SoXgbH/" + "https://fileserver.linaro.org/s/HrYMCjP7MEccjRP/" "download/SBSA_FLASH0.fd.xz" ) - fs0_xz_hash = "a210a09692bcbe0a3743ffd0df44e80e0c7ad8ab" + fs0_xz_hash = "447eff64a90b84ce47703c6ec41fbfc25befaaea" tar_xz_path = self.fetch_asset(fs0_xz_url, asset_hash=fs0_xz_hash) archive.extract(tar_xz_path, self.workdir) fs0_path = os.path.join(self.workdir, "SBSA_FLASH0.fd") @@ -93,15 +93,15 @@ class Aarch64SbsarefMachine(QemuSystemTest): # AP Trusted ROM wait_for_console_pattern(self, "Booting Trusted Firmware") - wait_for_console_pattern(self, "BL1: v2.8(release):v2.8") + wait_for_console_pattern(self, "BL1: v2.9(release):v2.9") wait_for_console_pattern(self, "BL1: Booting BL2") # Trusted Boot Firmware - wait_for_console_pattern(self, "BL2: v2.8(release)") + wait_for_console_pattern(self, "BL2: v2.9(release)") wait_for_console_pattern(self, "Booting BL31") # EL3 Runtime Software - wait_for_console_pattern(self, "BL31: v2.8(release)") + wait_for_console_pattern(self, "BL31: v2.9(release)") # Non-trusted Firmware wait_for_console_pattern(self, "UEFI firmware (version 1.0") @@ -136,21 +136,18 @@ class Aarch64SbsarefMachine(QemuSystemTest): self.vm.launch() wait_for_console_pattern(self, "Welcome to Alpine Linux 3.17") - @skipUnless(os.getenv("AVOCADO_TIMEOUT_EXPECTED"), "Test might timeout") def test_sbsaref_alpine_linux_cortex_a57(self): """ :avocado: tags=cpu:cortex-a57 """ self.boot_alpine_linux("cortex-a57") - @skipUnless(os.getenv("AVOCADO_TIMEOUT_EXPECTED"), "Test might timeout") def test_sbsaref_alpine_linux_neoverse_n1(self): """ :avocado: tags=cpu:max """ self.boot_alpine_linux("neoverse-n1") - @skip("requires TF-A update to handle FEAT_FGT") def test_sbsaref_alpine_linux_max(self): """ :avocado: tags=cpu:max -- cgit v1.1 From 7ea47af3905d37565d34beb4a35bc1b7bdc8a885 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 30 Jun 2023 19:04:02 +0100 Subject: tests/avocado: Make the test_arm_bpim2u_gmac test more reliable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test_arm_bpim2u_gmac test sometimes fails (ca. 1 out of 20 runs here) since the disk shows up as /dev/mmcblk1 instead of /dev/mmcblk0 in some runs. No matter of the name in /dev, the major:minor encoding seems always to be the same, so we can fix this issue by using the correct major:minor hex number in the "root=" parameter instead. Signed-off-by: Thomas Huth Message-Id: <20230630161604.446394-1-thuth@redhat.com> Tested-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-18-alex.bennee@linaro.org> --- tests/avocado/boot_linux_console.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/avocado/boot_linux_console.py b/tests/avocado/boot_linux_console.py index 6ed6606..6eab515 100644 --- a/tests/avocado/boot_linux_console.py +++ b/tests/avocado/boot_linux_console.py @@ -869,7 +869,7 @@ class BootLinuxConsole(LinuxKernelTest): self.vm.set_console() kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0,115200 ' - 'root=/dev/mmcblk0 rootwait rw ' + 'root=b300 rootwait rw ' 'panic=-1 noreboot') self.vm.add_args('-kernel', kernel_path, '-dtb', dtb_path, @@ -885,7 +885,7 @@ class BootLinuxConsole(LinuxKernelTest): exec_command_and_wait_for_pattern(self, 'cat /proc/cpuinfo', 'Allwinner sun8i Family') exec_command_and_wait_for_pattern(self, 'cat /proc/partitions', - 'mmcblk0') + 'mmcblk') exec_command_and_wait_for_pattern(self, 'ifconfig eth0 up', 'eth0: Link is Up') exec_command_and_wait_for_pattern(self, 'udhcpc eth0', -- cgit v1.1 From 465af4db965322630b253691b569b40cf84a2dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:03 +0100 Subject: target/arm: make arm_casq_ptw CONFIG_TCG only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ptw code is accessed by non-TCG code (specifically arm_pamax and arm_cpu_get_phys_page_attrs_debug) but most of it is really only for TCG emulation. Seeing as we already assert for a non TARGET_AARCH64 build lets extend the test rather than further messing with the ifdef ladder. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-19-alex.bennee@linaro.org> --- target/arm/ptw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 6015121..42355ca 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -628,7 +628,7 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, uint64_t new_val, S1Translate *ptw, ARMMMUFaultInfo *fi) { -#ifdef TARGET_AARCH64 +#if defined(TARGET_AARCH64) && defined(CONFIG_TCG) uint64_t cur_val; void *host = ptw->out_host; @@ -709,7 +709,7 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, return cur_val; #else - /* AArch32 does not have FEAT_HADFS. */ + /* AArch32 does not have FEAT_HADFS; non-TCG guests only use debug-mode. */ g_assert_not_reached(); #endif } -- cgit v1.1 From 6d03226b42247b68ab2f0b3663e0f624335a4055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:04 +0100 Subject: plugins: force slow path when plugins instrument memory ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lack of SVE memory instrumentation has been an omission in plugin handling since it was introduced. Fortunately we can utilise the probe_* functions to force all all memory access to follow the slow path. We do this by checking the access type and presence of plugin memory callbacks and if set return the TLB_MMIO flag. We have to jump through a few hoops in user mode to re-use the flag but it was the desired effect: ./qemu-system-aarch64 -display none -serial mon:stdio \ -M virt -cpu max -semihosting-config enable=on \ -kernel ./tests/tcg/aarch64-softmmu/memory-sve \ -plugin ./contrib/plugins/libexeclog.so,ifilter=st1w,afilter=0x40001808 -d plugin gives (disas doesn't currently understand st1w): 0, 0x40001808, 0xe54342a0, ".byte 0xa0, 0x42, 0x43, 0xe5", store, 0x40213010, RAM, store, 0x40213014, RAM, store, 0x40213018, RAM And for user-mode: ./qemu-aarch64 \ -plugin contrib/plugins/libexeclog.so,afilter=0x4007c0 \ -d plugin \ ./tests/tcg/aarch64-linux-user/sha512-sve gives: 1..10 ok 1 - do_test(&tests[i]) 0, 0x4007c0, 0xa4004b80, ".byte 0x80, 0x4b, 0x00, 0xa4", load, 0x5500800370, load, 0x5500800371, load, 0x5500800372, load, 0x5500800373, load, 0x5500800374, load, 0x5500800375, load, 0x5500800376, load, 0x5500800377, load, 0x5500800378, load, 0x5500800379, load, 0x550080037a, load, 0x550080037b, load, 0x550080037c, load, 0x550080037d, load, 0x550080037e, load, 0x550080037f, load, 0x5500800380, load, 0x5500800381, load, 0x5500800382, load, 0x5500800383, load, 0x5500800384, load, 0x5500800385, load, 0x5500800386, lo ad, 0x5500800387, load, 0x5500800388, load, 0x5500800389, load, 0x550080038a, load, 0x550080038b, load, 0x550080038c, load, 0x550080038d, load, 0x550080038e, load, 0x550080038f, load, 0x5500800390, load, 0x5500800391, load, 0x5500800392, load, 0x5500800393, load, 0x5500800394, load, 0x5500800395, load, 0x5500800396, load, 0x5500800397, load, 0x5500800398, load, 0x5500800399, load, 0x550080039a, load, 0x550080039b, load, 0x550080039c, load, 0x550080039d, load, 0x550080039e, load, 0x550080039f, load, 0x55008003a0, load, 0x55008003a1, load, 0x55008003a2, load, 0x55008003a3, load, 0x55008003a4, load, 0x55008003a5, load, 0x55008003a6, load, 0x55008003a7, load, 0x55008003a8, load, 0x55008003a9, load, 0x55008003aa, load, 0x55008003ab, load, 0x55008003ac, load, 0x55008003ad, load, 0x55008003ae, load, 0x55008003af (4007c0 is the ld1b in the sha512-sve) Reviewed-by: Richard Henderson Cc: Robert Henry Cc: Aaron Lindsay Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-20-alex.bennee@linaro.org> --- accel/tcg/cputlb.c | 41 ++++++++++++++++++++++++++++++++------- accel/tcg/user-exec.c | 8 ++++++-- include/exec/cpu-all.h | 2 +- include/exec/exec-all.h | 23 ++++++++++++++++++++++ include/hw/core/cpu.h | 17 ++++++++++++++++ target/arm/ptw.c | 14 ++++++------- target/arm/tcg/sve_helper.c | 4 ---- tests/tcg/aarch64/Makefile.target | 8 ++++++++ 8 files changed, 96 insertions(+), 21 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 5b51eff..c2b81ec 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1513,13 +1513,14 @@ static int probe_access_internal(CPUArchState *env, vaddr addr, int fault_size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, CPUTLBEntryFull **pfull, - uintptr_t retaddr) + uintptr_t retaddr, bool check_mem_cbs) { uintptr_t index = tlb_index(env, mmu_idx, addr); CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr); uint64_t tlb_addr = tlb_read_idx(entry, access_type); vaddr page_addr = addr & TARGET_PAGE_MASK; int flags = TLB_FLAGS_MASK & ~TLB_FORCE_SLOW; + bool force_mmio = check_mem_cbs && cpu_plugin_mem_cbs_enabled(env_cpu(env)); CPUTLBEntryFull *full; if (!tlb_hit_page(tlb_addr, page_addr)) { @@ -1553,7 +1554,9 @@ static int probe_access_internal(CPUArchState *env, vaddr addr, flags |= full->slow_flags[access_type]; /* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */ - if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) { + if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY)) + || + (access_type != MMU_INST_FETCH && force_mmio)) { *phost = NULL; return TLB_MMIO; } @@ -1569,7 +1572,7 @@ int probe_access_full(CPUArchState *env, vaddr addr, int size, uintptr_t retaddr) { int flags = probe_access_internal(env, addr, size, access_type, mmu_idx, - nonfault, phost, pfull, retaddr); + nonfault, phost, pfull, retaddr, true); /* Handle clean RAM pages. */ if (unlikely(flags & TLB_NOTDIRTY)) { @@ -1580,6 +1583,29 @@ int probe_access_full(CPUArchState *env, vaddr addr, int size, return flags; } +int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size, + MMUAccessType access_type, int mmu_idx, + void **phost, CPUTLBEntryFull **pfull) +{ + void *discard_phost; + CPUTLBEntryFull *discard_tlb; + + /* privately handle users that don't need full results */ + phost = phost ? phost : &discard_phost; + pfull = pfull ? pfull : &discard_tlb; + + int flags = probe_access_internal(env, addr, size, access_type, mmu_idx, + true, phost, pfull, 0, false); + + /* Handle clean RAM pages. */ + if (unlikely(flags & TLB_NOTDIRTY)) { + notdirty_write(env_cpu(env), addr, 1, *pfull, 0); + flags &= ~TLB_NOTDIRTY; + } + + return flags; +} + int probe_access_flags(CPUArchState *env, vaddr addr, int size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, uintptr_t retaddr) @@ -1590,7 +1616,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size, g_assert(-(addr | TARGET_PAGE_MASK) >= size); flags = probe_access_internal(env, addr, size, access_type, mmu_idx, - nonfault, phost, &full, retaddr); + nonfault, phost, &full, retaddr, true); /* Handle clean RAM pages. */ if (unlikely(flags & TLB_NOTDIRTY)) { @@ -1611,7 +1637,7 @@ void *probe_access(CPUArchState *env, vaddr addr, int size, g_assert(-(addr | TARGET_PAGE_MASK) >= size); flags = probe_access_internal(env, addr, size, access_type, mmu_idx, - false, &host, &full, retaddr); + false, &host, &full, retaddr, true); /* Per the interface, size == 0 merely faults the access. */ if (size == 0) { @@ -1644,7 +1670,7 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr, int flags; flags = probe_access_internal(env, addr, 0, access_type, - mmu_idx, true, &host, &full, 0); + mmu_idx, true, &host, &full, 0, false); /* No combination of flags are expected by the caller. */ return flags ? NULL : host; @@ -1667,7 +1693,8 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr, void *p; (void)probe_access_internal(env, addr, 1, MMU_INST_FETCH, - cpu_mmu_index(env, true), false, &p, &full, 0); + cpu_mmu_index(env, true), false, + &p, &full, 0, false); if (p == NULL) { return -1; } diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 8fbcbf9..d95b875 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -745,6 +745,10 @@ static int probe_access_internal(CPUArchState *env, vaddr addr, if (guest_addr_valid_untagged(addr)) { int page_flags = page_get_flags(addr); if (page_flags & acc_flag) { + if ((acc_flag == PAGE_READ || acc_flag == PAGE_WRITE) + && cpu_plugin_mem_cbs_enabled(env_cpu(env))) { + return TLB_MMIO; + } return 0; /* success */ } maperr = !(page_flags & PAGE_VALID); @@ -767,7 +771,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size, g_assert(-(addr | TARGET_PAGE_MASK) >= size); flags = probe_access_internal(env, addr, size, access_type, nonfault, ra); - *phost = flags ? NULL : g2h(env_cpu(env), addr); + *phost = (flags & TLB_INVALID_MASK) ? NULL : g2h(env_cpu(env), addr); return flags; } @@ -778,7 +782,7 @@ void *probe_access(CPUArchState *env, vaddr addr, int size, g_assert(-(addr | TARGET_PAGE_MASK) >= size); flags = probe_access_internal(env, addr, size, access_type, false, ra); - g_assert(flags == 0); + g_assert((flags & ~TLB_MMIO) == 0); return size ? g2h(env_cpu(env), addr) : NULL; } diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 8018ce7..472fe9a 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -301,7 +301,7 @@ CPUArchState *cpu_copy(CPUArchState *env); * be signaled by probe_access_flags(). */ #define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS_MIN - 1)) -#define TLB_MMIO 0 +#define TLB_MMIO (1 << (TARGET_PAGE_BITS_MIN - 2)) #define TLB_WATCHPOINT 0 #else diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 200c27e..5fa0687 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -464,6 +464,29 @@ int probe_access_full(CPUArchState *env, vaddr addr, int size, MMUAccessType access_type, int mmu_idx, bool nonfault, void **phost, CPUTLBEntryFull **pfull, uintptr_t retaddr); + +/** + * probe_access_mmu() - Like probe_access_full except cannot fault and + * doesn't trigger instrumentation. + * + * @env: CPUArchState + * @vaddr: virtual address to probe + * @size: size of the probe + * @access_type: read, write or execute permission + * @mmu_idx: softmmu index + * @phost: ptr to return value host address or NULL + * @pfull: ptr to return value CPUTLBEntryFull structure or NULL + * + * The CPUTLBEntryFull structure returned via @pfull is transient + * and must be consumed or copied immediately, before any further + * access or changes to TLB @mmu_idx. + * + * Returns: TLB flags as per probe_access_flags() + */ +int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size, + MMUAccessType access_type, int mmu_idx, + void **phost, CPUTLBEntryFull **pfull); + #endif /* Hide the qatomic_read to make code a little easier on the eyes */ diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index b08f8b7..fdcbe87 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -977,6 +977,23 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask); #endif /** + * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled? + * @cs: CPUState pointer + * + * The memory callbacks are installed if a plugin has instrumented an + * instruction for memory. This can be useful to know if you want to + * force a slow path for a series of memory accesses. + */ +static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu) +{ +#ifdef CONFIG_PLUGIN + return !!cpu->plugin_mem_cbs; +#else + return false; +#endif +} + +/** * cpu_get_address_space: * @cpu: CPU to get address space from * @asidx: index identifying which address space to get diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 42355ca..9aaff15 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -489,9 +489,9 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, int flags; env->tlb_fi = fi; - flags = probe_access_full(env, addr, 0, MMU_DATA_LOAD, - arm_to_core_mmu_idx(s2_mmu_idx), - true, &ptw->out_host, &full, 0); + flags = probe_access_full_mmu(env, addr, 0, MMU_DATA_LOAD, + arm_to_core_mmu_idx(s2_mmu_idx), + &ptw->out_host, &full); env->tlb_fi = NULL; if (unlikely(flags & TLB_INVALID_MASK)) { @@ -644,12 +644,12 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val, */ if (unlikely(!ptw->out_rw)) { int flags; - void *discard; env->tlb_fi = fi; - flags = probe_access_flags(env, ptw->out_virt, 0, MMU_DATA_STORE, - arm_to_core_mmu_idx(ptw->in_ptw_idx), - true, &discard, 0); + flags = probe_access_full_mmu(env, ptw->out_virt, 0, + MMU_DATA_STORE, + arm_to_core_mmu_idx(ptw->in_ptw_idx), + NULL, NULL); env->tlb_fi = NULL; if (unlikely(flags & TLB_INVALID_MASK)) { diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c index 0097522..7c103fc 100644 --- a/target/arm/tcg/sve_helper.c +++ b/target/arm/tcg/sve_helper.c @@ -5688,9 +5688,6 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr, flags = info.page[0].flags | info.page[1].flags; if (unlikely(flags != 0)) { -#ifdef CONFIG_USER_ONLY - g_assert_not_reached(); -#else /* * At least one page includes MMIO. * Any bus operation can fail with cpu_transaction_failed, @@ -5727,7 +5724,6 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr, memcpy(&env->vfp.zregs[(rd + i) & 31], &scratch[i], reg_max); } return; -#endif } /* The entire operation is in RAM, on valid pages. */ diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 3430fd3..cec1d4b 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -91,6 +91,14 @@ sha512-vector: sha512.c TESTS += sha512-vector +ifneq ($(CROSS_CC_HAS_SVE),) +sha512-sve: CFLAGS=-O3 -march=armv8.1-a+sve +sha512-sve: sha512.c + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) + +TESTS += sha512-sve +endif + ifeq ($(HOST_GDB_SUPPORTS_ARCH),y) GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py -- cgit v1.1 From 402587419993d86638f1f81fc4e55625a320ad2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:05 +0100 Subject: plugins: fix memory leak while parsing options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was hard to track down this leak as it was an internal allocation by glib and the backtraces did not give much away. The autofree was freeing the allocation with g_free() but not taking care of the individual strings. They should have been freed with g_strfreev() instead. Searching the glib source code for the correct string free function led to: G_DEFINE_AUTO_CLEANUP_FREE_FUNC(GStrv, g_strfreev, NULL) and indeed if you read to the bottom of the documentation page you will find: typedef gchar** GStrv; A typedef alias for gchar**. This is mostly useful when used together with g_auto(). So fix up all the g_autofree g_strsplit case that smugly thought they had de-allocation covered. Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-21-alex.bennee@linaro.org> --- contrib/plugins/cache.c | 2 +- contrib/plugins/drcov.c | 2 +- contrib/plugins/execlog.c | 2 +- contrib/plugins/hotblocks.c | 2 +- contrib/plugins/hotpages.c | 2 +- contrib/plugins/howvec.c | 2 +- contrib/plugins/hwprofile.c | 2 +- contrib/plugins/lockstep.c | 2 +- tests/plugin/bb.c | 2 +- tests/plugin/insn.c | 2 +- tests/plugin/mem.c | 2 +- tests/plugin/syscall.c | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c index 2e25184..5036213 100644 --- a/contrib/plugins/cache.c +++ b/contrib/plugins/cache.c @@ -772,7 +772,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "iblksize") == 0) { l1_iblksize = STRTOLL(tokens[1]); diff --git a/contrib/plugins/drcov.c b/contrib/plugins/drcov.c index b4a855a..686ae0a 100644 --- a/contrib/plugins/drcov.c +++ b/contrib/plugins/drcov.c @@ -148,7 +148,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, int argc, char **argv) { for (int i = 0; i < argc; i++) { - g_autofree char **tokens = g_strsplit(argv[i], "=", 2); + g_auto(GStrv) tokens = g_strsplit(argv[i], "=", 2); if (g_strcmp0(tokens[0], "filename") == 0) { file_name = g_strdup(tokens[1]); } diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c index e255bd2..7129d52 100644 --- a/contrib/plugins/execlog.c +++ b/contrib/plugins/execlog.c @@ -227,7 +227,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "ifilter") == 0) { parse_insn_match(tokens[1]); } else if (g_strcmp0(tokens[0], "afilter") == 0) { diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c index 062200a..6b74d25 100644 --- a/contrib/plugins/hotblocks.c +++ b/contrib/plugins/hotblocks.c @@ -135,7 +135,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, { for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c index 0d12910..8316ae5 100644 --- a/contrib/plugins/hotpages.c +++ b/contrib/plugins/hotpages.c @@ -169,7 +169,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", -1); + g_auto(GStrv) tokens = g_strsplit(opt, "=", -1); if (g_strcmp0(tokens[0], "sortby") == 0) { if (g_strcmp0(tokens[1], "reads") == 0) { diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c index 4a5ec3d..0ed01ea 100644 --- a/contrib/plugins/howvec.c +++ b/contrib/plugins/howvec.c @@ -333,7 +333,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *p = argv[i]; - g_autofree char **tokens = g_strsplit(p, "=", -1); + g_auto(GStrv) tokens = g_strsplit(p, "=", -1); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", p); diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c index 691d4ed..739ac0c 100644 --- a/contrib/plugins/hwprofile.c +++ b/contrib/plugins/hwprofile.c @@ -263,7 +263,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "track") == 0) { if (g_strcmp0(tokens[1], "read") == 0) { diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index a41ffe8..e36f0b9 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -323,7 +323,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *p = argv[i]; - g_autofree char **tokens = g_strsplit(p, "=", 2); + g_auto(GStrv) tokens = g_strsplit(p, "=", 2); if (g_strcmp0(tokens[0], "verbose") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &verbose)) { diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c index 7d470a1..df50d1f 100644 --- a/tests/plugin/bb.c +++ b/tests/plugin/bb.c @@ -104,7 +104,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c index 9bd6e44..5fd3017 100644 --- a/tests/plugin/insn.c +++ b/tests/plugin/insn.c @@ -189,7 +189,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, { for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c index 4570f7d..f3b9f69 100644 --- a/tests/plugin/mem.c +++ b/tests/plugin/mem.c @@ -83,7 +83,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "haddr") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_haddr)) { diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c index 96040c5..72e1a5b 100644 --- a/tests/plugin/syscall.c +++ b/tests/plugin/syscall.c @@ -121,7 +121,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "print") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_print)) { -- cgit v1.1 From ee293103b061b39c84334a29eb596e1e07a308da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:06 +0100 Subject: plugins: update lockstep to use g_memdup2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old g_memdup is deprecated, use the replacement. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-22-alex.bennee@linaro.org> --- contrib/plugins/lockstep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index e36f0b9..3614c35 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -130,7 +130,7 @@ static void report_divergance(ExecState *us, ExecState *them) } } divergence_log = g_slist_prepend(divergence_log, - g_memdup(&divrec, sizeof(divrec))); + g_memdup2(&divrec, sizeof(divrec))); /* Output short log entry of going out of sync... */ if (verbose || divrec.distance == 1 || diverged) { -- cgit v1.1 From 8b4528c829a787533359de5550796e87070c9bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:07 +0100 Subject: docs/devel: add some front matter to the devel index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give an overview of the most useful bits of the devel documentation to read depending on what the developer wants to do. Reviewed-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-23-alex.bennee@linaro.org> --- docs/devel/index-process.rst | 2 ++ docs/devel/index-tcg.rst | 2 ++ docs/devel/index.rst | 24 ++++++++++++++++++++++-- docs/devel/tcg.rst | 2 ++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst index d50dd74..362f97e 100644 --- a/docs/devel/index-process.rst +++ b/docs/devel/index-process.rst @@ -1,3 +1,5 @@ +.. _development_process: + QEMU Community Processes ------------------------ diff --git a/docs/devel/index-tcg.rst b/docs/devel/index-tcg.rst index b44ff8b..a992844 100644 --- a/docs/devel/index-tcg.rst +++ b/docs/devel/index-tcg.rst @@ -1,3 +1,5 @@ +.. _tcg: + TCG Emulation ------------- diff --git a/docs/devel/index.rst b/docs/devel/index.rst index 09cfb32..abf6045 100644 --- a/docs/devel/index.rst +++ b/docs/devel/index.rst @@ -2,10 +2,30 @@ Developer Information --------------------- -This section of the manual documents various parts of the internals of QEMU. -You only need to read it if you are interested in reading or +This section of the manual documents various parts of the internals of +QEMU. You only need to read it if you are interested in reading or modifying QEMU's source code. +QEMU is a large and mature project with a number of complex subsystems +that can be overwhelming to understand. The development documentation +is not comprehensive but hopefully presents enough to get you started. +If there are areas that are unclear please reach out either via the +IRC channel or mailing list and hopefully we can improve the +documentation for future developers. + +All developers will want to familiarise themselves with +:ref:`development_process` and how the community interacts. Please pay +particular attention to the :ref:`coding-style` and +:ref:`submitting-a-patch` sections to avoid common pitfalls. + +If you wish to implement a new hardware model you will want to read +through the :ref:`qom` documentation to understand how QEMU's object +model works. + +Those wishing to enhance or add new CPU emulation capabilities will +want to read our :ref:`tcg` documentation, especially the overview of +the :ref:`tcg_internals`. + .. toctree:: :maxdepth: 1 diff --git a/docs/devel/tcg.rst b/docs/devel/tcg.rst index b4096a1..2786f2f 100644 --- a/docs/devel/tcg.rst +++ b/docs/devel/tcg.rst @@ -1,3 +1,5 @@ +.. _tcg_internals: + ==================== Translator Internals ==================== -- cgit v1.1 From 6caf1571a97f38b2c14eca855941a0455b461041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:08 +0100 Subject: include/migration: mark vmstate_register() as a legacy function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mention that QOM-ified devices already have support for registering the description. Reviewed-by: Juan Quintela Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Acked-by: Peter Xu Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-24-alex.bennee@linaro.org> --- include/migration/vmstate.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 084f5e7..d1b8abe 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -1209,7 +1209,15 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id, int required_for_version, Error **errp); -/* Returns: 0 on success, -1 on failure */ +/** + * vmstate_register() - legacy function to register state + * serialisation description + * + * New code shouldn't be using this function as QOM-ified devices have + * dc->vmsd to store the serialisation description. + * + * Returns: 0 on success, -1 on failure + */ static inline int vmstate_register(VMStateIf *obj, int instance_id, const VMStateDescription *vmsd, void *opaque) -- cgit v1.1 From 6aebb1f62b2c249cae175cdaf0611b9feaf50788 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:09 +0100 Subject: include/hw/qdev-core: fixup kerneldoc annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix up the kerneldoc markup and start documenting the various fields in QDEV related structures. This involved: - moving overall description to a DOC: comment at top - fixing various markup issues for types and structures - adding missing Return: statements - adding some typedefs to hide QLIST macros in headers Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-25-alex.bennee@linaro.org> --- include/hw/qdev-core.h | 368 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 254 insertions(+), 114 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index f1070d6..06cadfc 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -10,6 +10,65 @@ #include "hw/hotplug.h" #include "hw/resettable.h" +/** + * DOC: The QEMU Device API + * + * All modern devices should represented as a derived QOM class of + * TYPE_DEVICE. The device API introduces the additional methods of + * @realize and @unrealize to represent additional stages in a device + * objects life cycle. + * + * Realization + * ----------- + * + * Devices are constructed in two stages: + * + * 1) object instantiation via object_initialize() and + * 2) device realization via the #DeviceState.realized property + * + * The former may not fail (and must not abort or exit, since it is called + * during device introspection already), and the latter may return error + * information to the caller and must be re-entrant. + * Trivial field initializations should go into #TypeInfo.instance_init. + * Operations depending on @props static properties should go into @realize. + * After successful realization, setting static properties will fail. + * + * As an interim step, the #DeviceState.realized property can also be + * set with qdev_realize(). In the future, devices will propagate this + * state change to their children and along busses they expose. The + * point in time will be deferred to machine creation, so that values + * set in @realize will not be introspectable beforehand. Therefore + * devices must not create children during @realize; they should + * initialize them via object_initialize() in their own + * #TypeInfo.instance_init and forward the realization events + * appropriately. + * + * Any type may override the @realize and/or @unrealize callbacks but needs + * to call the parent type's implementation if keeping their functionality + * is desired. Refer to QOM documentation for further discussion and examples. + * + * .. note:: + * Since TYPE_DEVICE doesn't implement @realize and @unrealize, types + * derived directly from it need not call their parent's @realize and + * @unrealize. For other types consult the documentation and + * implementation of the respective parent types. + * + * Hiding a device + * --------------- + * + * To hide a device, a DeviceListener function hide_device() needs to + * be registered. It can be used to defer adding a device and + * therefore hide it from the guest. The handler registering to this + * DeviceListener can save the QOpts passed to it for re-using it + * later. It must return if it wants the device to be hidden or + * visible. When the handler function decides the device shall be + * visible it will be added with qdev_device_add() and realized as any + * other device. Otherwise qdev_device_add() will return early without + * adding the device. The guest will not see a "hidden" device until + * it was marked visible and qdev_device_add called again. + * + */ + enum { DEV_NVECTORS_UNSPECIFIED = -1, }; @@ -38,7 +97,7 @@ typedef void (*BusRealize)(BusState *bus, Error **errp); typedef void (*BusUnrealize)(BusState *bus); /** - * DeviceClass: + * struct DeviceClass - The base class for all devices. * @props: Properties accessing state fields. * @realize: Callback function invoked when the #DeviceState:realized * property is changed to %true. @@ -47,72 +106,37 @@ typedef void (*BusUnrealize)(BusState *bus); * @hotpluggable: indicates if #DeviceClass is hotpluggable, available * as readonly "hotpluggable" property of #DeviceState instance * - * # Realization # - * Devices are constructed in two stages, - * 1) object instantiation via object_initialize() and - * 2) device realization via #DeviceState:realized property. - * The former may not fail (and must not abort or exit, since it is called - * during device introspection already), and the latter may return error - * information to the caller and must be re-entrant. - * Trivial field initializations should go into #TypeInfo.instance_init. - * Operations depending on @props static properties should go into @realize. - * After successful realization, setting static properties will fail. - * - * As an interim step, the #DeviceState:realized property can also be - * set with qdev_realize(). - * In the future, devices will propagate this state change to their children - * and along busses they expose. - * The point in time will be deferred to machine creation, so that values - * set in @realize will not be introspectable beforehand. Therefore devices - * must not create children during @realize; they should initialize them via - * object_initialize() in their own #TypeInfo.instance_init and forward the - * realization events appropriately. - * - * Any type may override the @realize and/or @unrealize callbacks but needs - * to call the parent type's implementation if keeping their functionality - * is desired. Refer to QOM documentation for further discussion and examples. - * - * - * - * Since TYPE_DEVICE doesn't implement @realize and @unrealize, types - * derived directly from it need not call their parent's @realize and - * @unrealize. - * For other types consult the documentation and implementation of the - * respective parent types. - * - * - * - * # Hiding a device # - * To hide a device, a DeviceListener function hide_device() needs to - * be registered. - * It can be used to defer adding a device and therefore hide it from - * the guest. The handler registering to this DeviceListener can save - * the QOpts passed to it for re-using it later. It must return if it - * wants the device to be hidden or visible. When the handler function - * decides the device shall be visible it will be added with - * qdev_device_add() and realized as any other device. Otherwise - * qdev_device_add() will return early without adding the device. The - * guest will not see a "hidden" device until it was marked visible - * and qdev_device_add called again. - * */ struct DeviceClass { - /*< private >*/ + /* private: */ ObjectClass parent_class; - /*< public >*/ + /* public: */ + + /** + * @categories: device categories device belongs to + */ DECLARE_BITMAP(categories, DEVICE_CATEGORY_MAX); + /** + * @fw_name: name used to identify device to firmware interfaces + */ const char *fw_name; + /** + * @desc: human readable description of device + */ const char *desc; - /* - * The underscore at the end ensures a compile-time error if someone - * assigns to dc->props instead of using device_class_set_props. + /** + * @props_: properties associated with device, should only be + * assigned by using device_class_set_props(). The underscore + * ensures a compile-time error if someone attempts to assign + * dc->props directly. */ Property *props_; - /* - * Can this device be instantiated with -device / device_add? + /** + * @user_creatable: Can user instantiate with -device / device_add? + * * All devices should support instantiation with device_add, and * this flag should not exist. But we're not there, yet. Some * devices fail to instantiate with cryptic error messages. @@ -120,25 +144,35 @@ struct DeviceClass { * behavior would be cruel; clearing this flag will protect them. * It should never be cleared without a comment explaining why it * is cleared. + * * TODO remove once we're there */ bool user_creatable; bool hotpluggable; /* callbacks */ - /* - * Reset method here is deprecated and replaced by methods in the - * resettable class interface to implement a multi-phase reset. + /** + * @reset: deprecated device reset method pointer + * + * Modern code should use the ResettableClass interface to + * implement a multi-phase reset. + * * TODO: remove once every reset callback is unused */ DeviceReset reset; DeviceRealize realize; DeviceUnrealize unrealize; - /* device state */ + /** + * @vmsd: device state serialisation description for + * migration/save/restore + */ const VMStateDescription *vmsd; - /* Private to qdev / bus. */ + /** + * @bus_type: bus type + * private: to qdev / bus. + */ const char *bus_type; }; @@ -167,37 +201,96 @@ typedef struct { bool engaged_in_io; } MemReentrancyGuard; + +typedef QLIST_HEAD(, NamedGPIOList) NamedGPIOListHead; +typedef QLIST_HEAD(, NamedClockList) NamedClockListHead; +typedef QLIST_HEAD(, BusState) BusStateHead; + /** - * DeviceState: - * @reset: ResettableState for the device; handled by Resettable interface. + * struct DeviceState - common device state, accessed with qdev helpers * * This structure should not be accessed directly. We declare it here * so that it can be embedded in individual device state structures. */ struct DeviceState { - /*< private >*/ + /* private: */ Object parent_obj; - /*< public >*/ + /* public: */ + /** + * @id: global device id + */ char *id; + /** + * @canonical_path: canonical path of realized device in the QOM tree + */ char *canonical_path; + /** + * @realized: has device been realized? + */ bool realized; + /** + * @pending_deleted_event: track pending deletion events during unplug + */ bool pending_deleted_event; + /** + * @pending_deleted_expires_ms: optional timeout for deletion events + */ int64_t pending_deleted_expires_ms; + /** + * @opts: QDict of options for the device + */ QDict *opts; + /** + * @hotplugged: was device added after PHASE_MACHINE_READY? + */ int hotplugged; + /** + * @allow_unplug_during_migration: can device be unplugged during migration + */ bool allow_unplug_during_migration; + /** + * @parent_bus: bus this device belongs to + */ BusState *parent_bus; - QLIST_HEAD(, NamedGPIOList) gpios; - QLIST_HEAD(, NamedClockList) clocks; - QLIST_HEAD(, BusState) child_bus; + /** + * @gpios: QLIST of named GPIOs the device provides. + */ + NamedGPIOListHead gpios; + /** + * @clocks: QLIST of named clocks the device provides. + */ + NamedClockListHead clocks; + /** + * @child_bus: QLIST of child buses + */ + BusStateHead child_bus; + /** + * @num_child_bus: number of @child_bus entries + */ int num_child_bus; + /** + * @instance_id_alias: device alias for handling legacy migration setups + */ int instance_id_alias; + /** + * @alias_required_for_version: indicates @instance_id_alias is + * needed for migration + */ int alias_required_for_version; + /** + * @reset: ResettableState for the device; handled by Resettable interface. + */ ResettableState reset; + /** + * @unplug_blockers: list of reasons to block unplugging of device + */ GSList *unplug_blockers; - - /* Is the device currently in mmio/pio/dma? Used to prevent re-entrancy */ + /** + * @mem_reentrancy_guard: Is the device currently in mmio/pio/dma? + * + * Used to prevent re-entrancy confusing things. + */ MemReentrancyGuard mem_reentrancy_guard; }; @@ -264,13 +357,24 @@ typedef struct BusChild { #define QDEV_HOTPLUG_HANDLER_PROPERTY "hotplug-handler" +typedef QTAILQ_HEAD(, BusChild) BusChildHead; +typedef QLIST_ENTRY(BusState) BusStateEntry; + /** - * BusState: + * struct BusState: + * @obj: parent object + * @parent: parent Device + * @name: name of bus * @hotplug_handler: link to a hotplug handler associated with bus. - * @reset: ResettableState for the bus; handled by Resettable interface. + * @max_index: max number of child buses + * @realized: is the bus itself realized? + * @full: is the bus full? + * @num_children: current number of child buses */ struct BusState { + /* private: */ Object obj; + /* public: */ DeviceState *parent; char *name; HotplugHandler *hotplug_handler; @@ -279,18 +383,24 @@ struct BusState { bool full; int num_children; - /* - * children is a RCU QTAILQ, thus readers must use RCU to access it, - * and writers must hold the big qemu lock + /** + * @children: an RCU protected QTAILQ, thus readers must use RCU + * to access it, and writers must hold the big qemu lock + */ + BusChildHead children; + /** + * @sibling: next bus + */ + BusStateEntry sibling; + /** + * @reset: ResettableState for the bus; handled by Resettable interface. */ - - QTAILQ_HEAD(, BusChild) children; - QLIST_ENTRY(BusState) sibling; ResettableState reset; }; /** - * GlobalProperty: + * typedef GlobalProperty - a global property type + * * @used: Set to true if property was used when initializing a device. * @optional: If set to true, GlobalProperty will be skipped without errors * if the property doesn't exist. @@ -324,7 +434,8 @@ compat_props_add(GPtrArray *arr, * This only allocates the memory and initializes the device state * structure, ready for the caller to set properties if they wish. * The device still needs to be realized. - * The returned object has a reference count of 1. + * + * Return: a derived DeviceState object with a reference count of 1. */ DeviceState *qdev_new(const char *name); @@ -334,16 +445,18 @@ DeviceState *qdev_new(const char *name); * * This is like qdev_new(), except it returns %NULL when type @name * does not exist, rather than asserting. + * + * Return: a derived DeviceState object with a reference count of 1 or + * NULL if type @name does not exist. */ DeviceState *qdev_try_new(const char *name); /** - * qdev_is_realized: + * qdev_is_realized() - check if device is realized * @dev: The device to check. * - * May be called outside big qemu lock. - * - * Returns: %true% if the device has been fully constructed, %false% otherwise. + * Context: May be called outside big qemu lock. + * Return: true if the device has been fully constructed, false otherwise. */ static inline bool qdev_is_realized(DeviceState *dev) { @@ -361,11 +474,11 @@ static inline bool qdev_is_realized(DeviceState *dev) * @dev must not be plugged into a bus already. * If @bus, plug @dev into @bus. This takes a reference to @dev. * If @dev has no QOM parent, make one up, taking another reference. - * On success, return true. - * On failure, store an error through @errp and return false. * * If you created @dev using qdev_new(), you probably want to use * qdev_realize_and_unref() instead. + * + * Return: true on success, else false setting @errp with error */ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp); @@ -392,6 +505,8 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp); * for the only reference to the child device to be held by the parent * via the child<> property, and so the reference-count-drop done here * would be incorrect. For that use case you want qdev_realize(). + * + * Return: true on success, else false setting @errp with error */ bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp); @@ -420,16 +535,16 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev); HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); bool qdev_hotplug_allowed(DeviceState *dev, Error **errp); + /** - * qdev_get_hotplug_handler: Get handler responsible for device wiring - * - * Find HOTPLUG_HANDLER for @dev that provides [pre|un]plug callbacks for it. + * qdev_get_hotplug_handler() - Get handler responsible for device wiring + * @dev: the device we want the HOTPLUG_HANDLER for. * * Note: in case @dev has a parent bus, it will be returned as handler unless * machine handler overrides it. * - * Returns: pointer to object that implements TYPE_HOTPLUG_HANDLER interface - * or NULL if there aren't any. + * Return: pointer to object that implements TYPE_HOTPLUG_HANDLER interface + * or NULL if there aren't any. */ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); void qdev_unplug(DeviceState *dev, Error **errp); @@ -459,15 +574,15 @@ void qdev_del_unplug_blocker(DeviceState *dev, Error *reason); * qdev_unplug_blocked: Confirm if a device is blocked from unplug * * @dev: Device to be tested - * @reason: Returns one of the reasons why the device is blocked, - * if any + * @errp: The reasons why the device is blocked, if any * - * Returns: true if device is blocked from unplug, false otherwise + * Returns: true (also setting @errp) if device is blocked from unplug, + * false otherwise */ bool qdev_unplug_blocked(DeviceState *dev, Error **errp); /** - * GpioPolarity: Polarity of a GPIO line + * typedef GpioPolarity - Polarity of a GPIO line * * GPIO lines use either positive (active-high) logic, * or negative (active-low) logic. @@ -499,6 +614,8 @@ typedef enum { * connect another device's output GPIO line to this input. * * For named input GPIO lines, use qdev_get_gpio_in_named(). + * + * Return: qemu_irq corresponding to anonymous input GPIO line */ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n); @@ -516,6 +633,8 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n); * array); this function will assert() if passed an invalid name or index. * * For anonymous input GPIO lines, use qdev_get_gpio_in(). + * + * Return: qemu_irq corresponding to named input GPIO line */ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n); @@ -523,7 +642,7 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n); * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines * @dev: Device whose GPIO to connect * @n: Number of the anonymous output GPIO line (which must be in range) - * @input_pin: qemu_irq to connect the output line to + * @pin: qemu_irq to connect the output line to * * This function connects an anonymous output GPIO line on a device * up to an arbitrary qemu_irq, so that when the device asserts that @@ -594,6 +713,8 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n, * * You probably don't need to use this function -- it is used only * by the platform-bus subsystem. + * + * Return: qemu_irq associated with GPIO or NULL if un-wired. */ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n); @@ -604,14 +725,17 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n); * @name: Name of the output GPIO array * @n: Number of the GPIO line in the array * - * This function is provided only for use by the qtest testing framework - * and is not suitable for use in non-testing parts of QEMU. + * .. note:: + * This function is provided only for use by the qtest testing framework + * and is not suitable for use in non-testing parts of QEMU. * * This function breaks an existing connection of an outbound GPIO * line from @dev, and replaces it with the new qemu_irq @icpt, as if * ``qdev_connect_gpio_out_named(dev, icpt, name, n)`` had been called. * The previously connected qemu_irq is returned, so it can be restored * by a second call to qdev_intercept_gpio_out() if desired. + * + * Return: old disconnected qemu_irq if one existed */ qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt, const char *name, int n); @@ -683,9 +807,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins, const char *name, int n); /** - * qdev_init_gpio_in_named_with_opaque: create an array of input GPIO lines - * for the specified device - * + * qdev_init_gpio_in_named_with_opaque() - create an array of input GPIO lines * @dev: Device to create input GPIOs for * @handler: Function to call when GPIO line value is set * @opaque: Opaque data pointer to pass to @handler @@ -698,8 +820,11 @@ void qdev_init_gpio_in_named_with_opaque(DeviceState *dev, const char *name, int n); /** - * qdev_init_gpio_in_named: create an array of input GPIO lines - * for the specified device + * qdev_init_gpio_in_named() - create an array of input GPIO lines + * @dev: device to add array to + * @handler: a &typedef qemu_irq_handler function to call when GPIO is set + * @name: Name of the GPIO input (must be unique for this device) + * @n: Number of GPIO lines in this input set * * Like qdev_init_gpio_in_named_with_opaque(), but the opaque pointer * passed to the handler is @dev (which is the most commonly desired behaviour). @@ -762,14 +887,17 @@ int qdev_walk_children(DeviceState *dev, void *opaque); /** - * device_cold_reset: + * device_cold_reset() - perform a recursive cold reset on a device + * @dev: device to reset. + * * Reset device @dev and perform a recursive processing using the resettable * interface. It triggers a RESET_TYPE_COLD. */ void device_cold_reset(DeviceState *dev); /** - * bus_cold_reset: + * bus_cold_reset() - perform a recursive cold reset on a bus + * @bus: bus to reset * * Reset bus @bus and perform a recursive processing using the resettable * interface. It triggers a RESET_TYPE_COLD. @@ -777,14 +905,18 @@ void device_cold_reset(DeviceState *dev); void bus_cold_reset(BusState *bus); /** - * device_is_in_reset: - * Return true if the device @dev is currently being reset. + * device_is_in_reset() - check device reset state + * @dev: device to check + * + * Return: true if the device @dev is currently being reset. */ bool device_is_in_reset(DeviceState *dev); /** - * bus_is_in_reset: - * Return true if the bus @bus is currently being reset. + * bus_is_in_reset() - check bus reset state + * @bus: bus to check + * + * Return: true if the bus @bus is currently being reset. */ bool bus_is_in_reset(BusState *bus); @@ -797,7 +929,14 @@ char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, DeviceState *dev); void device_class_set_props(DeviceClass *dc, Property *props); /** - * device_class_set_parent_reset: + * device_class_set_parent_reset() - legacy set device reset handlers + * @dc: device class + * @dev_reset: function pointer to reset handler + * @parent_reset: function pointer to parents reset handler + * + * Modern code should use the ResettableClass interface to + * implement a multi-phase reset instead. + * * TODO: remove the function when DeviceClass's reset method * is not used anymore. */ @@ -871,14 +1010,15 @@ void device_listener_register(DeviceListener *listener); void device_listener_unregister(DeviceListener *listener); /** - * @qdev_should_hide_device: + * qdev_should_hide_device() - check if device should be hidden + * * @opts: options QDict * @from_json: true if @opts entries are typed, false for all strings * @errp: pointer to error object * - * Check if a device should be added. - * When a device is added via qdev_device_add() this will be called, - * and return if the device should be added now or not. + * When a device is added via qdev_device_add() this will be called. + * + * Return: if the device should be added now or not. */ bool qdev_should_hide_device(const QDict *opts, bool from_json, Error **errp); -- cgit v1.1 From 307c0a4ac9c7acb7937ef663b2369f081ba94cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 30 Jun 2023 19:04:10 +0100 Subject: docs/devel/qom.rst: Correct code style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per commit 067109a11c ("docs/devel: mention the spacing requirement for QOM"): For a storage structure the first declaration should always be called “parent_obj” and for a class structure the first member should always be called “parent_class” Adapt the QOM rST document accordingly. Reported-by: BALATON Zoltan Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20230622101717.70468-1-philmd@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-26-alex.bennee@linaro.org> --- docs/devel/qom.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index c923795..2828843 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -26,7 +26,7 @@ features: typedef DeviceClass MyDeviceClass; typedef struct MyDevice { - DeviceState parent; + DeviceState parent_obj; int reg0, reg1, reg2; } MyDevice; @@ -147,7 +147,7 @@ will also have a wrapper function to call it easily: typedef struct MyDeviceClass { - DeviceClass parent; + DeviceClass parent_class; void (*frobnicate) (MyDevice *obj); } MyDeviceClass; -- cgit v1.1 From da966a8a7ccb3811546bf141e17e928ea291256e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:11 +0100 Subject: docs/devel: split qom-api reference into new file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets try and keep the overview of the sub-system digestible by splitting the core API stuff into a separate file. As QOM and QDEV work together we should also try and enumerate the qdev_ functions. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-27-alex.bennee@linaro.org> --- docs/devel/index-api.rst | 2 ++ docs/devel/qdev-api.rst | 7 +++++++ docs/devel/qom-api.rst | 9 +++++++++ docs/devel/qom.rst | 3 ++- 4 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 docs/devel/qdev-api.rst create mode 100644 docs/devel/qom-api.rst diff --git a/docs/devel/index-api.rst b/docs/devel/index-api.rst index 7108821..539ad29 100644 --- a/docs/devel/index-api.rst +++ b/docs/devel/index-api.rst @@ -11,5 +11,7 @@ generated from in-code annotations to function prototypes. loads-stores memory modules + qom-api + qdev-api ui zoned-storage diff --git a/docs/devel/qdev-api.rst b/docs/devel/qdev-api.rst new file mode 100644 index 0000000..3f35eea --- /dev/null +++ b/docs/devel/qdev-api.rst @@ -0,0 +1,7 @@ +.. _qdev-api: + +================================ +QEMU Device (qdev) API Reference +================================ + +.. kernel-doc:: include/hw/qdev-core.h diff --git a/docs/devel/qom-api.rst b/docs/devel/qom-api.rst new file mode 100644 index 0000000..ed1f17e --- /dev/null +++ b/docs/devel/qom-api.rst @@ -0,0 +1,9 @@ +.. _qom-api: + +===================================== +QEMU Object Model (QOM) API Reference +===================================== + +This is the complete API documentation for :ref:`qom`. + +.. kernel-doc:: include/qom/object.h diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 2828843..c342ce1 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -387,4 +387,5 @@ OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead: API Reference ------------- -.. kernel-doc:: include/qom/object.h +See the :ref:`QOM API` and :ref:`QDEV API` +documents for the complete API description. -- cgit v1.1 From 43398409ce1a7fa83856f6141204aa103f0fe72c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:12 +0100 Subject: docs/devel: introduce some key concepts for QOM development MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using QOM correctly is increasingly important to maintaining a modern code base. However the current documentation skips some important concepts before launching into a simple example. Lets: - at least mention properties - mention TYPE_OBJECT and TYPE_DEVICE - talk about why we have realize/unrealize - mention the QOM tree - lightly re-arrange the order we mention things Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-28-alex.bennee@linaro.org> --- docs/devel/qom.rst | 58 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index c342ce1..0b50642 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -13,6 +13,24 @@ features: - System for dynamically registering types - Support for single-inheritance of types - Multiple inheritance of stateless interfaces +- Mapping internal members to publicly exposed properties + +The root object class is TYPE_OBJECT which provides for the basic +object methods. + +The QOM tree +============ + +The QOM tree is a composition tree which represents all of the objects +that make up a QEMU "machine". You can view this tree by running +``info qom-tree`` in the :ref:`QEMU monitor`. It will contain both +objects created by the machine itself as well those created due to +user configuration. + +Creating a QOM class +==================== + +A simple minimal device implementation may look something like bellow: .. code-block:: c :caption: Creating a minimal type @@ -48,6 +66,12 @@ In the above example, we create a simple type that is described by #TypeInfo. #TypeInfo describes information about the type including what it inherits from, the instance and class size, and constructor/destructor hooks. +The TYPE_DEVICE class is the parent class for all modern devices +implemented in QEMU and adds some specific methods to handle QEMU +device model. This includes managing the lifetime of devices from +creation through to when they become visible to the guest and +eventually unrealized. + Alternatively several static types could be registered using helper macro DEFINE_TYPES() @@ -98,7 +122,7 @@ when the object is needed. module_obj(TYPE_MY_DEVICE); Class Initialization -==================== +-------------------- Before an object is initialized, the class for the object must be initialized. There is only one class object for all instance objects @@ -168,7 +192,7 @@ will also have a wrapper function to call it easily: } Interfaces -========== +---------- Interfaces allow a limited form of multiple inheritance. Instances are similar to normal types except for the fact that are only defined by @@ -182,7 +206,7 @@ an argument to a method on its corresponding SomethingIfClass, or to dynamically cast it to an object that implements the interface. Methods -======= +------- A *method* is a function within the namespace scope of a class. It usually operates on the object instance by passing it as a @@ -275,8 +299,8 @@ Alternatively, object_class_by_name() can be used to obtain the class and its non-overridden methods for a specific type. This would correspond to ``MyClass::method(...)`` in C++. -The first example of such a QOM method was #CPUClass.reset, -another example is #DeviceClass.realize. +One example of such methods is ``DeviceClass.reset``. More examples +can be found at :ref:`device-life-cycle`. Standard type declaration and definition macros =============================================== @@ -382,10 +406,32 @@ OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead: OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE) +.. _device-life-cycle: + +Device Life-cycle +================= + +As class initialisation cannot fail devices have an two additional +methods to handle the creation of dynamic devices. The ``realize`` +function is called with ``Error **`` pointer which should be set if +the device cannot complete its setup. Otherwise on successful +completion of the ``realize`` method the device object is added to the +QOM tree and made visible to the guest. + +The reverse function is ``unrealize`` and should be were clean-up +code lives to tidy up after the system is done with the device. + +All devices can be instantiated by C code, however only some can +created dynamically via the command line or monitor. +Likewise only some can be unplugged after creation and need an +explicit ``unrealize`` implementation. This is determined by the +``user_creatable`` variable in the root ``DeviceClass`` structure. +Devices can only be unplugged if their ``parent_bus`` has a registered +``HotplugHandler``. API Reference -------------- +============= See the :ref:`QOM API` and :ref:`QDEV API` documents for the complete API description. -- cgit v1.1 From d2fe226467995c6843bba98da2e51dd5e85cef58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:13 +0100 Subject: gdbstub: lightly refactor connection to avoid snprintf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This may be a bit too much to avoid an snprintf and the slightly dodgy assign to a const variable. But hopefully not. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-29-alex.bennee@linaro.org> --- gdbstub/softmmu.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c index 99d994e..f509b72 100644 --- a/gdbstub/softmmu.c +++ b/gdbstub/softmmu.c @@ -332,11 +332,9 @@ static void create_processes(GDBState *s) int gdbserver_start(const char *device) { - trace_gdbstub_op_start(device); - - char gdbstub_device_name[128]; Chardev *chr = NULL; Chardev *mon_chr; + g_autoptr(GString) cs = g_string_new(device); if (!first_cpu) { error_report("gdbstub: meaningless to attach gdb to a " @@ -350,15 +348,16 @@ int gdbserver_start(const char *device) return -1; } - if (!device) { + if (cs->len == 0) { return -1; } - if (strcmp(device, "none") != 0) { - if (strstart(device, "tcp:", NULL)) { + + trace_gdbstub_op_start(cs->str); + + if (g_strcmp0(cs->str, "none") != 0) { + if (g_str_has_prefix(cs->str, "tcp:")) { /* enforce required TCP attributes */ - snprintf(gdbstub_device_name, sizeof(gdbstub_device_name), - "%s,wait=off,nodelay=on,server=on", device); - device = gdbstub_device_name; + g_string_append_printf(cs, ",wait=off,nodelay=on,server=on"); } #ifndef _WIN32 else if (strcmp(device, "stdio") == 0) { @@ -373,7 +372,7 @@ int gdbserver_start(const char *device) * FIXME: it's a bit weird to allow using a mux chardev here * and implicitly setup a monitor. We may want to break this. */ - chr = qemu_chr_new_noreplay("gdb", device, true, NULL); + chr = qemu_chr_new_noreplay("gdb", cs->str, true, NULL); if (!chr) { return -1; } -- cgit v1.1 From 3b72d68162f63ae4abc78edaa40775b045128ff6 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Fri, 30 Jun 2023 19:04:14 +0100 Subject: gdbstub: Permit reverse step/break to provide stop response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The final part of the reverse step and break handling is to bring the machine back to a debug stop state. gdb expects a response. A gdb 'rsi' command hangs forever because the gdbstub filters out the response (also observable with reverse_debugging.py avocado tests). Fix by setting allow_stop_reply for the gdb backward packets. Fixes: 758370052fb ("gdbstub: only send stop-reply packets when allowed to") Cc: qemu-stable@nongnu.org Cc: Matheus Tavares Bernardino Cc: Alex Bennée Cc: Taylor Simpson Signed-off-by: Nicholas Piggin Acked-by: Matheus Tavares Bernardino Message-Id: <20230623035304.279833-1-npiggin@gmail.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-30-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index be18568..9496d7b 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -1814,6 +1814,7 @@ static int gdb_handle_packet(const char *line_buf) .handler = handle_backward, .cmd = "b", .cmd_startswith = 1, + .allow_stop_reply = true, .schema = "o0" }; cmd_parser = &backward_cmd_desc; -- cgit v1.1 From 2261b73c2802ed976903cdd4e22e3e4efb299a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Fri, 30 Jun 2023 19:04:15 +0100 Subject: gdbstub: clean-up vcont handling to avoid goto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can handle all the error exit cases by using g_autofree() for the one thing that needs cleaning up on the exit. Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-31-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 9496d7b..49143c7 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -573,7 +573,6 @@ static int gdb_handle_vcont(const char *p) { int res, signal = 0; char cur_action; - char *newstates; unsigned long tmp; uint32_t pid, tid; GDBProcess *process; @@ -581,7 +580,7 @@ static int gdb_handle_vcont(const char *p) GDBThreadIdKind kind; unsigned int max_cpus = gdb_get_max_cpus(); /* uninitialised CPUs stay 0 */ - newstates = g_new0(char, max_cpus); + g_autofree char *newstates = g_new0(char, max_cpus); /* mark valid CPUs with 1 */ CPU_FOREACH(cpu) { @@ -597,8 +596,7 @@ static int gdb_handle_vcont(const char *p) res = 0; while (*p) { if (*p++ != ';') { - res = -ENOTSUP; - goto out; + return -ENOTSUP; } cur_action = *p++; @@ -606,13 +604,12 @@ static int gdb_handle_vcont(const char *p) cur_action = qemu_tolower(cur_action); res = qemu_strtoul(p, &p, 16, &tmp); if (res) { - goto out; + return res; } signal = gdb_signal_to_target(tmp); } else if (cur_action != 'c' && cur_action != 's') { /* unknown/invalid/unsupported command */ - res = -ENOTSUP; - goto out; + return -ENOTSUP; } if (*p == '\0' || *p == ';') { @@ -625,14 +622,12 @@ static int gdb_handle_vcont(const char *p) } else if (*p++ == ':') { kind = read_thread_id(p, &p, &pid, &tid); } else { - res = -ENOTSUP; - goto out; + return -ENOTSUP; } switch (kind) { case GDB_READ_THREAD_ERR: - res = -EINVAL; - goto out; + return -EINVAL; case GDB_ALL_PROCESSES: cpu = gdb_first_attached_cpu(); @@ -649,8 +644,7 @@ static int gdb_handle_vcont(const char *p) process = gdb_get_process(pid); if (!process->attached) { - res = -EINVAL; - goto out; + return -EINVAL; } cpu = get_first_cpu_in_process(process); @@ -668,8 +662,7 @@ static int gdb_handle_vcont(const char *p) /* invalid CPU/thread specified */ if (!cpu) { - res = -EINVAL; - goto out; + return -EINVAL; } /* only use if no previous match occourred */ @@ -679,12 +672,9 @@ static int gdb_handle_vcont(const char *p) break; } } + gdbserver_state.signal = signal; gdb_continue_partial(newstates); - -out: - g_free(newstates); - return res; } -- cgit v1.1 From a4dab0a0d3a40079b439dcab94d347d33184c166 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:16 +0100 Subject: linux-user: Expose do_guest_openat() and do_guest_readlink() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These functions will be required by the GDB stub in order to provide the guest view of /proc to GDB. Reviewed-by: Alex Bennée Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230621203627.1808446-2-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-32-alex.bennee@linaro.org> --- linux-user/qemu.h | 3 +++ linux-user/syscall.c | 54 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 92f9f5a..a5830ec 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -165,6 +165,9 @@ typedef struct TaskState { } TaskState; abi_long do_brk(abi_ulong new_brk); +int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, + int flags, mode_t mode); +ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz); /* user access */ diff --git a/linux-user/syscall.c b/linux-user/syscall.c index f2cb101..fa83737 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8448,7 +8448,8 @@ static int open_hardware(CPUArchState *cpu_env, int fd) } #endif -static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, int flags, mode_t mode) +int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, + int flags, mode_t mode) { struct fake_open { const char *filename; @@ -8520,6 +8521,36 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, int return safe_openat(dirfd, path(pathname), flags, mode); } +ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz) +{ + ssize_t ret; + + if (!pathname || !buf) { + errno = EFAULT; + return -1; + } + + if (!bufsiz) { + /* Short circuit this for the magic exe check. */ + errno = EINVAL; + return -1; + } + + if (is_proc_myself((const char *)pathname, "exe")) { + /* + * Don't worry about sign mismatch as earlier mapping + * logic would have thrown a bad address error. + */ + ret = MIN(strlen(exec_path), bufsiz); + /* We cannot NUL terminate the string. */ + memcpy(buf, exec_path, ret); + } else { + ret = readlink(path(pathname), buf, bufsiz); + } + + return ret; +} + static int do_execveat(CPUArchState *cpu_env, int dirfd, abi_long pathname, abi_long guest_argp, abi_long guest_envp, int flags) @@ -8994,7 +9025,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, case TARGET_NR_open: if (!(p = lock_user_string(arg1))) return -TARGET_EFAULT; - ret = get_errno(do_openat(cpu_env, AT_FDCWD, p, + ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p, target_to_host_bitmask(arg2, fcntl_flags_tbl), arg3)); fd_trans_unregister(ret); @@ -9004,7 +9035,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, case TARGET_NR_openat: if (!(p = lock_user_string(arg2))) return -TARGET_EFAULT; - ret = get_errno(do_openat(cpu_env, arg1, p, + ret = get_errno(do_guest_openat(cpu_env, arg1, p, target_to_host_bitmask(arg3, fcntl_flags_tbl), arg4)); fd_trans_unregister(ret); @@ -10229,22 +10260,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, void *p2; p = lock_user_string(arg1); p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0); - if (!p || !p2) { - ret = -TARGET_EFAULT; - } else if (!arg3) { - /* Short circuit this for the magic exe check. */ - ret = -TARGET_EINVAL; - } else if (is_proc_myself((const char *)p, "exe")) { - /* - * Don't worry about sign mismatch as earlier mapping - * logic would have thrown a bad address error. - */ - ret = MIN(strlen(exec_path), arg3); - /* We cannot NUL terminate the string. */ - memcpy(p2, exec_path, ret); - } else { - ret = get_errno(readlink(path(p), p2, arg3)); - } + ret = get_errno(do_guest_readlink(p, p2, arg3)); unlock_user(p2, arg2, ret); unlock_user(p, arg1, 0); } -- cgit v1.1 From 35be898e2f019bc63bffd02ab7fbd19287fd3cf4 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:17 +0100 Subject: linux-user: Add "safe" parameter to do_guest_openat() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gdbstub cannot meaningfully handle QEMU_ERESTARTSYS, and it doesn't need to. Add a parameter to do_guest_openat() that makes it use openat() instead of safe_openat(), so that it becomes usable from gdbstub. Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230621203627.1808446-3-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-33-alex.bennee@linaro.org> --- linux-user/qemu.h | 2 +- linux-user/syscall.c | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index a5830ec..9b8e086 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -166,7 +166,7 @@ typedef struct TaskState { abi_long do_brk(abi_ulong new_brk); int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, - int flags, mode_t mode); + int flags, mode_t mode, bool safe); ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz); /* user access */ diff --git a/linux-user/syscall.c b/linux-user/syscall.c index fa83737..ecd9f5e 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8449,7 +8449,7 @@ static int open_hardware(CPUArchState *cpu_env, int fd) #endif int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, - int flags, mode_t mode) + int flags, mode_t mode, bool safe) { struct fake_open { const char *filename; @@ -8476,7 +8476,11 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, }; if (is_proc_myself(pathname, "exe")) { - return safe_openat(dirfd, exec_path, flags, mode); + if (safe) { + return safe_openat(dirfd, exec_path, flags, mode); + } else { + return openat(dirfd, exec_path, flags, mode); + } } for (fake_open = fakes; fake_open->filename; fake_open++) { @@ -8518,7 +8522,11 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, return fd; } - return safe_openat(dirfd, path(pathname), flags, mode); + if (safe) { + return safe_openat(dirfd, path(pathname), flags, mode); + } else { + return openat(dirfd, path(pathname), flags, mode); + } } ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz) @@ -9027,7 +9035,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, return -TARGET_EFAULT; ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p, target_to_host_bitmask(arg2, fcntl_flags_tbl), - arg3)); + arg3, true)); fd_trans_unregister(ret); unlock_user(p, arg1, 0); return ret; @@ -9037,7 +9045,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, return -TARGET_EFAULT; ret = get_errno(do_guest_openat(cpu_env, arg1, p, target_to_host_bitmask(arg3, fcntl_flags_tbl), - arg4)); + arg4, true)); fd_trans_unregister(ret); unlock_user(p, arg2, 0); return ret; -- cgit v1.1 From 77ae5761f3c9e6b68f8545cff06704f823c3d387 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:18 +0100 Subject: linux-user: Emulate /proc/self/smaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /proc/self/smaps is an extension of /proc/self/maps: it provides the same lines, plus additional information about each range. GDB uses /proc/self/smaps when available, which means that generate-core-file tries it first before falling back to /proc/self/maps. This, in turn, causes it to dump the host mappings, since /proc/self/smaps is not emulated and is just passed through. Fix by emulating /proc/self/smaps. Provide true values only for Size, KernelPageSize, MMUPageSize and VmFlags. Leave all other values at 0, which is a valid conservative estimate. Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230621203627.1808446-4-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-34-alex.bennee@linaro.org> --- linux-user/syscall.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ecd9f5e..08162cc 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8042,7 +8042,36 @@ static int open_self_cmdline(CPUArchState *cpu_env, int fd) return 0; } -static int open_self_maps(CPUArchState *cpu_env, int fd) +static void show_smaps(int fd, unsigned long size) +{ + unsigned long page_size_kb = TARGET_PAGE_SIZE >> 10; + unsigned long size_kb = size >> 10; + + dprintf(fd, "Size: %lu kB\n" + "KernelPageSize: %lu kB\n" + "MMUPageSize: %lu kB\n" + "Rss: 0 kB\n" + "Pss: 0 kB\n" + "Pss_Dirty: 0 kB\n" + "Shared_Clean: 0 kB\n" + "Shared_Dirty: 0 kB\n" + "Private_Clean: 0 kB\n" + "Private_Dirty: 0 kB\n" + "Referenced: 0 kB\n" + "Anonymous: 0 kB\n" + "LazyFree: 0 kB\n" + "AnonHugePages: 0 kB\n" + "ShmemPmdMapped: 0 kB\n" + "FilePmdMapped: 0 kB\n" + "Shared_Hugetlb: 0 kB\n" + "Private_Hugetlb: 0 kB\n" + "Swap: 0 kB\n" + "SwapPss: 0 kB\n" + "Locked: 0 kB\n" + "THPeligible: 0\n", size_kb, page_size_kb, page_size_kb); +} + +static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps) { CPUState *cpu = env_cpu(cpu_env); TaskState *ts = cpu->opaque; @@ -8089,6 +8118,18 @@ static int open_self_maps(CPUArchState *cpu_env, int fd) } else { dprintf(fd, "\n"); } + if (smaps) { + show_smaps(fd, max - min); + dprintf(fd, "VmFlags:%s%s%s%s%s%s%s%s\n", + (flags & PAGE_READ) ? " rd" : "", + (flags & PAGE_WRITE_ORG) ? " wr" : "", + (flags & PAGE_EXEC) ? " ex" : "", + e->is_priv ? "" : " sh", + (flags & PAGE_READ) ? " mr" : "", + (flags & PAGE_WRITE_ORG) ? " mw" : "", + (flags & PAGE_EXEC) ? " me" : "", + e->is_priv ? "" : " ms"); + } } } @@ -8103,11 +8144,25 @@ static int open_self_maps(CPUArchState *cpu_env, int fd) " --xp 00000000 00:00 0", TARGET_VSYSCALL_PAGE, TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE); dprintf(fd, "%*s%s\n", 73 - count, "", "[vsyscall]"); + if (smaps) { + show_smaps(fd, TARGET_PAGE_SIZE); + dprintf(fd, "VmFlags: ex\n"); + } #endif return 0; } +static int open_self_maps(CPUArchState *cpu_env, int fd) +{ + return open_self_maps_1(cpu_env, fd, false); +} + +static int open_self_smaps(CPUArchState *cpu_env, int fd) +{ + return open_self_maps_1(cpu_env, fd, true); +} + static int open_self_stat(CPUArchState *cpu_env, int fd) { CPUState *cpu = env_cpu(cpu_env); @@ -8459,6 +8514,7 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, const struct fake_open *fake_open; static const struct fake_open fakes[] = { { "maps", open_self_maps, is_proc_myself }, + { "smaps", open_self_smaps, is_proc_myself }, { "stat", open_self_stat, is_proc_myself }, { "auxv", open_self_auxv, is_proc_myself }, { "cmdline", open_self_cmdline, is_proc_myself }, -- cgit v1.1 From a3fcc111fa2bd8333f53883efb41d539d95b3cd0 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:19 +0100 Subject: gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These functions will be needed by user-target.c in order to retrieve the name of the executable. Reviewed-by: Alex Bennée Signed-off-by: Ilya Leoshkevich Message-Id: <20230621203627.1808446-5-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-35-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 16 ++++++++-------- gdbstub/internals.h | 2 ++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 49143c7..ce3e4a2 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -211,7 +211,7 @@ static uint32_t gdb_get_cpu_pid(CPUState *cpu) return cpu->cluster_index + 1; } -static GDBProcess *gdb_get_process(uint32_t pid) +GDBProcess *gdb_get_process(uint32_t pid) { int i; @@ -247,7 +247,7 @@ static CPUState *find_cpu(uint32_t thread_id) return NULL; } -static CPUState *get_first_cpu_in_process(GDBProcess *process) +CPUState *gdb_get_first_cpu_in_process(GDBProcess *process) { CPUState *cpu; @@ -325,7 +325,7 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid) return NULL; } - return get_first_cpu_in_process(process); + return gdb_get_first_cpu_in_process(process); } else { /* a specific thread */ cpu = find_cpu(tid); @@ -354,7 +354,7 @@ static const char *get_feature_xml(const char *p, const char **newp, size_t len; int i; const char *name; - CPUState *cpu = get_first_cpu_in_process(process); + CPUState *cpu = gdb_get_first_cpu_in_process(process); CPUClass *cc = CPU_GET_CLASS(cpu); len = 0; @@ -490,7 +490,7 @@ void gdb_register_coprocessor(CPUState *cpu, static void gdb_process_breakpoint_remove_all(GDBProcess *p) { - CPUState *cpu = get_first_cpu_in_process(p); + CPUState *cpu = gdb_get_first_cpu_in_process(p); while (cpu) { gdb_breakpoint_remove_all(cpu); @@ -647,7 +647,7 @@ static int gdb_handle_vcont(const char *p) return -EINVAL; } - cpu = get_first_cpu_in_process(process); + cpu = gdb_get_first_cpu_in_process(process); while (cpu) { if (newstates[cpu->cpu_index] == 1) { newstates[cpu->cpu_index] = cur_action; @@ -1270,7 +1270,7 @@ static void handle_v_attach(GArray *params, void *user_ctx) goto cleanup; } - cpu = get_first_cpu_in_process(process); + cpu = gdb_get_first_cpu_in_process(process); if (!cpu) { goto cleanup; } @@ -1393,7 +1393,7 @@ static void handle_query_curr_tid(GArray *params, void *user_ctx) * first thread). */ process = gdb_get_cpu_process(gdbserver_state.g_cpu); - cpu = get_first_cpu_in_process(process); + cpu = gdb_get_first_cpu_in_process(process); g_string_assign(gdbserver_state.str_buf, "QC"); gdb_append_thread_id(cpu, gdbserver_state.str_buf); gdb_put_strbuf(); diff --git a/gdbstub/internals.h b/gdbstub/internals.h index 33d21d6..25e4d5e 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -129,6 +129,8 @@ void gdb_read_byte(uint8_t ch); */ bool gdb_got_immediate_ack(void); /* utility helpers */ +GDBProcess *gdb_get_process(uint32_t pid); +CPUState *gdb_get_first_cpu_in_process(GDBProcess *process); CPUState *gdb_first_attached_cpu(void); void gdb_append_thread_id(CPUState *cpu, GString *buf); int gdb_get_cpu_index(CPUState *cpu); -- cgit v1.1 From dc14a7a6e95571122ec2428abb355fe2c43e05c6 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:20 +0100 Subject: gdbstub: Report the actual qemu-user pid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently qemu-user reports pid 1 to GDB. Resolve the TODO and report the actual PID. Using getpid() relies on the assumption that there is only one GDBProcess. Add an assertion to make sure that future changes don't break it. Reviewed-by: Alex Bennée Signed-off-by: Ilya Leoshkevich Message-Id: <20230621203627.1808446-6-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-36-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index ce3e4a2..697dd4b 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -202,13 +202,16 @@ void gdb_memtox(GString *buf, const char *mem, int len) static uint32_t gdb_get_cpu_pid(CPUState *cpu) { - /* TODO: In user mode, we should use the task state PID */ +#ifdef CONFIG_USER_ONLY + return getpid(); +#else if (cpu->cluster_index == UNASSIGNED_CLUSTER_INDEX) { /* Return the default process' PID */ int index = gdbserver_state.process_num - 1; return gdbserver_state.processes[index].pid; } return cpu->cluster_index + 1; +#endif } GDBProcess *gdb_get_process(uint32_t pid) @@ -2137,19 +2140,25 @@ void gdb_read_byte(uint8_t ch) void gdb_create_default_process(GDBState *s) { GDBProcess *process; - int max_pid = 0; + int pid; +#ifdef CONFIG_USER_ONLY + assert(gdbserver_state.process_num == 0); + pid = getpid(); +#else if (gdbserver_state.process_num) { - max_pid = s->processes[s->process_num - 1].pid; + pid = s->processes[s->process_num - 1].pid; + } else { + pid = 0; } + /* We need an available PID slot for this process */ + assert(pid < UINT32_MAX); + pid++; +#endif s->processes = g_renew(GDBProcess, s->processes, ++s->process_num); process = &s->processes[s->process_num - 1]; - - /* We need an available PID slot for this process */ - assert(max_pid < UINT32_MAX); - - process->pid = max_pid + 1; + process->pid = pid; process->attached = false; process->target_xml[0] = '\0'; } -- cgit v1.1 From e282010b2e1e34e1579c933cadf278833d527812 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:21 +0100 Subject: gdbstub: Add support for info proc mappings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the GDB's generate-core-file command doesn't work well with qemu-user: the resulting dumps are huge [1] and at the same time incomplete (argv and envp are missing). The reason is that GDB has no access to proc mappings and therefore has to fall back to using heuristics for discovering them. This is, in turn, because qemu-user does not implement the Host I/O feature of the GDB Remote Serial Protocol. Implement vFile:{open,close,pread,readlink} and also qXfer:exec-file:read+. With that, generate-core-file begins to work on aarch64 and s390x. [1] https://sourceware.org/pipermail/gdb-patches/2023-May/199432.html Co-developed-by: Dominik 'Disconnect3d' Czarnota Signed-off-by: Ilya Leoshkevich Message-Id: <20230621203627.1808446-7-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-37-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 45 ++++++++++++++++- gdbstub/internals.h | 5 ++ gdbstub/user-target.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 2 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 697dd4b..6911b73 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -1327,6 +1327,36 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = { .cmd = "Kill;", .cmd_startswith = 1 }, +#ifdef CONFIG_USER_ONLY + /* + * Host I/O Packets. See [1] for details. + * [1] https://sourceware.org/gdb/onlinedocs/gdb/Host-I_002fO-Packets.html + */ + { + .handler = gdb_handle_v_file_open, + .cmd = "File:open:", + .cmd_startswith = 1, + .schema = "s,L,L0" + }, + { + .handler = gdb_handle_v_file_close, + .cmd = "File:close:", + .cmd_startswith = 1, + .schema = "l0" + }, + { + .handler = gdb_handle_v_file_pread, + .cmd = "File:pread:", + .cmd_startswith = 1, + .schema = "l,L,L0" + }, + { + .handler = gdb_handle_v_file_readlink, + .cmd = "File:readlink:", + .cmd_startswith = 1, + .schema = "s0" + }, +#endif }; static void handle_v_commands(GArray *params, void *user_ctx) @@ -1472,11 +1502,14 @@ static void handle_query_supported(GArray *params, void *user_ctx) ";ReverseStep+;ReverseContinue+"); } -#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX) +#if defined(CONFIG_USER_ONLY) +#if defined(CONFIG_LINUX) if (gdbserver_state.c_cpu->opaque) { g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+"); } #endif + g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+"); +#endif if (params->len && strstr(get_param(params, 0)->data, "multiprocess+")) { @@ -1615,7 +1648,8 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = { .cmd_startswith = 1, .schema = "s:l,l0" }, -#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX) +#if defined(CONFIG_USER_ONLY) +#if defined(CONFIG_LINUX) { .handler = gdb_handle_query_xfer_auxv, .cmd = "Xfer:auxv:read::", @@ -1624,6 +1658,13 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = { }, #endif { + .handler = gdb_handle_query_xfer_exec_file, + .cmd = "Xfer:exec-file:read:", + .cmd_startswith = 1, + .schema = "l:l,l0" + }, +#endif + { .handler = gdb_handle_query_attached, .cmd = "Attached:", .cmd_startswith = 1 diff --git a/gdbstub/internals.h b/gdbstub/internals.h index 25e4d5e..f2b46cc 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -189,6 +189,11 @@ typedef union GdbCmdVariant { void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* softmmu */ void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */ void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */ +void gdb_handle_v_file_open(GArray *params, void *user_ctx); /* user */ +void gdb_handle_v_file_close(GArray *params, void *user_ctx); /* user */ +void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */ +void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */ +void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user */ void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */ diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c index fa0e59e..6e21c31 100644 --- a/gdbstub/user-target.c +++ b/gdbstub/user-target.c @@ -11,6 +11,10 @@ #include "exec/gdbstub.h" #include "qemu.h" #include "internals.h" +#ifdef CONFIG_LINUX +#include "linux-user/loader.h" +#include "linux-user/qemu.h" +#endif /* * Map target signal numbers to GDB protocol signal numbers and vice @@ -281,3 +285,136 @@ void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx) gdbserver_state.str_buf->len, true); } #endif + +static const char *get_filename_param(GArray *params, int i) +{ + const char *hex_filename = get_param(params, i)->data; + gdb_hextomem(gdbserver_state.mem_buf, hex_filename, + strlen(hex_filename) / 2); + g_byte_array_append(gdbserver_state.mem_buf, (const guint8 *)"", 1); + return (const char *)gdbserver_state.mem_buf->data; +} + +static void hostio_reply_with_data(const void *buf, size_t n) +{ + g_string_printf(gdbserver_state.str_buf, "F%zx;", n); + gdb_memtox(gdbserver_state.str_buf, buf, n); + gdb_put_packet_binary(gdbserver_state.str_buf->str, + gdbserver_state.str_buf->len, true); +} + +void gdb_handle_v_file_open(GArray *params, void *user_ctx) +{ + const char *filename = get_filename_param(params, 0); + uint64_t flags = get_param(params, 1)->val_ull; + uint64_t mode = get_param(params, 2)->val_ull; + +#ifdef CONFIG_LINUX + int fd = do_guest_openat(gdbserver_state.g_cpu->env_ptr, 0, filename, + flags, mode, false); +#else + int fd = open(filename, flags, mode); +#endif + if (fd < 0) { + g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno); + } else { + g_string_printf(gdbserver_state.str_buf, "F%d", fd); + } + gdb_put_strbuf(); +} + +void gdb_handle_v_file_close(GArray *params, void *user_ctx) +{ + int fd = get_param(params, 0)->val_ul; + + if (close(fd) == -1) { + g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno); + gdb_put_strbuf(); + return; + } + + gdb_put_packet("F00"); +} + +void gdb_handle_v_file_pread(GArray *params, void *user_ctx) +{ + int fd = get_param(params, 0)->val_ul; + size_t count = get_param(params, 1)->val_ull; + off_t offset = get_param(params, 2)->val_ull; + + size_t bufsiz = MIN(count, BUFSIZ); + g_autofree char *buf = g_try_malloc(bufsiz); + if (buf == NULL) { + gdb_put_packet("E12"); + return; + } + + ssize_t n = pread(fd, buf, bufsiz, offset); + if (n < 0) { + g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno); + gdb_put_strbuf(); + return; + } + hostio_reply_with_data(buf, n); +} + +void gdb_handle_v_file_readlink(GArray *params, void *user_ctx) +{ + const char *filename = get_filename_param(params, 0); + + g_autofree char *buf = g_try_malloc(BUFSIZ); + if (buf == NULL) { + gdb_put_packet("E12"); + return; + } + +#ifdef CONFIG_LINUX + ssize_t n = do_guest_readlink(filename, buf, BUFSIZ); +#else + ssize_t n = readlink(filename, buf, BUFSIZ); +#endif + if (n < 0) { + g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno); + gdb_put_strbuf(); + return; + } + hostio_reply_with_data(buf, n); +} + +void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx) +{ + uint32_t pid = get_param(params, 0)->val_ul; + uint32_t offset = get_param(params, 1)->val_ul; + uint32_t length = get_param(params, 2)->val_ul; + + GDBProcess *process = gdb_get_process(pid); + if (!process) { + gdb_put_packet("E00"); + return; + } + + CPUState *cpu = gdb_get_first_cpu_in_process(process); + if (!cpu) { + gdb_put_packet("E00"); + return; + } + + TaskState *ts = cpu->opaque; + if (!ts || !ts->bprm || !ts->bprm->filename) { + gdb_put_packet("E00"); + return; + } + + size_t total_length = strlen(ts->bprm->filename); + if (offset > total_length) { + gdb_put_packet("E00"); + return; + } + if (offset + length > total_length) { + length = total_length - offset; + } + + g_string_printf(gdbserver_state.str_buf, "l%.*s", length, + ts->bprm->filename + offset); + gdb_put_strbuf(); +} -- cgit v1.1 From abf7ba310b7b0c13b4fe936621bbb8e416754c0c Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:22 +0100 Subject: docs: Document security implications of debugging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the GDB stub explicitly implements reading host files (note that it was already possible by changing the emulated code to open and read those files), concerns may arise that it undermines security. Document the status quo, which is that the users are already responsible for securing the GDB connection themselves. Reviewed-by: Alex Bennée Signed-off-by: Ilya Leoshkevich Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230621203627.1808446-8-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-38-alex.bennee@linaro.org> --- docs/system/gdb.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst index 7d3718d..9906991 100644 --- a/docs/system/gdb.rst +++ b/docs/system/gdb.rst @@ -214,3 +214,18 @@ The memory mode can be checked by sending the following command: ``maintenance packet Qqemu.PhyMemMode:0`` This will change it back to normal memory mode. + +Security considerations +======================= + +Connecting to the GDB socket allows running arbitrary code inside the guest; +in case of the TCG emulation, which is not considered a security boundary, this +also means running arbitrary code on the host. Additionally, when debugging +qemu-user, it allows directly downloading any file readable by QEMU from the +host. + +The GDB socket is not protected by authentication, authorization or encryption. +It is therefore a responsibility of the user to make sure that only authorized +clients can connect to it, e.g., by using a unix socket with proper +permissions, or by opening a TCP socket only on interfaces that are not +reachable by potential attackers. -- cgit v1.1 From a6341482695e1d15f11915f12dba98724efb0697 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 30 Jun 2023 19:04:23 +0100 Subject: tests/tcg: Add a test for info proc mappings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Message-Id: <20230621203627.1808446-9-iii@linux.ibm.com> Signed-off-by: Alex Bennée Message-Id: <20230630180423.558337-39-alex.bennee@linaro.org> --- tests/tcg/multiarch/Makefile.target | 9 +++- tests/tcg/multiarch/gdbstub/test-proc-mappings.py | 65 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-mappings.py diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target index 373db69..43bddea 100644 --- a/tests/tcg/multiarch/Makefile.target +++ b/tests/tcg/multiarch/Makefile.target @@ -81,6 +81,13 @@ run-gdbstub-qxfer-auxv-read: sha1 --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \ basic gdbstub qXfer:auxv:read support) +run-gdbstub-proc-mappings: sha1 + $(call run-test, $@, $(GDB_SCRIPT) \ + --gdb $(HAVE_GDB_BIN) \ + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ + --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc-mappings.py, \ + proc mappings support) + run-gdbstub-thread-breakpoint: testthread $(call run-test, $@, $(GDB_SCRIPT) \ --gdb $(HAVE_GDB_BIN) \ @@ -97,7 +104,7 @@ run-gdbstub-%: $(call skip-test, "gdbstub test $*", "need working gdb") endif EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \ - run-gdbstub-thread-breakpoint + run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint # ARM Compatible Semi Hosting Tests # diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py new file mode 100644 index 0000000..7b596ac --- /dev/null +++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py @@ -0,0 +1,65 @@ +"""Test that gdbstub has access to proc mappings. + +This runs as a sourced script (via -x, via run-test.py).""" +from __future__ import print_function +import gdb +import sys + + +n_failures = 0 + + +def report(cond, msg): + """Report success/fail of a test""" + if cond: + print("PASS: {}".format(msg)) + else: + print("FAIL: {}".format(msg)) + global n_failures + n_failures += 1 + + +def run_test(): + """Run through the tests one by one""" + try: + mappings = gdb.execute("info proc mappings", False, True) + except gdb.error as exc: + exc_str = str(exc) + if "Not supported on this target." in exc_str: + # Detect failures due to an outstanding issue with how GDB handles + # the x86_64 QEMU's target.xml, which does not contain the + # definition of orig_rax. Skip the test in this case. + print("SKIP: {}".format(exc_str)) + return + raise + report(isinstance(mappings, str), "Fetched the mappings from the inferior") + report("/sha1" in mappings, "Found the test binary name in the mappings") + + +def main(): + """Prepare the environment and run through the tests""" + try: + inferior = gdb.selected_inferior() + print("ATTACHED: {}".format(inferior.architecture().name())) + except (gdb.error, AttributeError): + print("SKIPPING (not connected)") + exit(0) + + if gdb.parse_and_eval('$pc') == 0: + print("SKIP: PC not set") + exit(0) + + try: + # These are not very useful in scripts + gdb.execute("set pagination off") + gdb.execute("set confirm off") + + # Run the actual tests + run_test() + except gdb.error: + report(False, "GDB Exception: {}".format(sys.exc_info()[0])) + print("All tests complete: %d failures" % n_failures) + exit(n_failures) + + +main() -- cgit v1.1