From 4ebb040f1fd6985df191b63321493d1b6fb504f5 Mon Sep 17 00:00:00 2001 From: Daniel P. Berrangé Date: Wed, 5 Jan 2022 13:49:42 +0000 Subject: tests: integrate lcitool for generating build env manifests This introduces https://gitlab.com/libvirt/libvirt-ci as a git submodule at tests/lcitool/libvirt-ci The 'lcitool' program within this submodule will be used to automatically generate build environment manifests from a definition of requirements in tests/lcitool/projects/qemu.yml It will ultimately be capable of generating - Dockerfiles - Package lists for installation in VMs - Variables for configuring Cirrus CI environments When a new build pre-requisite is needed for QEMU, if this package is not currently known to libvirt-ci, it must first be added to the 'mappings.yml' file in the above git repo. Then the submodule can be updated and the build pre-requisite added to the tests/lcitool/projects/qemu.yml file. Now all the build env manifests can be re-generated using 'make lcitool-refresh' This ensures that when a new build pre-requisite is introduced, it is added to all the different OS containers, VMs and Cirrus CI environments consistently. It also facilitates the addition of containers targetting new distros or updating existing containers to new versions of the same distro, where packages might have been renamed. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé Signed-off-by: Alex Bennée Message-Id: <20211215141949.3512719-8-berrange@redhat.com> Message-Id: <20220105135009.1584676-8-alex.bennee@linaro.org> --- docs/devel/testing.rst | 104 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 3 deletions(-) (limited to 'docs') diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 755343c7dd..d744b5909c 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -382,14 +382,112 @@ Along with many other images, the ``centos8`` image is defined in a Dockerfile in ``tests/docker/dockerfiles/``, called ``centos8.docker``. ``make docker-help`` command will list all the available images. -To add a new image, simply create a new ``.docker`` file under the -``tests/docker/dockerfiles/`` directory. - A ``.pre`` script can be added beside the ``.docker`` file, which will be executed before building the image under the build context directory. This is mainly used to do necessary host side setup. One such setup is ``binfmt_misc``, for example, to make qemu-user powered cross build containers work. +Most of the existing Dockerfiles were written by hand, simply by creating a +a new ``.docker`` file under the ``tests/docker/dockerfiles/`` directory. +This has led to an inconsistent set of packages being present across the +different containers. + +Thus going forward, QEMU is aiming to automatically generate the Dockerfiles +using the ``lcitool`` program provided by the ``libvirt-ci`` project: + + https://gitlab.com/libvirt/libvirt-ci + +In that project, there is a ``mappings.yml`` file defining the distro native +package names for a wide variety of third party projects. This is processed +in combination with a project defined list of build pre-requisites to determine +the list of native packages to install on each distribution. This can be used +to generate dockerfiles, VM package lists and Cirrus CI variables needed to +setup build environments across OS distributions with a consistent set of +packages present. + +When preparing a patch series that adds a new build pre-requisite to QEMU, +updates to various lcitool data files may be required. + + +Adding new build pre-requisites +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In the simple case where the pre-requisite is already known to ``libvirt-ci`` +the following steps are needed + + * Edit ``tests/lcitool/projects/qemu.yml`` and add the pre-requisite + + * Run ``make lcitool-refresh`` to re-generate all relevant build environment + manifests + +In some cases ``libvirt-ci`` will not know about the build pre-requisite and +thus some extra preparation steps will be required first + + * Fork the ``libvirt-ci`` project on gitlab + + * Edit the ``mappings.yml`` change to add an entry for the new build + prerequisite, listing its native package name on as many OS distros + as practical. + + * Commit the ``mappings.yml`` change and submit a merge request to + the ``libvirt-ci`` project, noting in the description that this + is a new build pre-requisite desired for use with QEMU + + * CI pipeline will run to validate that the changes to ``mappings.yml`` + are correct, by attempting to install the newly listed package on + all OS distributions supported by ``libvirt-ci``. + + * Once the merge request is accepted, go back to QEMU and update + the ``libvirt-ci`` submodule to point to a commit that contains + the ``mappings.yml`` update. + + +Adding new OS distros +^^^^^^^^^^^^^^^^^^^^^ + +In some cases ``libvirt-ci`` will not know about the OS distro that is +desired to be tested. Before adding a new OS distro, discuss the proposed +addition: + + * Send a mail to qemu-devel, copying people listed in the + MAINTAINERS file for ``Build and test automation``. + + There are limited CI compute resources available to QEMU, so the + cost/benefit tradeoff of adding new OS distros needs to be considered. + + * File an issue at https://gitlab.com/libvirt/libvirt-ci/-/issues + pointing to the qemu-devel mail thread in the archives. + + This alerts other people who might be interested in the work + to avoid duplication, as well as to get feedback from libvirt-ci + maintainers on any tips to ease the addition + +Assuming there is agreement to add a new OS distro then + + * Fork the ``libvirt-ci`` project on gitlab + + * Add metadata under ``guests/lcitool/lcitool/ansible/group_vars/`` + for the new OS distro. There might be code changes required if + the OS distro uses a package format not currently known. The + ``libvirt-ci`` maintainers can advise on this when the issue + is file. + + * Edit the ``mappings.yml`` change to update all the existing package + entries, providing details of the new OS distro + + * Commit the ``mappings.yml`` change and submit a merge request to + the ``libvirt-ci`` project, noting in the description that this + is a new build pre-requisite desired for use with QEMU + + * CI pipeline will run to validate that the changes to ``mappings.yml`` + are correct, by attempting to install the newly listed package on + all OS distributions supported by ``libvirt-ci``. + + * Once the merge request is accepted, go back to QEMU and update + the ``libvirt-ci`` submodule to point to a commit that contains + the ``mappings.yml`` update. + + Tests ~~~~~ -- cgit v1.2.3-55-g7522 From 33973e1e1f88b7588fe9629645e279ff2c6ca1c4 Mon Sep 17 00:00:00 2001 From: Alex Bennée Date: Wed, 5 Jan 2022 13:49:56 +0000 Subject: hw/arm: add control knob to disable kaslr_seed via DTB Generally a guest needs an external source of randomness to properly enable things like address space randomisation. However in a trusted boot environment where the firmware will cryptographically verify components having random data in the DTB will cause verification to fail. Add a control knob so we can prevent this being added to the system DTB. Signed-off-by: Alex Bennée Tested-by: Heinrich Schuchardt Acked-by: Ilias Apalodimas Acked-by: Jerome Forissier Reviewed-by: Andrew Jones Message-Id: <20220105135009.1584676-22-alex.bennee@linaro.org> --- docs/system/arm/virt.rst | 8 ++++++++ hw/arm/virt.c | 32 ++++++++++++++++++++++++++++++-- include/hw/arm/virt.h | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) (limited to 'docs') diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst index 850787495b..1544632b67 100644 --- a/docs/system/arm/virt.rst +++ b/docs/system/arm/virt.rst @@ -121,6 +121,14 @@ ras Set ``on``/``off`` to enable/disable reporting host memory errors to a guest using ACPI and guest external abort exceptions. The default is off. +dtb-kaslr-seed + Set ``on``/``off`` to pass a random seed via the guest dtb + kaslr-seed node (in both "/chosen" and /secure-chosen) to use + for features like address space randomisation. The default is + ``on``. You will want to disable it if your trusted boot chain will + verify the DTB it is passed. It would be the responsibility of the + firmware to come up with a seed and pass it on if it wants to. + Linux guest kernel configuration """""""""""""""""""""""""""""""" diff --git a/hw/arm/virt.c b/hw/arm/virt.c index b45b52c90e..84c2444fff 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -247,11 +247,15 @@ static void create_fdt(VirtMachineState *vms) /* /chosen must exist for load_dtb to fill in necessary properties later */ qemu_fdt_add_subnode(fdt, "/chosen"); - create_kaslr_seed(ms, "/chosen"); + if (vms->dtb_kaslr_seed) { + create_kaslr_seed(ms, "/chosen"); + } if (vms->secure) { qemu_fdt_add_subnode(fdt, "/secure-chosen"); - create_kaslr_seed(ms, "/secure-chosen"); + if (vms->dtb_kaslr_seed) { + create_kaslr_seed(ms, "/secure-chosen"); + } } /* Clock node, for the benefit of the UART. The kernel device tree @@ -2235,6 +2239,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp) vms->its = value; } +static bool virt_get_dtb_kaslr_seed(Object *obj, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(obj); + + return vms->dtb_kaslr_seed; +} + +static void virt_set_dtb_kaslr_seed(Object *obj, bool value, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(obj); + + vms->dtb_kaslr_seed = value; +} + static char *virt_get_oem_id(Object *obj, Error **errp) { VirtMachineState *vms = VIRT_MACHINE(obj); @@ -2764,6 +2782,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) "Set on/off to enable/disable " "ITS instantiation"); + object_class_property_add_bool(oc, "dtb-kaslr-seed", + virt_get_dtb_kaslr_seed, + virt_set_dtb_kaslr_seed); + object_class_property_set_description(oc, "dtb-kaslr-seed", + "Set off to disable passing of kaslr-seed " + "dtb node to guest"); + object_class_property_add_str(oc, "x-oem-id", virt_get_oem_id, virt_set_oem_id); @@ -2828,6 +2853,9 @@ static void virt_instance_init(Object *obj) /* MTE is disabled by default. */ vms->mte = false; + /* Supply a kaslr-seed by default */ + vms->dtb_kaslr_seed = true; + vms->irqmap = a15irqmap; virt_flash_create(vms); diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index dc6b66ffc8..be0534608f 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -148,6 +148,7 @@ struct VirtMachineState { bool virt; bool ras; bool mte; + bool dtb_kaslr_seed; OnOffAuto acpi; VirtGICType gic_version; VirtIOMMUType iommu; -- cgit v1.2.3-55-g7522 From a68e025bf5103280be6cfeab19452491b793e679 Mon Sep 17 00:00:00 2001 From: Alex Bennée Date: Wed, 5 Jan 2022 13:49:58 +0000 Subject: docs/devel: update C standard to C11 Since 8a9d3d5640 (configure: Use -std=gnu11) we have allowed C11 code so lets reflect that in the style guide. Signed-off-by: Alex Bennée Reviewed-by: Daniel P. Berrangé Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220105135009.1584676-24-alex.bennee@linaro.org> --- docs/devel/style.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'docs') diff --git a/docs/devel/style.rst b/docs/devel/style.rst index 9c5c0fffd9..4f770002a7 100644 --- a/docs/devel/style.rst +++ b/docs/devel/style.rst @@ -483,11 +483,11 @@ of arguments. C standard, implementation defined and undefined behaviors ========================================================== -C code in QEMU should be written to the C99 language specification. A copy -of the final version of the C99 standard with corrigenda TC1, TC2, and TC3 -included, formatted as a draft, can be downloaded from: +C code in QEMU should be written to the C11 language specification. A +copy of the final version of the C11 standard formatted as a draft, +can be downloaded from: - ``_ + ``_ The C language specification defines regions of undefined behavior and implementation defined behavior (to give compiler authors enough leeway to -- cgit v1.2.3-55-g7522 From 3918fe16b0f8c6657928a14f41b7ff50061e33e1 Mon Sep 17 00:00:00 2001 From: Alex Bennée Date: Wed, 5 Jan 2022 13:49:59 +0000 Subject: docs/devel: more documentation on the use of suffixes Using _qemu is a little confusing. Let's use _compat for these sorts of things. We should also mention _impl which is another common suffix in the code base. Signed-off-by: Alex Bennée Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220105135009.1584676-25-alex.bennee@linaro.org> --- docs/devel/style.rst | 6 ++++++ include/glib-compat.h | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'docs') diff --git a/docs/devel/style.rst b/docs/devel/style.rst index 4f770002a7..793a8d4280 100644 --- a/docs/devel/style.rst +++ b/docs/devel/style.rst @@ -151,6 +151,12 @@ If there are two versions of a function to be called with or without a lock held, the function that expects the lock to be already held usually uses the suffix ``_locked``. +If a function is a shim designed to deal with compatibility +workarounds we use the suffix ``_compat``. These are generally not +called directly and aliased to the plain function name via the +pre-processor. Another common suffix is ``_impl``; it is used for the +concrete implementation of a function that will not be called +directly, but rather through a macro or an inline function. Block structure =============== diff --git a/include/glib-compat.h b/include/glib-compat.h index 8d01a8c01f..3113a7d2af 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -46,9 +46,9 @@ * int g_foo(const char *wibble) * * We must define a static inline function with the same signature that does - * what we need, but with a "_qemu" suffix e.g. + * what we need, but with a "_compat" suffix e.g. * - * static inline void g_foo_qemu(const char *wibble) + * static inline void g_foo_compat(const char *wibble) * { * #if GLIB_CHECK_VERSION(X, Y, 0) * g_foo(wibble) @@ -61,7 +61,7 @@ * ensuring this wrapper function impl doesn't trigger the compiler warning * about using too new glib APIs. Finally we can do * - * #define g_foo(a) g_foo_qemu(a) + * #define g_foo(a) g_foo_compat(a) * * So now the code elsewhere in QEMU, which *does* have the * -Wdeprecated-declarations warning active, can call g_foo(...) as normal, -- cgit v1.2.3-55-g7522