[libvirt test] 159832: regressions - FAIL
flight 159832 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/159832/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a version targeted for testing: libvirt f81d504b71a86ed501f8caca2f08613863784239 baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 238 days Failing since151818 2020-07-11 04:18:52 Z 237 days 230 attempts Testing same since 159832 2021-03-05 04:21:09 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Andika Triwidada Andrea Bolognani Balázs Meskó Barrett Schonefeld Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Boris Fiuczynski Brian Turek Bruno Haible Christian Ehrhardt Christian Schoenebeck Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Dmytro Linkin Eiichi Tsukata Erik Skultety Fabian Affolter Fabian Freyer Fangge Jin Farhan Ali Fedora Weblate Translation gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Helmut Grohne Ian Wienand Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen Jean-Baptiste Holcroft Jianan Gao Jim Fehlig Jin Yan Jiri Denemark John Ferlan Jonathan Watt Jonathon Jongsma Julio Faracco Ján Tomko Kashyap Chamarthy Kevin Locke Kristina Hanicova Laine Stump Laszlo Ersek Liao Pingfang Lin Ma Lin Ma Lin Ma Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Masayoshi Mizuma Matt Coleman Matt Coleman Mauro Matteo Cascella Meina Li Michal Privoznik Michał Smyk Milo Casagrande Moshe Levi Muha Aliss Neal Gompa Nick Shyrokovskiy Nickys Music Group Nico Pache Nikolay Shirokovskiy Olaf Hering Olesya Gerasimenko Orion Poplawski Pany Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino Toscano Piotr Drąg Prathamesh Chavan Ricky Tigg Roman Bogorodskiy Roman Bolshakov Ryan Gahagan Ryan Schmidt Sam Hartman Scott Shambarger Sebastian Mitterle Shalini Chellathurai Saroja Shaojun Yang Shi Lei Simon Gaiser Stefan Bader Stefan Berger Stefan Berger Szymon Scholz Thomas Huth Tim Wiederhake Tomáš Golembiovský Tomáš Janoušek Tuguoyi Ville Skyttä Wang Xin Weblate Yalei Li <274268...@qq.com> Yalei Li Yang Hang Yanqiu Zhang Yi Li Yi Wang Yuri Chornoivan Zheng Chuan zhenwei pi Zhenyu Zheng jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt
[qemu-mainline test] 159828: regressions - FAIL
flight 159828 qemu-mainline real [real] flight 159833 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/159828/ http://logs.test-lab.xenproject.org/osstest/logs/159833/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 152631 test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail REGR. vs. 152631 test-armhf-armhf-xl-vhd 17 guest-start/debian.repeat fail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152631 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152631 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail like 152631 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 152631 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152631 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152631 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 152631 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152631 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: qemuucb90ecf9349198558569f6c86c4c27d215406095 baseline version: qemuu1d806cef0e38b5db8347a8e12f214d543204a314 Last test of basis 152631 2020-08-20 09:07:46 Z 196 days Failing since152659 2020-08-21 14:07:39 Z 195 days 377 attempts Testing same since 159828 2021-03-04 14:08:29 Z0 days1 attempts 433 people touched revisions under test, not listing them all jobs:
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On 04/03/2021 14:47, Roger Pau Monne wrote: > Introduce an option to allow selecting a less strict behaviour for > rdmsr accesses targeting a MSR not explicitly handled by Xen. Since > commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by > Xen result in the injection of a #GP to the guest. This is a behavior > change since previously a #GP was only injected if accessing the MSR > on the real hardware will also trigger a #GP. > > This commit attempts to offer a fallback option similar to the > previous behavior. Note however that the value of the underlying MSR > is never leaked to the guest, as the newly introduced option only > changes whether a #GP is injected or not. > > Long term the plan is to properly handle all the MSRs, so the option > introduced here should be considered a temporary resort for OSes that > don't work properly with the new MSR policy. Any OS that requires this > option to be enabled should be reported to > xen-devel@lists.xenproject.org. > > Signed-off-by: Roger Pau Monné > --- > Changes since v1: > - Only apply the option to HVM guests. > - Only apply the special handling to MSR reads. > - Sanitize the newly introduced flags field. > - Print a warning message when the option is used. > --- > Cc: Boris Ostrovsky > --- > Boris, could you please test with Solaris to see if this fixes the > issue? > > I wonder whether we need to to enable this option by default for > guests being migrated from previous Xen versions? Maybe that's not > required as the option is helpful mostly for early boot I would > assume, afterwards an OS should already have the #GP handler setup > when accessing MSRs. We know when building a domain whether it is a migrate or not, but don't recall any version information existing at an appropriate point in the migration stream to do this easily. We can buffer the stream forward and peek at the libxc domain header, which does have the source hypervisor version, but that is going to be very invasive to implement. > > From a release PoV the biggest risk would be breaking some of the > existing MSR functionality. I think that's a necessary risk in order > to offer such fallback option, or else we might discover after the > release that guests that worked on Xen 4.14 don't work anymore in Xen > 4.15. Much as I'd prefer not to have this, I agree with the sentiment that we should have an "emergency undo" which people can use, and carry it for at least a short while. However, to be useful for the purpose of unbreaking VMs, it must change both the read and write behaviour, because both are potential compatibility concerns (without reintroducing the information leak). > --- > docs/man/xl.cfg.5.pod.in | 17 + > tools/include/libxl.h | 8 > tools/libs/light/libxl_types.idl | 2 ++ > tools/libs/light/libxl_x86.c | 4 > tools/xl/xl_parse.c | 7 +++ > xen/arch/x86/domain.c | 10 ++ > xen/arch/x86/hvm/svm/svm.c| 6 ++ > xen/arch/x86/hvm/vmx/vmx.c| 7 +++ > xen/include/asm-x86/hvm/domain.h | 3 +++ > xen/include/public/arch-x86/xen.h | 8 This needs changes to the Ocaml bindings as well. I guess I'll add that to the todo list. ~Andrew
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On 04/03/2021 23:09, Boris Ostrovsky wrote: > On 3/4/21 9:47 AM, Roger Pau Monne wrote: >> Introduce an option to allow selecting a less strict behaviour for >> rdmsr accesses targeting a MSR not explicitly handled by Xen. Since >> commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by >> Xen result in the injection of a #GP to the guest. This is a behavior >> change since previously a #GP was only injected if accessing the MSR >> on the real hardware will also trigger a #GP. >> >> This commit attempts to offer a fallback option similar to the >> previous behavior. Note however that the value of the underlying MSR >> is never leaked to the guest, as the newly introduced option only >> changes whether a #GP is injected or not. >> >> Long term the plan is to properly handle all the MSRs, so the option >> introduced here should be considered a temporary resort for OSes that >> don't work properly with the new MSR policy. Any OS that requires this >> option to be enabled should be reported to >> xen-devel@lists.xenproject.org. >> >> Signed-off-by: Roger Pau Monné >> --- >> Changes since v1: >> - Only apply the option to HVM guests. >> - Only apply the special handling to MSR reads. >> - Sanitize the newly introduced flags field. >> - Print a warning message when the option is used. >> --- >> Cc: Boris Ostrovsky >> --- >> Boris, could you please test with Solaris to see if this fixes the >> issue? > > Yes, still works. (It worked especially well after I noticed new option name > ;-)) I'm afraid I want to break and rework how this bugfix happens. Solaris is still broken on all older branches and this isn't a suitable fix to backport. ~Andrew
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On 3/4/21 9:47 AM, Roger Pau Monne wrote: > Introduce an option to allow selecting a less strict behaviour for > rdmsr accesses targeting a MSR not explicitly handled by Xen. Since > commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by > Xen result in the injection of a #GP to the guest. This is a behavior > change since previously a #GP was only injected if accessing the MSR > on the real hardware will also trigger a #GP. > > This commit attempts to offer a fallback option similar to the > previous behavior. Note however that the value of the underlying MSR > is never leaked to the guest, as the newly introduced option only > changes whether a #GP is injected or not. > > Long term the plan is to properly handle all the MSRs, so the option > introduced here should be considered a temporary resort for OSes that > don't work properly with the new MSR policy. Any OS that requires this > option to be enabled should be reported to > xen-devel@lists.xenproject.org. > > Signed-off-by: Roger Pau Monné > --- > Changes since v1: > - Only apply the option to HVM guests. > - Only apply the special handling to MSR reads. > - Sanitize the newly introduced flags field. > - Print a warning message when the option is used. > --- > Cc: Boris Ostrovsky > --- > Boris, could you please test with Solaris to see if this fixes the > issue? Yes, still works. (It worked especially well after I noticed new option name ;-)) -boris
[linux-5.4 test] 159826: tolerable FAIL - PUSHED
flight 159826 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/159826/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159702 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 159702 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159702 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 159702 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159702 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159702 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159702 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 159702 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 159702 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 159702 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 159702 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass version targeted for testing: linux7f324ea75baa059ea126cddd4141198895880a69 baseline version: linuxef1fcccf6e5fe3aabe7c3590964efac6d5220c43 Last test of basis 159702 2021-02-26 09:41:36 Z6 days Testing same since 159826 2021-03-04 09:40:07 Z0 days1 attempts 336 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64
Re: [PATCH v3 4/7] hw/core: implement a guest-loader to support static hypervisor guests
On Wed, Mar 3, 2021 at 12:37 PM Alex Bennée wrote:
>
> Hypervisors, especially type-1 ones, need the firmware/bootcode to put
> their initial guest somewhere in memory and pass the information to it
> via platform data. The guest-loader is modelled after the generic
> loader for exactly this sort of purpose:
>
> $QEMU $ARGS -kernel ~/xen.git/xen/xen \
> -append "dom0_mem=1G,max:1G loglvl=all guest_loglvl=all" \
> -device
> guest-loader,addr=0x4200,kernel=Image,bootargs="root=/dev/sda2 ro
> console=hvc0 earlyprintk=xen" \
> -device guest-loader,addr=0x4700,initrd=rootfs.cpio
>
> Signed-off-by: Alex Bennée
> Message-Id: <20201105175153.30489-5-alex.ben...@linaro.org>
> Message-Id: <20210211171945.18313-5-alex.ben...@linaro.org>
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/core/guest-loader.h | 34 ++
> hw/core/guest-loader.c | 145 +
> MAINTAINERS| 5 ++
> hw/core/meson.build| 2 +
> 4 files changed, 186 insertions(+)
> create mode 100644 hw/core/guest-loader.h
> create mode 100644 hw/core/guest-loader.c
>
> diff --git a/hw/core/guest-loader.h b/hw/core/guest-loader.h
> new file mode 100644
> index 00..07f4b4884b
> --- /dev/null
> +++ b/hw/core/guest-loader.h
> @@ -0,0 +1,34 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée
> + * (based on the generic-loader by Li Guang )
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef GUEST_LOADER_H
> +#define GUEST_LOADER_H
> +
> +#include "hw/qdev-core.h"
> +#include "qom/object.h"
> +
> +struct GuestLoaderState {
> +/* */
> +DeviceState parent_obj;
> +
> +/* */
> +uint64_t addr;
> +char *kernel;
> +char *args;
> +char *initrd;
> +};
> +
> +#define TYPE_GUEST_LOADER "guest-loader"
> +OBJECT_DECLARE_SIMPLE_TYPE(GuestLoaderState, GUEST_LOADER)
> +
> +#endif
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> new file mode 100644
> index 00..bde44e27b4
> --- /dev/null
> +++ b/hw/core/guest-loader.c
> @@ -0,0 +1,145 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée
> + * (based on the generic-loader by Li Guang )
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +/*
> + * Much like the generic-loader this is treated as a special device
> + * inside QEMU. However unlike the generic-loader this device is used
> + * to load guest images for hypervisors. As part of that process the
> + * hypervisor needs to have platform information passed to it by the
> + * lower levels of the stack (e.g. firmware/bootloader). If you boot
> + * the hypervisor directly you use the guest-loader to load the Dom0
> + * or equivalent guest images in the right place in the same way a
> + * boot loader would.
> + *
> + * This is only relevant for full system emulation.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/cpu.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/dma.h"
> +#include "hw/loader.h"
> +#include "hw/qdev-properties.h"
> +#include "qapi/error.h"
> +#include "qemu/module.h"
> +#include "guest-loader.h"
> +#include "sysemu/device_tree.h"
> +#include "hw/boards.h"
> +
> +/*
> + * Insert some FDT nodes for the loaded blob.
> + */
> +static void loader_insert_platform_data(GuestLoaderState *s, int size,
> +Error **errp)
> +{
> +MachineState *machine = MACHINE(qdev_get_machine());
> +void *fdt = machine->fdt;
> +g_autofree char *node = g_strdup_printf("/chosen/module@0x%08" PRIx64,
> +s->addr);
> +uint64_t reg_attr[2] = {cpu_to_be64(s->addr), cpu_to_be64(size)};
> +
> +if (!fdt) {
> +error_setg(errp, "Cannot modify FDT fields if the machine has none");
> +return;
> +}
> +
> +qemu_fdt_add_subnode(fdt, node);
> +qemu_fdt_setprop(fdt, node, "reg", _attr, sizeof(reg_attr));
> +
> +if (s->kernel) {
> +const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> +if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> + (char **) ,
> + ARRAY_SIZE(compat)) < 0) {
> +error_setg(errp, "couldn't set %s/compatible", node);
> +return;
> +}
> +if (s->args) {
> +if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0)
> {
> +error_setg(errp, "couldn't set %s/bootargs", node);
> +}
> +}
> +} else if (s->initrd) {
> +const char *compat[2] = { "multiboot,module",
Re: [PATCH for-4.15] tools/libxendevicemodel: Strip __XEN_TOOLS__ header guard
On 04/03/2021 13:03, Andrew Cooper wrote: > This is inappropriate for the header file of a standalone library with stable > API and ABI. > > Signed-off-by: Andrew Cooper > --- > CC: Ian Jackson > CC: Wei Liu > > Discovered when trying to actually remove the use of unstable libraries from a > trivial userspace emulator. Current users of xendevicemodel.h inherit > __XEN_TOOLS__ from libxenctrl.h (or equiv). > --- ... and this patch is broken. But CI doesn't pick it up because we've also broken the header check scripts as part of library refactoring. The 4.13 header checks do work, and point out that there is yet another set of bogus __XEN_TOOLS__ ifdef in the Xen public ABI, which hides the ioserverid_t type and (rightly) breaks the build. Which in turn proves that the header checks (for xendevicemodel.h at least) never ever checked anything originally, because they didn't pass __XEN_TOOLS__ in at the top and just saw an empty file. ~Andrew
[xen-unstable test] 159825: tolerable FAIL
flight 159825 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/159825/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat fail in 159820 pass in 159825 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 20 guest-start/debianhvm.repeat fail pass in 159820 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159820 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159820 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159820 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159820 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 159820 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 159820 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 159820 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 159820 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 159820 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 159820 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159820 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen 243036df0d55673de59c214e240b9b914d278b65 baseline version: xen 243036df0d55673de59c214e240b9b914d278b65 Last test of basis 159825 2021-03-04 08:50:06 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm pass
Re: [GIT PULL] xen: branch for v5.12-rc2
The pull request you sent on Thu, 4 Mar 2021 12:00:53 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git > for-linus-5.12b-rc2-tag has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c5a58f877ca645a3303f7a57476f2de837fdb97a Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[xen-unstable-smoke test] 159829: tolerable all pass - PUSHED
flight 159829 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/159829/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen f40e1c52e4e0a3e084b025ed6b68f1e6ebaea027 baseline version: xen 243036df0d55673de59c214e240b9b914d278b65 Last test of basis 159819 2021-03-03 19:01:33 Z0 days Testing same since 159829 2021-03-04 16:02:36 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Julien Grall Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 243036df0d..f40e1c52e4 f40e1c52e4e0a3e084b025ed6b68f1e6ebaea027 -> smoke
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
Roger Pau Monné writes ("Re: [PATCH v2 for-4.15] x86/msr: introduce an option
for HVM relaxed rdmsr behavior"):
> One option we could go for is making this behavior depend on Kconfig:
> enable strict MSR policy for debug builds and fallback to the
> 'relaxed' one for non-debug builds. That might get us some more data,
> but again I fear most people out there will run non-debug builds
> anyway.
Hmmm.
Well, anyway, my R-A for this patch stands.
I wanted to explore our options, but I won't try to insist on a change
to the default configuration if you experts are against it.
Ian.
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On Thu, Mar 04, 2021 at 05:13:15PM +, Ian Jackson wrote:
> Roger Pau Monné writes ("Re: [PATCH v2 for-4.15] x86/msr: introduce an option
> for HVM relaxed rdmsr behavior"):
> > On Thu, Mar 04, 2021 at 03:20:34PM +, Ian Jackson wrote:
> > > The guest could be stopped with xl shutdown and then recrated with xl
> > > create, from the config file. I don't think we want to break that use
> > > case here either.
> >
> > So my original approach was to actually risk breaking creation from
> > config file and require the user to set the rdmsr_relaxed option, and
> > report the problem upstream. I think ideally we would like to get to a
> > point where we could drop the rdmsr_relaxed option, but maybe that's
> > too optimistic.
>
> Isn't there some way we can move in this direction without the first
> thing that users experience being their guests not being able to be
> created ?
>
> Maybe we could print a warning on the console or something ?
I (sadly) fear unless you get a guest crash no-one will ever look at
the logs and notice those messages.
> > We have done quite a lot of testing of this new policy, but obviously
> > it's not possible to test all possible guest OSes. Forcing the new
> > policy by default might be too risky, so indeed falling back to
> > enabling this by default could be the only solution.
> >
> > The main downside of enabling by default is that then we have to
> > resign to always having this kind of quirky behavior for MSR
> > accesses as the default.
>
> What would stop us changing the default later, when we had a better
> idea of the set of RDMSRs that need to be special-cased ?
We could. From the Citrix side I'm afraid there's not much more
testing that we can do however. We tested the new policy against all
possible guests known by XenRT, but obviously that's not every
possible OS.
One option we could go for is making this behavior depend on Kconfig:
enable strict MSR policy for debug builds and fallback to the
'relaxed' one for non-debug builds. That might get us some more data,
but again I fear most people out there will run non-debug builds
anyway.
Thanks, Roger.
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
Roger Pau Monné writes ("Re: [PATCH v2 for-4.15] x86/msr: introduce an option
for HVM relaxed rdmsr behavior"):
> On Thu, Mar 04, 2021 at 03:20:34PM +, Ian Jackson wrote:
> > The guest could be stopped with xl shutdown and then recrated with xl
> > create, from the config file. I don't think we want to break that use
> > case here either.
>
> So my original approach was to actually risk breaking creation from
> config file and require the user to set the rdmsr_relaxed option, and
> report the problem upstream. I think ideally we would like to get to a
> point where we could drop the rdmsr_relaxed option, but maybe that's
> too optimistic.
Isn't there some way we can move in this direction without the first
thing that users experience being their guests not being able to be
created ?
Maybe we could print a warning on the console or something ?
> We have done quite a lot of testing of this new policy, but obviously
> it's not possible to test all possible guest OSes. Forcing the new
> policy by default might be too risky, so indeed falling back to
> enabling this by default could be the only solution.
>
> The main downside of enabling by default is that then we have to
> resign to always having this kind of quirky behavior for MSR
> accesses as the default.
What would stop us changing the default later, when we had a better
idea of the set of RDMSRs that need to be special-cased ?
Ian.
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On Thu, Mar 04, 2021 at 03:20:34PM +, Ian Jackson wrote:
> Roger Pau Monné writes ("Re: [PATCH v2 for-4.15] x86/msr: introduce an option
> for HVM relaxed rdmsr behavior"):
> > On Thu, Mar 04, 2021 at 02:59:38PM +, Ian Jackson wrote:
> > > I think it's almost as bad to have guests which can be migrated in,
> > > but which then cannot reboot.
> >
> > Ups, yes, right.
> >
> > > Historically we have taken the view that new Xen must support old
> > > guests, even if that means being bug-compatible. So I am strongly in
> > > favour of avoiding such a usability regression.
> >
> > I'm not a xl/libxl expert, but couldn't we set the option in a
> > persistent way for migrated-in guests?
> >
> > IIRC at domain creation libxl knows whether it's a restore or a fresh
> > domain, and hence we could set the option there?
> >
> > The part I'm not sure is about how to make it persistent.
>
> The guest could be stopped with xl shutdown and then recrated with xl
> create, from the config file. I don't think we want to break that use
> case here either.
So my original approach was to actually risk breaking creation from
config file and require the user to set the rdmsr_relaxed option, and
report the problem upstream. I think ideally we would like to get to a
point where we could drop the rdmsr_relaxed option, but maybe that's
too optimistic.
We have done quite a lot of testing of this new policy, but obviously
it's not possible to test all possible guest OSes. Forcing the new
policy by default might be too risky, so indeed falling back to
enabling this by default could be the only solution.
The main downside of enabling by default is that then we have to
resign to always having this kind of quirky behavior for MSR
accesses as the default.
Thanks, Roger.
[linux-linus test] 159823: regressions - FAIL
flight 159823 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/159823/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 Tests which are failing intermittently (not blocking): test-amd64-amd64-examine4 memdisk-try-append fail in 159818 pass in 159823 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10 fail pass in 159818 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 159818 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
Roger Pau Monné writes ("Re: [PATCH v2 for-4.15] x86/msr: introduce an option
for HVM relaxed rdmsr behavior"):
> On Thu, Mar 04, 2021 at 02:59:38PM +, Ian Jackson wrote:
> > I think it's almost as bad to have guests which can be migrated in,
> > but which then cannot reboot.
>
> Ups, yes, right.
>
> > Historically we have taken the view that new Xen must support old
> > guests, even if that means being bug-compatible. So I am strongly in
> > favour of avoiding such a usability regression.
>
> I'm not a xl/libxl expert, but couldn't we set the option in a
> persistent way for migrated-in guests?
>
> IIRC at domain creation libxl knows whether it's a restore or a fresh
> domain, and hence we could set the option there?
>
> The part I'm not sure is about how to make it persistent.
The guest could be stopped with xl shutdown and then recrated with xl
create, from the config file. I don't think we want to break that use
case here either.
Ian.
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
On Thu, Mar 04, 2021 at 02:59:38PM +, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 for-4.15] x86/msr: introduce an option for
> HVM relaxed rdmsr behavior"):
> > Introduce an option to allow selecting a less strict behaviour for
> > rdmsr accesses targeting a MSR not explicitly handled by Xen. Since
> > commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by
> > Xen result in the injection of a #GP to the guest. This is a behavior
> > change since previously a #GP was only injected if accessing the MSR
> > on the real hardware will also trigger a #GP.
> ...
> > I wonder whether we need to to enable this option by default for
> > guests being migrated from previous Xen versions? Maybe that's not
> > required as the option is helpful mostly for early boot I would
> > assume, afterwards an OS should already have the #GP handler setup
> > when accessing MSRs.
>
> I think it's almost as bad to have guests which can be migrated in,
> but which then cannot reboot.
Ups, yes, right.
> Historically we have taken the view that new Xen must support old
> guests, even if that means being bug-compatible. So I am strongly in
> favour of avoiding such a usability regression.
I'm not a xl/libxl expert, but couldn't we set the option in a
persistent way for migrated-in guests?
IIRC at domain creation libxl knows whether it's a restore or a fresh
domain, and hence we could set the option there?
The part I'm not sure is about how to make it persistent.
Thanks, Roger.
Re: [PATCH XENSTORE v1 09/10] xs: handle daemon socket error
Norbert Manthey writes ("Re: [PATCH XENSTORE v1 09/10] xs: handle daemon socket
error"):
> On 3/3/21 5:13 PM, Ian Jackson wrote:
> > Norbert Manthey writes ("[PATCH XENSTORE v1 09/10] xs: handle daemon socket
> > error"):
> >> When starting the daemon, we might see a NULL pointer instead of the
> >> path to the socket.
..
> > ... this is client code ?
>
> This is client code, yes. The patched 'get_handle' function receives the
> parameter 'connect_to' in the function xs_open. There, the value of the
> functions 'xs_deamon_socket_ro', 'xs_deamon_socket' and 'xs_domain_dev'
> are passed to this function, without checking for the value NULL.
>
> I agree that the description might be confusing, as the fix is applied
> to a function that does not cause the actual problem. How about
> rephrasing the first part of the commit message to the above proposal?
Improving the commit message seems to me to be needed in any case
since as far as I can tell from what I read here, the existing one is
actualy wrong :-).
With my maintainer/reviewer hat on, I think this new exit path
involves returning an error from this function without setting errno,
so it's wrong ?
As for the release, I think this is a very low-impact bug and now it
seems not 100% obvious, so unless someone would like to explain why it
should go into 4.15 I'd like to see it in -next.
Thanks,
Ian.
Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
Roger Pau Monne writes ("[PATCH v2 for-4.15] x86/msr: introduce an option for
HVM relaxed rdmsr behavior"):
> Introduce an option to allow selecting a less strict behaviour for
> rdmsr accesses targeting a MSR not explicitly handled by Xen. Since
> commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by
> Xen result in the injection of a #GP to the guest. This is a behavior
> change since previously a #GP was only injected if accessing the MSR
> on the real hardware will also trigger a #GP.
...
> I wonder whether we need to to enable this option by default for
> guests being migrated from previous Xen versions? Maybe that's not
> required as the option is helpful mostly for early boot I would
> assume, afterwards an OS should already have the #GP handler setup
> when accessing MSRs.
I think it's almost as bad to have guests which can be migrated in,
but which then cannot reboot.
Historically we have taken the view that new Xen must support old
guests, even if that means being bug-compatible. So I am strongly in
favour of avoiding such a usability regression.
Which I think means enabling this option by default ?
> >From a release PoV the biggest risk would be breaking some of the
> existing MSR functionality. I think that's a necessary risk in order
> to offer such fallback option, or else we might discover after the
> release that guests that worked on Xen 4.14 don't work anymore in Xen
> 4.15.
Yes.
Release-Acked-by: Ian Jackson
Ian.
Re: [PATCH XENSTORE v1 09/10] xs: handle daemon socket error
On 3/3/21 5:13 PM, Ian Jackson wrote:
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
>
>
>
> Norbert Manthey writes ("[PATCH XENSTORE v1 09/10] xs: handle daemon socket
> error"):
>> When starting the daemon, we might see a NULL pointer instead of the
>> path to the socket.
This first sentence could be more specific, i.e.:
When connecting to the deamon in xs_open, the functions that return the
socket or device location might return NULL in corner cases.
>>
>> Only relevant in case we start the process in a very deep directory
>> path, with a length close to 4096 so that appending "/socket" would
>> exceed the limit. Hence, such an error is unlikely, but should still be
>> fixed to not result in a NULL pointer dereference.
> This description talks about starting the daemon ...
>
>> ---
>> tools/libs/store/xs.c | 3 +++
>> 1 file changed, 3 insertions(+)
> But I think ...
>
>> + if (!connect_to)
>> + return NULL;
>> +
> ... this is client code ?
This is client code, yes. The patched 'get_handle' function receives the
parameter 'connect_to' in the function xs_open. There, the value of the
functions 'xs_deamon_socket_ro', 'xs_deamon_socket' and 'xs_domain_dev'
are passed to this function, without checking for the value NULL.
I agree that the description might be confusing, as the fix is applied
to a function that does not cause the actual problem. How about
rephrasing the first part of the commit message to the above proposal?
Best,
Norbert
>
> Apologies if I am confused.
>
> Ian.
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
[PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior
Introduce an option to allow selecting a less strict behaviour for
rdmsr accesses targeting a MSR not explicitly handled by Xen. Since
commit 84e848fd7a162f669 accesses to MSRs not explicitly handled by
Xen result in the injection of a #GP to the guest. This is a behavior
change since previously a #GP was only injected if accessing the MSR
on the real hardware will also trigger a #GP.
This commit attempts to offer a fallback option similar to the
previous behavior. Note however that the value of the underlying MSR
is never leaked to the guest, as the newly introduced option only
changes whether a #GP is injected or not.
Long term the plan is to properly handle all the MSRs, so the option
introduced here should be considered a temporary resort for OSes that
don't work properly with the new MSR policy. Any OS that requires this
option to be enabled should be reported to
xen-devel@lists.xenproject.org.
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Only apply the option to HVM guests.
- Only apply the special handling to MSR reads.
- Sanitize the newly introduced flags field.
- Print a warning message when the option is used.
---
Cc: Boris Ostrovsky
---
Boris, could you please test with Solaris to see if this fixes the
issue?
I wonder whether we need to to enable this option by default for
guests being migrated from previous Xen versions? Maybe that's not
required as the option is helpful mostly for early boot I would
assume, afterwards an OS should already have the #GP handler setup
when accessing MSRs.
>From a release PoV the biggest risk would be breaking some of the
existing MSR functionality. I think that's a necessary risk in order
to offer such fallback option, or else we might discover after the
release that guests that worked on Xen 4.14 don't work anymore in Xen
4.15.
---
docs/man/xl.cfg.5.pod.in | 17 +
tools/include/libxl.h | 8
tools/libs/light/libxl_types.idl | 2 ++
tools/libs/light/libxl_x86.c | 4
tools/xl/xl_parse.c | 7 +++
xen/arch/x86/domain.c | 10 ++
xen/arch/x86/hvm/svm/svm.c| 6 ++
xen/arch/x86/hvm/vmx/vmx.c| 7 +++
xen/include/asm-x86/hvm/domain.h | 3 +++
xen/include/public/arch-x86/xen.h | 8
10 files changed, 72 insertions(+)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 040374dcd6..62178b9829 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2861,6 +2861,23 @@ No MCA capabilities in above list are enabled.
=back
+=item B
+
+Select whether to use a relaxed behavior for read accesses to MSRs not
+explicitly handled by Xen instead of injecting a #GP to the guest. Such access
+mode will force Xen to replicate the behaviour from the hardware it's currently
+running on in order to decide whether a #GP is injected to the guest. Note
+that the guest is never allowed to read the value of unhandled MSRs, this
+option only changes whether a #GP might be injected or not.
+
+This option is only relevant for HVM guests, and will be removed in future
+releases once we are certain the default MSR access policy has been properly
+tested by a wide variety of guests. If you need to use this option please send
+a bug report to xen-devel@lists.xenproject.org with the details of the guests
+you are running that require it.
+
+=back
+
=back
=head1 SEE ALSO
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index a7b673e89d..1cc40a2d67 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -495,6 +495,14 @@
*/
#define LIBXL_HAVE_VMTRACE_BUF_KB 1
+/*
+ * LIBXL_HAVE_RDMSR_RELAXED indicates the toolstack has support for switching
+ * the rdmsr handling in the hypervisor to relaxed mode, where #GP is only
+ * injected to guests for unhandled MSRs if accessing the MSR on the physical
+ * hardware also triggers a #GP.
+ */
+#define LIBXL_HAVE_RDMSR_RELAXED 1
+
/*
* libxl ABI compatibility
*
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 5b85a7419f..03b0c80146 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -644,6 +644,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
("vuart", libxl_vuart_type),
])),
+("arch_x86", Struct(None, [("rdmsr_relaxed", libxl_defbool),
+ ])),
# Alternate p2m is not bound to any architecture or guest type, as it is
# supported by x86 HVM and ARM support is planned.
("altp2m", libxl_altp2m_mode),
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 58187ed760..c9cff44088 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -5,9 +5,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
On 04/03/2021 14:02, Andrew Cooper wrote: > On 01/03/2021 19:33, Boris Ostrovsky wrote: >> On 3/1/21 11:23 AM, Roger Pau Monne wrote: >>> Introduce an option to allow selecting the legacy behavior for >>> accesses to MSRs not explicitly handled. Since commit >>> 84e848fd7a162f669 and 322ec7c89f6640e accesses to MSRs not explicitly >>> handled by Xen result in the injection of a #GP to the guest. This is >>> a behavior change since previously a #GP was only injected if >>> accessing the MSR on the real hardware will also trigger a #GP. >>> >>> This seems to be problematic for some guests, so introduce an option >>> to fallback to this legacy behavior. The main difference between what >>> was previously done is that the hardware MSR value is not leaked to >>> the guests on reads. >>> >>> Signed-off-by: Roger Pau Monné >>> --- >>> Cc: Boris Ostrovsky >>> --- >>> Note that this option is not made available to dom0. I'm not sure >>> whether it makes sense to do so, since anyone updating Xen to such >>> newer version will also likely pair it with a newish kernel that >>> doesn't require such workarounds. >>> >>> RFC because there's still some debate as to how we should solve the >>> MSR issue, this is one possible way, but IMO we need to make a >>> decision soon-ish because of the release timeline. >>> >>> Boris, could you please test with Solaris to see if this fixes the >>> issue? >> Yes, it does. Thanks. > Really? This doesn't stop #GP being raised for RAPL_POWER_LIMIT > AFAICT. Or am I mistaken about how the bug manifested? Actually never mind. I've figured out why. I have another bugfix to submit. ~Andrew
Re: [PATCH for-4.15] tools/libxendevicemodel: Strip __XEN_TOOLS__ header guard
Andrew Cooper writes ("[PATCH for-4.15] tools/libxendevicemodel: Strip
__XEN_TOOLS__ header guard"):
> This is inappropriate for the header file of a standalone library with stable
> API and ABI.
>
> Signed-off-by: Andrew Cooper
Reviewed-by: Ian Jackson
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
On 01/03/2021 19:33, Boris Ostrovsky wrote: > On 3/1/21 11:23 AM, Roger Pau Monne wrote: >> Introduce an option to allow selecting the legacy behavior for >> accesses to MSRs not explicitly handled. Since commit >> 84e848fd7a162f669 and 322ec7c89f6640e accesses to MSRs not explicitly >> handled by Xen result in the injection of a #GP to the guest. This is >> a behavior change since previously a #GP was only injected if >> accessing the MSR on the real hardware will also trigger a #GP. >> >> This seems to be problematic for some guests, so introduce an option >> to fallback to this legacy behavior. The main difference between what >> was previously done is that the hardware MSR value is not leaked to >> the guests on reads. >> >> Signed-off-by: Roger Pau Monné >> --- >> Cc: Boris Ostrovsky >> --- >> Note that this option is not made available to dom0. I'm not sure >> whether it makes sense to do so, since anyone updating Xen to such >> newer version will also likely pair it with a newish kernel that >> doesn't require such workarounds. >> >> RFC because there's still some debate as to how we should solve the >> MSR issue, this is one possible way, but IMO we need to make a >> decision soon-ish because of the release timeline. >> >> Boris, could you please test with Solaris to see if this fixes the >> issue? > > Yes, it does. Thanks. Really? This doesn't stop #GP being raised for RAPL_POWER_LIMIT AFAICT. Or am I mistaken about how the bug manifested? ~Andrew
[qemu-mainline test] 159822: regressions - FAIL
flight 159822 qemu-mainline real [real] flight 159827 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/159822/ http://logs.test-lab.xenproject.org/osstest/logs/159827/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 152631 test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail REGR. vs. 152631 test-armhf-armhf-xl-vhd 17 guest-start/debian.repeat fail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152631 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152631 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 152631 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152631 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152631 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 152631 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152631 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: qemuuc40ae5a3ee387b13116948cbfe7824f03311db7e baseline version: qemuu1d806cef0e38b5db8347a8e12f214d543204a314 Last test of basis 152631 2020-08-20 09:07:46 Z 196 days Failing since152659 2020-08-21 14:07:39 Z 194 days 376 attempts Testing same since 159822 2021-03-04 02:04:12 Z0 days1 attempts 433 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm
Re: [PATCH for-4.15] tools/libxendevicemodel: Strip __XEN_TOOLS__ header guard
Andrew Cooper writes ("[PATCH for-4.15] tools/libxendevicemodel: Strip
__XEN_TOOLS__ header guard"):
> This is inappropriate for the header file of a standalone library with stable
> API and ABI.
wat
> Discovered when trying to actually remove the use of unstable libraries from a
> trivial userspace emulator. Current users of xendevicemodel.h inherit
> __XEN_TOOLS__ from libxenctrl.h (or equiv).
Release-Acked-by: Ian Jackson
Re: [PATCH for-4.15 1/2] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
On 04/03/2021 13:01, Andrew Cooper wrote:
> The const_op boolean needs clobbering to cause data to be written back to the
> caller.
>
> Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
> Signed-off-by: Andrew Cooper
> Reviewed-by: Roger Pau Monné
> Release-Acked-by: Ian Jackson
Apologies - I made a mistake with git-send-email. No further action
required.
~Andrew
[PATCH for-4.15] tools/libxendevicemodel: Strip __XEN_TOOLS__ header guard
This is inappropriate for the header file of a standalone library with stable API and ABI. Signed-off-by: Andrew Cooper --- CC: Ian Jackson CC: Wei Liu Discovered when trying to actually remove the use of unstable libraries from a trivial userspace emulator. Current users of xendevicemodel.h inherit __XEN_TOOLS__ from libxenctrl.h (or equiv). --- tools/include/xendevicemodel.h | 4 1 file changed, 4 deletions(-) diff --git a/tools/include/xendevicemodel.h b/tools/include/xendevicemodel.h index 33698d67f3..797e0c6b29 100644 --- a/tools/include/xendevicemodel.h +++ b/tools/include/xendevicemodel.h @@ -17,8 +17,6 @@ #ifndef XENDEVICEMODEL_H #define XENDEVICEMODEL_H -#ifdef __XEN_TOOLS__ - #include #include @@ -377,8 +375,6 @@ int xendevicemodel_nr_vcpus( */ int xendevicemodel_restrict(xendevicemodel_handle *dmod, domid_t domid); -#endif /* __XEN_TOOLS__ */ - #endif /* XENDEVICEMODEL_H */ /* -- 2.11.0
[PATCH for-4.15 1/2] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
The const_op boolean needs clobbering to cause data to be written back to the
caller.
Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
Release-Acked-by: Ian Jackson
---
xen/arch/arm/dm.c | 1 +
xen/arch/x86/hvm/dm.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index d689e336fd..1b3fd6bc7d 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -128,6 +128,7 @@ int dm_op(const struct dmop_args *op_args)
struct xen_dm_op_nr_vcpus *data = _vcpus;
data->vcpus = d->max_vcpus;
+const_op = false;
rc = 0;
break;
}
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index f4f0910463..b60b9f3364 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -612,6 +612,7 @@ int dm_op(const struct dmop_args *op_args)
struct xen_dm_op_nr_vcpus *data = _vcpus;
data->vcpus = d->max_vcpus;
+const_op = false;
rc = 0;
break;
}
--
2.11.0
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
Jan Beulich writes ("[PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype
for gcc11"):
> Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
>
> --- a/xen/include/crypto/rijndael.h
> +++ b/xen/include/crypto/rijndael.h
> @@ -52,7 +52,7 @@
>
> int rijndaelKeySetupEnc(unsigned int [], const unsigned char [], int);
> int rijndaelKeySetupDec(unsigned int [], const unsigned char [], int);
> -void rijndaelEncrypt(const unsigned int [], int, const unsigned char [],
> - unsigned char []);
> +void rijndaelEncrypt(const unsigned int [], int, const unsigned char [16],
> + unsigned char [16]);
>
> #endif /* __RIJNDAEL_H */
Release-Acked-by: Ian Jackson
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
On 04.03.2021 13:41, Ian Jackson wrote:
> Julien Grall writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
> prototype for gcc11"):
>> On 04/03/2021 11:21, Ian Jackson wrote:
>>> Jan Beulich writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
>>> prototype for gcc11"):
> ...
>>> It has been idiomatic in some codebases for a long time to write
>>> const unsigned char[]
>>> as a formal parameter for an array (of whatever size).
>>
>> AFAICT, this is not what GCC is trying to warn about. It is complaining
>> that the prototype and the declaration doesn't use the same signature.
>
> Oh! I would have to check whether that is legal (I would guess
> probably it is UB because the C authors hate us all) but I agree that
> the warning is justified and the code should be changed.
May I translate this into a release ack then?
Jan
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
On 04.03.2021 12:21, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
> prototype for gcc11"):
>> On 03.03.2021 20:09, Julien Grall wrote:
>>> On 01/03/2021 07:57, Jan Beulich wrote:
The upcoming release complains, not entirely unreasonably:
In file included from rijndael.c:33:
.../xen/include/crypto/rijndael.h:55:53: note: previously declared as
'const unsigned char[]'
55 | voidrijndaelEncrypt(const unsigned int [], int, const
unsigned char [],
|
^~
rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned
char[16]'} with mismatched bound [-Werror=array-parameter=]
865 | u8 ct[16])
>
> I think this is an erroneous compiler warning.
>
> It has been idiomatic in some codebases for a long time to write
> const unsigned char[]
> as a formal parameter for an array (of whatever size).
>
.../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified
bound 4 exceeds source size 0 [-Werror=stringop-overread]
101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
| ^~~
mpparse.c:722:13: note: in expansion of macro 'memcmp'
722 | if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
| ^~
Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
>>>
>>> It looks like there is a report on the redhat bug tracker for it [1]. Do
>>> you know if there is a bug report on the GCC tracker as well?
>>
>> I have no idea, to be honest.
>
> This erroneous message makes me think that there is simply a bug in
> this version of GCC, where formal parameters declared as
> const unsigned char[]
> are treated as
> const unsigned char[0]
> rather than as
> const unsigned char*
I don't think so, no. In addition to what Julien has said, I think
the intention here is that the compiler can check that both consumer
and producers of arguments obey to the strictest available bounds of
such an array, no matter that at the syntactic level char[N] and
char[] both convert to char* when used in prototypes.
Jan
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
Julien Grall writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
prototype for gcc11"):
> On 04/03/2021 11:21, Ian Jackson wrote:
> > Jan Beulich writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
> > prototype for gcc11"):
...
> > It has been idiomatic in some codebases for a long time to write
> > const unsigned char[]
> > as a formal parameter for an array (of whatever size).
>
> AFAICT, this is not what GCC is trying to warn about. It is complaining
> that the prototype and the declaration doesn't use the same signature.
Oh! I would have to check whether that is legal (I would guess
probably it is UB because the C authors hate us all) but I agree that
the warning is justified and the code should be changed.
Sorry for the misunderstanding.
Ian.
Re: [PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
Andrew Cooper writes ("[PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to
actually return data"):
> The const_op boolean needs clobbering to cause data to be written back to the
> caller.
>
> Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Roger Pau Monné
> CC: Wei Liu
> CC: Stefano Stabellini
> CC: Julien Grall
> CC: Volodymyr Babchuk
> CC: Ian Jackson
>
> If we weren't in a release freeze, I'd rewrite large chunks of this.
> 'const_op' is what we call 'copyback' everywhere else, but with inverted
> sense. I'll guess this gets added to the pile of other unbreakage work which
> might happen in 4.16
>
> My ad-hoc unit test appears to have had a false positive for the success case,
> which I've fixed. However, the chances of the full test landing in 4.15 is
> getting slimmer, not to mention the fact that it curretly takes out Xen with
> reference counting error...
>
> As for 4.15, this is a bug in a brand-newly introduced hypercall, and is of 0
> risk for other areas of the release. If this bugfix is not taken, we should
> revert c4441ab1f1 to take the hypercall out, but this would be a bad move.
Thanks for that explanation.
Release-Acked-by: Ian Jackson
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
On 04/03/2021 11:21, Ian Jackson wrote:
Jan Beulich writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype
for gcc11"):
On 03.03.2021 20:09, Julien Grall wrote:
On 01/03/2021 07:57, Jan Beulich wrote:
The upcoming release complains, not entirely unreasonably:
In file included from rijndael.c:33:
.../xen/include/crypto/rijndael.h:55:53: note: previously declared as 'const
unsigned char[]'
55 | voidrijndaelEncrypt(const unsigned int [], int, const unsigned
char [],
|
^~
rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned char[16]'}
with mismatched bound [-Werror=array-parameter=]
865 | u8 ct[16])
I think this is an erroneous compiler warning.
It has been idiomatic in some codebases for a long time to write
const unsigned char[]
as a formal parameter for an array (of whatever size).
AFAICT, this is not what GCC is trying to warn about. It is complaining
that the prototype and the declaration doesn't use the same signature.
Indeed, the former is using [] while the declaration is using [16].
So compiler is working as intended when -Warray-parameter is selected
(see [1]).
[1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
--
Julien Grall
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
On 04/03/2021 08:06, Jan Beulich wrote:
On 03.03.2021 20:09, Julien Grall wrote:
On 01/03/2021 07:57, Jan Beulich wrote:
The upcoming release complains, not entirely unreasonably:
In file included from rijndael.c:33:
.../xen/include/crypto/rijndael.h:55:53: note: previously declared as 'const
unsigned char[]'
55 | voidrijndaelEncrypt(const unsigned int [], int, const unsigned
char [],
|
^~
rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned char[16]'}
with mismatched bound [-Werror=array-parameter=]
865 | u8 ct[16])
| ~~~^~
In file included from rijndael.c:33:
.../xen/include/crypto/rijndael.h:56:13: note: previously declared as 'unsigned
char[]'
56 | unsigned char []);
| ^~~~
While it's not really clear to me why it would complain only for arg 4,
the adjustment to make is obvious and riskfree also for arg 3: Simply
declare the correct array dimension right away. This then allows
compilers to apply checking at call sites, which seems desirable anyway.
I am a bit confused, if GCC is not complaining for arg3, then what is
the following error message for:
> In file included from rijndael.c:33:
> .../xen/include/crypto/rijndael.h:55:53: note: previously declared as
'const unsigned char[]'
> 55 | voidrijndaelEncrypt(const unsigned int [], int, const
unsigned char [],
>|
^~
Hmm, good point. I didn't observe this myself, and simply copied the
part of the error message that I was handed. I suppose there was an
"error: argument 3 of type ..." there then as well. Charles - any
chance you could confirm this, and perhaps even quote the full set
of error messages in our internal patch?
I'll adjust the wording of the description in any event.
With the description adjusted:
Reviewed-by: Julien Grall
There are quite a few more issues with gcc11, but from my brief initial
inspection I'm suspecting (hoping) it'll rather be the compiler which
will get further changed by the time their release gets finalized. Just
one example:
.../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified bound
4 exceeds source size 0 [-Werror=stringop-overread]
101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
| ^~~
mpparse.c:722:13: note: in expansion of macro 'memcmp'
722 | if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
| ^~
Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
It looks like there is a report on the redhat bug tracker for it [1]. Do
you know if there is a bug report on the GCC tracker as well?
I have no idea, to be honest.
I had a look and couldn't find any. It might be worth to fill one in
case they are not aware.
Cheers,
--
Julien Grall
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
Jan Beulich writes ("Re: [PATCH RFC for-4.15] x86/msr: introduce an option for
legacy MSR behavior selection"):
> On 04.03.2021 11:05, Roger Pau Monné wrote:
...
> > This one seems like a fine candidate to implement in
> > svm_msr_read_intercept, because Xen needs to return a specific value
> > for this MSR.
> >
> > Regarding the global approach to fixing the fallout from the MSR
> > policy change, I don't see why we couldn't do a mix between pro-active
> > and reactive.
> >
> > I think we must have an option to fallback to something similar to the
> > old policy for HVM guests so that users have a way to get their guests
> > running after an update without requiring a code change.
> >
> > That doesn't mean we can't reactively add the missing MSRs as we go
> > discovering them. I would even print a warning when using such
> > fallback legacy MSR handling option that you need to report the issue
> > to xen-devel because the option might be removed in future releases.
> >
> > Does the above seem like a sensible plan?
>
> I think so, yes. I wonder what Andrew thinks, though.
FTR I am on board with this plan. I would like to see quick progress
on this issue as it seems like one of the major risks in the release.
Thanks,
Ian.
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
Jan Beulich writes ("Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt()
prototype for gcc11"):
> On 03.03.2021 20:09, Julien Grall wrote:
> > On 01/03/2021 07:57, Jan Beulich wrote:
> >> The upcoming release complains, not entirely unreasonably:
> >>
> >> In file included from rijndael.c:33:
> >> .../xen/include/crypto/rijndael.h:55:53: note: previously declared as
> >> 'const unsigned char[]'
> >> 55 | voidrijndaelEncrypt(const unsigned int [], int, const
> >> unsigned char [],
> >>|
> >> ^~
> >> rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned
> >> char[16]'} with mismatched bound [-Werror=array-parameter=]
> >>865 | u8 ct[16])
I think this is an erroneous compiler warning.
It has been idiomatic in some codebases for a long time to write
const unsigned char[]
as a formal parameter for an array (of whatever size).
> >> .../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified
> >> bound 4 exceeds source size 0 [-Werror=stringop-overread]
> >>101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
> >>| ^~~
> >> mpparse.c:722:13: note: in expansion of macro 'memcmp'
> >>722 | if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
> >>| ^~
> >>
> >> Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
> >
> > It looks like there is a report on the redhat bug tracker for it [1]. Do
> > you know if there is a bug report on the GCC tracker as well?
>
> I have no idea, to be honest.
This erroneous message makes me think that there is simply a bug in
this version of GCC, where formal parameters declared as
const unsigned char[]
are treated as
const unsigned char[0]
rather than as
const unsigned char*
Ian.
[GIT PULL] xen: branch for v5.12-rc2
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-5.12b-rc2-tag xen: branch for v5.12-rc2 It contains fixes for 2 security issues (XSA-367 and XSA-369). Thanks. Juergen arch/arm/xen/p2m.c| 35 ++--- arch/x86/include/asm/xen/page.h | 12 + arch/x86/xen/p2m.c| 54 ++- arch/x86/xen/setup.c | 25 +++--- drivers/net/xen-netback/netback.c | 12 - 5 files changed, 104 insertions(+), 34 deletions(-) Jan Beulich (2): Xen/gnttab: handle p2m update errors on a per-slot basis xen-netback: respect gnttab_map_refs()'s return value Juergen Gross (1): xen: fix p2m size in dom0 for disabled memory hotplug case
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
On 04.03.2021 11:05, Roger Pau Monné wrote:
> On Thu, Mar 04, 2021 at 09:48:25AM +0100, Jan Beulich wrote:
>> On 03.03.2021 16:38, Roger Pau Monné wrote:
>>> It also raises the question whether we will allow any more exceptions
>>> to the MSR policy, like we did for Windows and OpenBSD in:
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=ca88a43e660c75796656a544e54a648c60d26ef0
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=4175fd3ccd17face664036fa98e9329aa317f7b6
>>>
>>> Or if we are just going to require those guests to enable the legacy
>>> MSR handling instead.
>>
>> It is my understanding that Andrew's view is that adding such special
>> cases is the only acceptable thing. As voiced before I don't share
>> this view, as it means we're going to be entirely reactive to people
>> reporting issues, when I think we should be pro-active as far as is
>> reasonable. Independent of any pro-active measures there'll still be
>> enough reasons for reactive changes - for example I assume Linux
>> would now issue the log message from
>>
>> if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
>>
>> if (c->x86 > 0x10 ||
>> (c->x86 == 0x10 && c->x86_model >= 0x2)) {
>> u64 val;
>>
>> rdmsrl(MSR_K7_HWCR, val);
>> if (!(val & BIT(24)))
>> pr_warn(FW_BUG "TSC doesn't count with P0
>> frequency!\n");
>> }
>> }
>>
>> since we surface a zero value right now (but I didn't verify this in
>> practice yet).
>
> I think we inject a #GP to the guest if it tries to access
> MSR_K7_HWCR? As I don't see this MSR handled explicitly in
> svm_msr_read_intercept. So Linux would complain from the unchecked MSR
> access and the MSR value not having the bit set.
Right - my description was of the behavior with my workaround
already in place.
> This one seems like a fine candidate to implement in
> svm_msr_read_intercept, because Xen needs to return a specific value
> for this MSR.
>
> Regarding the global approach to fixing the fallout from the MSR
> policy change, I don't see why we couldn't do a mix between pro-active
> and reactive.
>
> I think we must have an option to fallback to something similar to the
> old policy for HVM guests so that users have a way to get their guests
> running after an update without requiring a code change.
>
> That doesn't mean we can't reactively add the missing MSRs as we go
> discovering them. I would even print a warning when using such
> fallback legacy MSR handling option that you need to report the issue
> to xen-devel because the option might be removed in future releases.
>
> Does the above seem like a sensible plan?
I think so, yes. I wonder what Andrew thinks, though.
Jan
Xen Security Advisory 369 v1 - Linux: special config may crash when trying to map foreign pages
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory XSA-369 Linux: special config may crash when trying to map foreign pages ISSUE DESCRIPTION = With CONFIG_XEN_BALLOON_MEMORY_HOTPLUG disabled and CONFIG_XEN_UNPOPULATED_ALLOC enabled the Linux kernel will use guest physical addresses allocated via the ZONE_DEVICE functionality for mapping foreign guest's pages. This will result in problems, as the p2m list will only cover the initial memory size of the domain plus some padding at the end. Most ZONE_DEVICE allocated addresses will be outside the p2m range and thus a mapping can't be established with those memory addresses, resulting in a crash. The attack involves doing I/O requiring large amounts of data to be mapped by the Dom0 or driver domain. The amount of data needed to result in a crash can vary depending on the memory layout of the affected Dom0 or driver domain. IMPACT == A Dom0 or driver domain based on a Linux kernel (configured as described above) can be crashed by a malicious guest administrator, or possibly malicious unprivileged guest processes. VULNERABLE SYSTEMS == Only x86 paravirtualized (PV) Dom0 or driver domains are affected. Only Linux kernels configured *with* CONFIG_XEN_UNPOPULATED_ALLOC and *without* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG are vulnerable. Only kernels from kernel version 5.9 onwards are affected. CONFIG_XEN_BALLOON_MEMORY_HOTPLUG is enabled by default in upstream Linux when Xen support is enabled, so kernels using upstream default Kconfig are not affected. Most distribution kernels supporting Xen dom0 use are likewise not vulnerable. Arm systems or x86 PVH or x86 HVM driver domains are not affected. MITIGATION == There is no mitigation available. RESOLUTION == Applying the appropriate attached patch resolves this issue. xsa369-linux.patch Linux 5.9-stable - 5.12-rc $ sha256sum xsa369* 937df4f078a070cf47bdd718c6b8a042ec6bee255eedc422d833c2ae3dd561c7 xsa369-linux.patch $ CREDITS === This issue was discovered by Marek Marczykowski-Górecki of Invisible Things Lab. For patch: Reported-by: Marek Marczykowski-Górecki NOTE REGARDING LACK OF EMBARGO == This was reported publicly multiple times, before the XSA could be issued. -BEGIN PGP SIGNATURE- iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmBAvMQMHHBncEB4ZW4u b3JnAAoJEIP+FMlX6CvZ5PoH/2EY28X1Fe+2RW5SrnAo2dZWLXeIrXQIXbsDCdlI GKhFChUhYHJP3wLhE4F7J5SAjl48ta/gtdpbpJWXsZSS+2KIdV/dDZ3ZA6cxWFAI DuVvqqt5O0xpF02bgTZrL1GUL8975L0O7cwtGmsIbPjVSF5UktuLS0Q1zRAiYvG9 l5Xu32nekxz2fGebMYrJTIPYNc8LOg3d+MIAE4W1u3Wj46S8yRJhyNQmsPQXZTEk nlTp0ed8ScAt7pIZn7dbnLz8zUAQ64h2yar0UBih51kd3Bss5E4PXsS0zlXlVNfk 046nBhbFfB3dgM49NlJ3oHhiZh6dN5LpMblmGK4Tb+FJqNE= =QwG+ -END PGP SIGNATURE- xsa369-linux.patch Description: Binary data
Re: [PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
On Thu, Mar 04, 2021 at 10:48:05AM +, Andrew Cooper wrote:
> The const_op boolean needs clobbering to cause data to be written back to the
> caller.
>
> Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
> Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
Thanks, Roger.
[PATCH for-4.15] xen/dmop: Fix XEN_DMOP_nr_vcpus to actually return data
The const_op boolean needs clobbering to cause data to be written back to the
caller.
Fixes: c4441ab1f1 ("dmop: Add XEN_DMOP_nr_vcpus")
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Ian Jackson
If we weren't in a release freeze, I'd rewrite large chunks of this.
'const_op' is what we call 'copyback' everywhere else, but with inverted
sense. I'll guess this gets added to the pile of other unbreakage work which
might happen in 4.16
My ad-hoc unit test appears to have had a false positive for the success case,
which I've fixed. However, the chances of the full test landing in 4.15 is
getting slimmer, not to mention the fact that it curretly takes out Xen with
reference counting error...
As for 4.15, this is a bug in a brand-newly introduced hypercall, and is of 0
risk for other areas of the release. If this bugfix is not taken, we should
revert c4441ab1f1 to take the hypercall out, but this would be a bad move.
---
xen/arch/arm/dm.c | 1 +
xen/arch/x86/hvm/dm.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index d689e336fd..1b3fd6bc7d 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -128,6 +128,7 @@ int dm_op(const struct dmop_args *op_args)
struct xen_dm_op_nr_vcpus *data = _vcpus;
data->vcpus = d->max_vcpus;
+const_op = false;
rc = 0;
break;
}
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index f4f0910463..b60b9f3364 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -612,6 +612,7 @@ int dm_op(const struct dmop_args *op_args)
struct xen_dm_op_nr_vcpus *data = _vcpus;
data->vcpus = d->max_vcpus;
+const_op = false;
rc = 0;
break;
}
--
2.11.0
Xen Security Advisory 367 v1 - Linux: netback fails to honor grant mapping errors
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory XSA-367 Linux: netback fails to honor grant mapping errors ISSUE DESCRIPTION = XSA-362 tried to address issues here, but in the case of the netback driver the changes were insufficient: It left the relevant function invocation with, effectively, no error handling at all. As a result, memory allocation failures there could still lead to frontend-induced crashes of the backend. IMPACT == A malicious or buggy networking frontend driver may be able to crash the corresponding backend driver, potentially affecting the entire domain running the backend driver. In a typical (non-disaggregated) system that is a host-wide denial of service (DoS). VULNERABLE SYSTEMS == Linux versions from at least 2.6.39 onwards are vulnerable, when run in PV mode. Earlier versions differ significantly in behavior and may therefore instead surface other issues under the same conditions. Linux run in HVM / PVH modes is not vulnerable. MITIGATION == For Linux, running the backends in HVM or PVH domains will avoid the vulnerability. For example, by running the dom0 in PVH mode. In all other cases there is no known mitigation. RESOLUTION == Applying the attached patch resolves this issue. xsa367-linux.patch Linux 5.12-rc $ sha256sum xsa367* b0244bfddee91cd7986172893e70664b74e698c5d44f25865870f179f80f9a92 xsa367-linux.patch $ CREDITS === This issue was reported by Intel's kernel test robot and recognized as a security issue by Jan Beulich of SUSE. NOTE REGARDING LACK OF EMBARGO == This issue was reported publicly, before the XSA could be issued. -BEGIN PGP SIGNATURE- iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmBAuOYMHHBncEB4ZW4u b3JnAAoJEIP+FMlX6CvZUCAH/1zw5d2l1R3k+nvJ659plwOYDe8Cmh4GeJ02PoUv fC/5efe7l/tXEmfg4rg5WiY8JZqQGeGmhwiOs8bI/8c5IXucaPOM1wDUaHUMkWTA tl/P/tbDamzd1/dSK4DdILTApibU+M/nmUn0sBBYpu53VUbeyXq2EAtjmliKgCG9 Oo4PW4ys5ro+hwrPtYdLD1ktIN64+C+TqkKUdJset7po5sWX4nV1Cwp/4oKaNyeF Alh495TUCnhgc8gnXUgXhmxWKp3Iag/tHjmtu34mT5HHZdBrNBShFKhHSP5bJHE2 CxYD1b/KbkRiLPOgZXNec+ikDQT4bTCeVLpnWvOXQ1FTXR4= =hY2s -END PGP SIGNATURE- xsa367-linux.patch Description: Binary data
Re: dom0less boot two compressed kernel images out-of-memory work-around
Hi Jan,
On 04/03/2021 08:34, Jan Beulich wrote:
On 03.03.2021 20:36, Julien Grall wrote:
(BCCing xen-users, CCing xen-devel + a few folks)
How about the following (untested):
commit e1cd2d85234c8d0aa62ad32c824a5568a57be930 (HEAD -> dev)
Author: Julien Grall
Date: Wed Mar 3 19:27:56 2021 +
xen/gunzip: Allow perform_gunzip() to be called multiple times
Currently perform_gunzip() can only be called once because the the
internal allocator is not fully re-initialized.
This works fine if you are only booting dom0. But this will break when
booting multiple using the dom0less that uses compressed kernel images.
This can be resolved by re-initializing malloc_ptr and malloc_count
every time perform_gunzip() is called.
Note the latter is only re-initialized for hardening purpose as
there is
no guarantee that every malloc() are followed by free() (It should in
theory!).
Take the opportunity to check the return of alloc_heap_pages() to
return
an error rather than dereferencing a NULL pointer later on failure.
Reported-by: Charles Chiou
Signed-off-by: Julien Grall
---
This patch is candidate for Xen 4.15. Without this patch, it will
not be
possible to boot multiple domain using dom0less when they are using
compressed kernel images.
Other decompression methods are unaffected?
Arm is only using gzip so far. I quickly looked through bunzip2 and
unlz4 (I know there are others), they look fine because they don't
allocate a large global pool.
We probably want to check the others.
--- a/xen/common/gunzip.c
+++ b/xen/common/gunzip.c
@@ -114,7 +114,11 @@ __init int perform_gunzip(char *output, char
*image, unsigned long image_len)
window = (unsigned char *)output;
free_mem_ptr = (unsigned long)alloc_xenheap_pages(HEAPORDER, 0);
+if ( !free_mem_ptr )
+return -ENOMEM;
+
free_mem_end_ptr = free_mem_ptr + (PAGE_SIZE << HEAPORDER);
+init_allocator();
inbuf = (unsigned char *)image;
insize = image_len;
diff --git a/xen/common/inflate.c b/xen/common/inflate.c
index f99c985d6135..d8c28a3e9593 100644
--- a/xen/common/inflate.c
+++ b/xen/common/inflate.c
@@ -238,6 +238,12 @@ STATIC const ush mask_bits[] = {
static unsigned long INITDATA malloc_ptr;
static int INITDATA malloc_count;
+static void init_allocator(void)
Please add INIT here. (I wouldn't mind if you used __init instead,
as there's going to be file-wide replacement after 4.15 anyway,
but of course this would render things inconsistent until then.)
I will use INIT. I will wait a bit before sending the patch formally
(I'd like a confirmation that it solves the problem reported).
Cheers,
--
Julien Grall
Re: [PATCH for-4.15] tools/xenstored: liveupdate: Properly check long transaction
Hi Juergen,
On 04/03/2021 09:48, Jürgen Groß wrote:
On 04.03.21 10:39, Julien Grall wrote:
On 04/03/2021 09:00, Jürgen Groß wrote:
On 03.03.21 18:05, Julien Grall wrote:
From: Julien Grall
As XenStored is single-threaded, conn->ta_start_time will always be
smaller than now. As we substract the latter from the former, it means
a transaction will never be considered long running.
Invert the two operands of the substraction in both lu_reject_reason()
and lu_check_allowed(). In addition to that, the former also needs to
check that conn->ta_start_time is not 0 (i.e the transaction is not
active).
Take the opportunity to document the return condition of
lu_check_allowed().
Fixes: e04e53a5be20 ("tools/xenstore: allow live update only with no
transaction active")
Reported-by: Bjoern Doebel
Signed-off-by: Julien Grall
Reviewed-by: Juergen Gross
---
I am a bit puzzled on how -F is implemented. From my understanding we
will force LiveUpdate when one of the following conditions is met:
1) All the active transactions are long running
2) If we didn't manage to LiveUpdate after N sec
It is not quite clear why we need the both as 2) would indirectly cover
1). However 2) is probably unsafe as we may reset transactions for
"well-behaving" guest.
So I am thinking to send a patch to drop 2). Any opinions?
This will enable two guests working together to block LU by using
overlapping transactions:
Guest 1: - - - ...
Guest 2: -- - - --- ... >
There is always a transaction active, but none is regarded to be
long running.
Right, how do you know whether two guests are working together?
We can't know that. And this is the reason why you have to use the -F
option to force a LU.
I understand that... But the consequence are potentially devastating on
all the other connections, correct?
I understand that "-F" means that things could break... However, I am
not entirely sure who can realistically use this option without
risking breaking other guests. For instance, there might be a 3rd
guest that has an active transaction and not cooperating with the
first two.
Yes. OTOH the chances are rather low that multiple LU attempts are
failing due to transactions being active all the time.
Give me access to your server and I can run you a workload that prevent
LiveUpdate without -F ;).
Joke aside, a guest crashing in the middle of the transaction can
prevent LiveUpdate to succeed. As the guest owner may not be the host
owner, you don't necessarily know when the problem will be remediated.
This is where I would expect -F to be useful as breaking transaction for
such guest is low-risk. However, the side-effect of -F looks quite
undesirable so far.
Rather than forcing in this case, how about we quiesce the connection
if it starts a transaction when LiveUpdate is pending?
Yes, this should work, too.
I will have a look. It is not going to be material for 4.15 though.
Cheers,
--
Julien Grall
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
On Thu, Mar 04, 2021 at 09:48:25AM +0100, Jan Beulich wrote: > On 03.03.2021 16:38, Roger Pau Monné wrote: > > On Tue, Mar 02, 2021 at 04:18:59PM +0100, Jan Beulich wrote: > >> On 02.03.2021 16:00, Roger Pau Monné wrote: > >>> On Tue, Mar 02, 2021 at 12:16:12PM +0100, Jan Beulich wrote: > On 01.03.2021 17:23, Roger Pau Monne wrote: > > RFC because there's still some debate as to how we should solve the > > MSR issue, this is one possible way, but IMO we need to make a > > decision soon-ish because of the release timeline. > > Generally I think it would be far better from a user pov if > things worked out of the box, at least in cases where it can be > made work. Arguably for Solaris this isn't going to be possible > (leaving aside the non-option of fully returning back to original > behavior), but for the old-Linux-PV case I still think my proposed > change is better in this regard. I realize Andrew said (on irc) > that this is making the behavior even weaker. I take a different > perspective: Considering a guest will install exception handlers > at some point, I continue to fail to see what good it will do to > try to inject a #GP(0) when we know the guest will die because of > not having a handler registered just yet. It is a kernel flaw, > yes, but they ended up living with it merely because of our odd > prior behavior, so we can't put all the blame on them. > >>> > >>> My concern with this would mostly be with newer guests, in the sense > >>> that people could come to rely on this behavior of not injecting a > >>> #GP if the handler is not setup, which I think we should try to avoid. > >>> > >>> One option would be to introduce a feature that could be used in the > >>> elfnotes to signal that the kernel doesn't require such workarounds > >>> for MSR #GP handling, maybe: > >>> > >>> /* > >>> * Signal that the kernel doesn't require suppressing the #GP on > >>> * unknown MSR accesses if the handler is not setup. New kernels > >>> * should always have this set. > >>> */ > >>> #define XENFEAT_msr_early_gp 16 > >>> > >>> We could try to backport this on the Linux kernel as far as we can > >>> that we know it's safe to do so. > >> > >> I too did consider something like this. While I'm not opposed, it > >> effectively requires well-behaved consumers who haven't been well- > >> behaved in the past. > >> > >>> If this is not acceptable then I guess your solution is fine. Arguably > >>> PV has it's own (weird) architecture, in which it might be safe to say > >>> that no #GP will be injected as a result of a MSR access unless the > >>> handler is setup. Ideally we should document this behaviour somewhere. > >>> > >>> Maybe we could add a rdmsr_safe to your path akin to what's done > >>> here? > >> > >> Probably. Would need trying out on the affected older version, > >> just like ... > >> > > --- a/docs/man/xl.cfg.5.pod.in > > +++ b/docs/man/xl.cfg.5.pod.in > > @@ -2861,6 +2861,17 @@ No MCA capabilities in above list are enabled. > > > > =back > > > > +=item B > > + > > +Select whether to use the legacy behaviour for accesses to MSRs not > > explicitly > > +handled by Xen instead of injecting a #GP to the guest. Such legacy > > access > > +mode will force Xen to replicate the behaviour from the hardware it's > > currently > > +running on in order to decide whether a #GP is injected to the guest. > > Note > > +that the guest is never allowed access to unhandled MSRs, this option > > only > > +changes whether a #GP might be injected or not. > > This description is appropriate for reads, but not for writes: > Whether a write would fault can only be known by trying a write, > yet for safety reasons we can't risk doing more than a read. An > option might be to make writes fault is the to be written value > differs from that which the probing read has returned (i.e. the > same condition which originally had caused a log message to appear > in 4.14 for PV guests). > >>> > >>> Read values for unhandled MSRs will always be 0 with the proposed > >>> code, so we would inject a #GP to the guest for any write attempt to > >>> unhandled MSRs with a value different than 0. > >>> > >>> Maybe we should just inject a #GP to the guest for any attempts to > >>> write to unhandled MSRs? > >> > >> ... doing this would need to. Iirc I did add the write side of the > >> handling in my patch just for symmetry. I'd prefer handling to be > >> symmetric, but I can see why we may not want it to be so. > > > > Kind of in the wrong context, but I will reply here because it's the > > last message related to the MSR stuff. > > > > Jan: would you be fine with modifying your path to not change the > > behaviour for writes (ie: always inject #GP to the guest for unhandled > > accesses), and then add a rdmsr_safe to the read path
Re: [PATCH for-4.15] tools/xenstored: liveupdate: Properly check long transaction
On 04.03.21 10:39, Julien Grall wrote:
On 04/03/2021 09:00, Jürgen Groß wrote:
On 03.03.21 18:05, Julien Grall wrote:
From: Julien Grall
As XenStored is single-threaded, conn->ta_start_time will always be
smaller than now. As we substract the latter from the former, it means
a transaction will never be considered long running.
Invert the two operands of the substraction in both lu_reject_reason()
and lu_check_allowed(). In addition to that, the former also needs to
check that conn->ta_start_time is not 0 (i.e the transaction is not
active).
Take the opportunity to document the return condition of
lu_check_allowed().
Fixes: e04e53a5be20 ("tools/xenstore: allow live update only with no
transaction active")
Reported-by: Bjoern Doebel
Signed-off-by: Julien Grall
Reviewed-by: Juergen Gross
---
I am a bit puzzled on how -F is implemented. From my understanding we
will force LiveUpdate when one of the following conditions is met:
1) All the active transactions are long running
2) If we didn't manage to LiveUpdate after N sec
It is not quite clear why we need the both as 2) would indirectly cover
1). However 2) is probably unsafe as we may reset transactions for
"well-behaving" guest.
So I am thinking to send a patch to drop 2). Any opinions?
This will enable two guests working together to block LU by using
overlapping transactions:
Guest 1: - - - ...
Guest 2: -- - - --- ... >
There is always a transaction active, but none is regarded to be
long running.
Right, how do you know whether two guests are working together?
We can't know that. And this is the reason why you have to use the -F
option to force a LU.
I understand that "-F" means that things could break... However, I am
not entirely sure who can realistically use this option without risking
breaking other guests. For instance, there might be a 3rd guest that has
an active transaction and not cooperating with the first two.
Yes. OTOH the chances are rather low that multiple LU attempts are
failing due to transactions being active all the time.
Rather than forcing in this case, how about we quiesce the connection if
it starts a transaction when LiveUpdate is pending?
Yes, this should work, too.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH for-4.15] tools/xenstored: liveupdate: Properly check long transaction
On 04/03/2021 09:00, Jürgen Groß wrote:
On 03.03.21 18:05, Julien Grall wrote:
From: Julien Grall
As XenStored is single-threaded, conn->ta_start_time will always be
smaller than now. As we substract the latter from the former, it means
a transaction will never be considered long running.
Invert the two operands of the substraction in both lu_reject_reason()
and lu_check_allowed(). In addition to that, the former also needs to
check that conn->ta_start_time is not 0 (i.e the transaction is not
active).
Take the opportunity to document the return condition of
lu_check_allowed().
Fixes: e04e53a5be20 ("tools/xenstore: allow live update only with no
transaction active")
Reported-by: Bjoern Doebel
Signed-off-by: Julien Grall
Reviewed-by: Juergen Gross
---
I am a bit puzzled on how -F is implemented. From my understanding we
will force LiveUpdate when one of the following conditions is met:
1) All the active transactions are long running
2) If we didn't manage to LiveUpdate after N sec
It is not quite clear why we need the both as 2) would indirectly cover
1). However 2) is probably unsafe as we may reset transactions for
"well-behaving" guest.
So I am thinking to send a patch to drop 2). Any opinions?
This will enable two guests working together to block LU by using
overlapping transactions:
Guest 1: - - - ...
Guest 2: -- - - --- ... >
There is always a transaction active, but none is regarded to be
long running.
Right, how do you know whether two guests are working together?
I understand that "-F" means that things could break... However, I am
not entirely sure who can realistically use this option without risking
breaking other guests. For instance, there might be a 3rd guest that has
an active transaction and not cooperating with the first two.
Rather than forcing in this case, how about we quiesce the connection if
it starts a transaction when LiveUpdate is pending?
Cheers,
--
Julien Grall
Re: [PATCH for-4.15] tools/xenstored: liveupdate: Properly check long transaction
On 03.03.21 18:05, Julien Grall wrote:
From: Julien Grall
As XenStored is single-threaded, conn->ta_start_time will always be
smaller than now. As we substract the latter from the former, it means
a transaction will never be considered long running.
Invert the two operands of the substraction in both lu_reject_reason()
and lu_check_allowed(). In addition to that, the former also needs to
check that conn->ta_start_time is not 0 (i.e the transaction is not
active).
Take the opportunity to document the return condition of
lu_check_allowed().
Fixes: e04e53a5be20 ("tools/xenstore: allow live update only with no transaction
active")
Reported-by: Bjoern Doebel
Signed-off-by: Julien Grall
Reviewed-by: Juergen Gross
---
I am a bit puzzled on how -F is implemented. From my understanding we
will force LiveUpdate when one of the following conditions is met:
1) All the active transactions are long running
2) If we didn't manage to LiveUpdate after N sec
It is not quite clear why we need the both as 2) would indirectly cover
1). However 2) is probably unsafe as we may reset transactions for
"well-behaving" guest.
So I am thinking to send a patch to drop 2). Any opinions?
This will enable two guests working together to block LU by using
overlapping transactions:
Guest 1: - - - ...
Guest 2: -- - - --- ...
There is always a transaction active, but none is regarded to be
long running.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH RFC for-4.15] x86/msr: introduce an option for legacy MSR behavior selection
On 03.03.2021 16:38, Roger Pau Monné wrote: > On Tue, Mar 02, 2021 at 04:18:59PM +0100, Jan Beulich wrote: >> On 02.03.2021 16:00, Roger Pau Monné wrote: >>> On Tue, Mar 02, 2021 at 12:16:12PM +0100, Jan Beulich wrote: On 01.03.2021 17:23, Roger Pau Monne wrote: > RFC because there's still some debate as to how we should solve the > MSR issue, this is one possible way, but IMO we need to make a > decision soon-ish because of the release timeline. Generally I think it would be far better from a user pov if things worked out of the box, at least in cases where it can be made work. Arguably for Solaris this isn't going to be possible (leaving aside the non-option of fully returning back to original behavior), but for the old-Linux-PV case I still think my proposed change is better in this regard. I realize Andrew said (on irc) that this is making the behavior even weaker. I take a different perspective: Considering a guest will install exception handlers at some point, I continue to fail to see what good it will do to try to inject a #GP(0) when we know the guest will die because of not having a handler registered just yet. It is a kernel flaw, yes, but they ended up living with it merely because of our odd prior behavior, so we can't put all the blame on them. >>> >>> My concern with this would mostly be with newer guests, in the sense >>> that people could come to rely on this behavior of not injecting a >>> #GP if the handler is not setup, which I think we should try to avoid. >>> >>> One option would be to introduce a feature that could be used in the >>> elfnotes to signal that the kernel doesn't require such workarounds >>> for MSR #GP handling, maybe: >>> >>> /* >>> * Signal that the kernel doesn't require suppressing the #GP on >>> * unknown MSR accesses if the handler is not setup. New kernels >>> * should always have this set. >>> */ >>> #define XENFEAT_msr_early_gp 16 >>> >>> We could try to backport this on the Linux kernel as far as we can >>> that we know it's safe to do so. >> >> I too did consider something like this. While I'm not opposed, it >> effectively requires well-behaved consumers who haven't been well- >> behaved in the past. >> >>> If this is not acceptable then I guess your solution is fine. Arguably >>> PV has it's own (weird) architecture, in which it might be safe to say >>> that no #GP will be injected as a result of a MSR access unless the >>> handler is setup. Ideally we should document this behaviour somewhere. >>> >>> Maybe we could add a rdmsr_safe to your path akin to what's done >>> here? >> >> Probably. Would need trying out on the affected older version, >> just like ... >> > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -2861,6 +2861,17 @@ No MCA capabilities in above list are enabled. > > =back > > +=item B > + > +Select whether to use the legacy behaviour for accesses to MSRs not > explicitly > +handled by Xen instead of injecting a #GP to the guest. Such legacy > access > +mode will force Xen to replicate the behaviour from the hardware it's > currently > +running on in order to decide whether a #GP is injected to the guest. > Note > +that the guest is never allowed access to unhandled MSRs, this option > only > +changes whether a #GP might be injected or not. This description is appropriate for reads, but not for writes: Whether a write would fault can only be known by trying a write, yet for safety reasons we can't risk doing more than a read. An option might be to make writes fault is the to be written value differs from that which the probing read has returned (i.e. the same condition which originally had caused a log message to appear in 4.14 for PV guests). >>> >>> Read values for unhandled MSRs will always be 0 with the proposed >>> code, so we would inject a #GP to the guest for any write attempt to >>> unhandled MSRs with a value different than 0. >>> >>> Maybe we should just inject a #GP to the guest for any attempts to >>> write to unhandled MSRs? >> >> ... doing this would need to. Iirc I did add the write side of the >> handling in my patch just for symmetry. I'd prefer handling to be >> symmetric, but I can see why we may not want it to be so. > > Kind of in the wrong context, but I will reply here because it's the > last message related to the MSR stuff. > > Jan: would you be fine with modifying your path to not change the > behaviour for writes (ie: always inject #GP to the guest for unhandled > accesses), and then add a rdmsr_safe to the read path in order to > decide whether to inject a #GP to the guest even if the #GP handler is > not setup? I don't mind as long as it ends up making things work (I have no reason to believe it won't). I'll give that a try. > I can modify the option introduced on this
[xen-unstable test] 159820: tolerable FAIL - PUSHED
flight 159820 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/159820/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 159814 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159814 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 159814 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159814 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159814 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159814 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 159814 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 159814 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 159814 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 159814 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 159814 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159814 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen 243036df0d55673de59c214e240b9b914d278b65 baseline version: xen 4834936549f788378918da8e9bc97df7dd3ee16d Last test of basis 159814 2021-03-03 08:51:49 Z0 days Testing same since 159820 2021-03-03 22:07:38 Z0 days1 attempts People who touched revisions under test: Michael Kurth Norbert Manthey jobs: build-amd64-xsm pass build-arm64-xsm
Re: dom0less boot two compressed kernel images out-of-memory work-around
On 03.03.2021 20:36, Julien Grall wrote:
> (BCCing xen-users, CCing xen-devel + a few folks)
>
> Hi,
>
> Moving the discussion to xen-devel.
>
> On 22/02/2021 05:02, Charles Chiou wrote:
>> When trying to boot two zImage using dom0less boot on ARM, we encountered
>> this problem when xen runs gunzip on second guest:
>>
>> (XEN)
>> (XEN) Panic on CPU 0:
>> (XEN) Out of memory
>> (XEN)
>>
>> And worked around it with the following patch. We'd like to check to see if
>> this is a known issue and if the work-around looks reasonable. Thank you
>
> I haven't seen any similar report in the past.
>
>>
>>
>> diff --git a/xen/common/gunzip.c b/xen/common/gunzip.c
>> index db4efcd34b..e5bd19ba7f 100644
>> --- a/xen/common/gunzip.c
>> +++ b/xen/common/gunzip.c
>> @@ -113,8 +113,10 @@ __init int perform_gunzip(char *output, char *image,
>> unsigned long image_len)
>>
>> window = (unsigned char *)output;
>>
>> +if (!free_mem_ptr) {
>> free_mem_ptr = (unsigned long)alloc_xenheap_pages(HEAPORDER, 0);
>> free_mem_end_ptr = free_mem_ptr + (PAGE_SIZE << HEAPORDER);
>> +}
>>
>> inbuf = (unsigned char *)image;
>> insize = image_len;
>> @@ -131,7 +133,12 @@ __init int perform_gunzip(char *output, char *image,
>> unsigned long image_len)
>> rc = 0;
>> }
>>
>> +if (free_mem_ptr) {
>> free_xenheap_pages((void *)free_mem_ptr, HEAPORDER);
>> +free_mem_ptr = 0;
>> +}
>> +
>> +bytes_out = 0;
>>
>> return rc;
>> }
>> diff --git a/xen/common/inflate.c b/xen/common/inflate.c
>> index f99c985d61..de96002188 100644
>> --- a/xen/common/inflate.c
>> +++ b/xen/common/inflate.c
>> @@ -244,7 +244,7 @@ static void *INIT malloc(int size)
>>
>> if (size < 0)
>> error("Malloc error");
>> -if (!malloc_ptr)
>> +if ((!malloc_ptr) || (!malloc_count))
>> malloc_ptr = free_mem_ptr;
>>
>
> IMHO, this is a bit risky to assume that malloc_count will always be 0
> after each gunzip.
>
> Instead I think, it would be better if we re-initialize the allocator
> every time.
I agree.
> How about the following (untested):
>
> commit e1cd2d85234c8d0aa62ad32c824a5568a57be930 (HEAD -> dev)
> Author: Julien Grall
> Date: Wed Mar 3 19:27:56 2021 +
>
> xen/gunzip: Allow perform_gunzip() to be called multiple times
>
> Currently perform_gunzip() can only be called once because the the
> internal allocator is not fully re-initialized.
>
> This works fine if you are only booting dom0. But this will break when
> booting multiple using the dom0less that uses compressed kernel images.
>
> This can be resolved by re-initializing malloc_ptr and malloc_count
> every time perform_gunzip() is called.
>
> Note the latter is only re-initialized for hardening purpose as
> there is
> no guarantee that every malloc() are followed by free() (It should in
> theory!).
>
> Take the opportunity to check the return of alloc_heap_pages() to
> return
> an error rather than dereferencing a NULL pointer later on failure.
>
> Reported-by: Charles Chiou
> Signed-off-by: Julien Grall
>
> ---
>
> This patch is candidate for Xen 4.15. Without this patch, it will
> not be
> possible to boot multiple domain using dom0less when they are using
> compressed kernel images.
Other decompression methods are unaffected?
> --- a/xen/common/gunzip.c
> +++ b/xen/common/gunzip.c
> @@ -114,7 +114,11 @@ __init int perform_gunzip(char *output, char
> *image, unsigned long image_len)
> window = (unsigned char *)output;
>
> free_mem_ptr = (unsigned long)alloc_xenheap_pages(HEAPORDER, 0);
> +if ( !free_mem_ptr )
> +return -ENOMEM;
> +
> free_mem_end_ptr = free_mem_ptr + (PAGE_SIZE << HEAPORDER);
> +init_allocator();
>
> inbuf = (unsigned char *)image;
> insize = image_len;
> diff --git a/xen/common/inflate.c b/xen/common/inflate.c
> index f99c985d6135..d8c28a3e9593 100644
> --- a/xen/common/inflate.c
> +++ b/xen/common/inflate.c
> @@ -238,6 +238,12 @@ STATIC const ush mask_bits[] = {
> static unsigned long INITDATA malloc_ptr;
> static int INITDATA malloc_count;
>
> +static void init_allocator(void)
Please add INIT here. (I wouldn't mind if you used __init instead,
as there's going to be file-wide replacement after 4.15 anyway,
but of course this would render things inconsistent until then.)
Jan
> +{
> +malloc_ptr = free_mem_ptr;
> +malloc_count = 0;
> +}
> +
> static void *INIT malloc(int size)
> {
> void *p;
>
> Best regards,
>
Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11
On 03.03.2021 20:09, Julien Grall wrote:
> On 01/03/2021 07:57, Jan Beulich wrote:
>> The upcoming release complains, not entirely unreasonably:
>>
>> In file included from rijndael.c:33:
>> .../xen/include/crypto/rijndael.h:55:53: note: previously declared as 'const
>> unsigned char[]'
>> 55 | voidrijndaelEncrypt(const unsigned int [], int, const unsigned
>> char [],
>>|
>> ^~
>> rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned
>> char[16]'} with mismatched bound [-Werror=array-parameter=]
>>865 | u8 ct[16])
>>| ~~~^~
>> In file included from rijndael.c:33:
>> .../xen/include/crypto/rijndael.h:56:13: note: previously declared as
>> 'unsigned char[]'
>> 56 | unsigned char []);
>>| ^~~~
>>
>> While it's not really clear to me why it would complain only for arg 4,
>> the adjustment to make is obvious and riskfree also for arg 3: Simply
>> declare the correct array dimension right away. This then allows
>> compilers to apply checking at call sites, which seems desirable anyway.
>
> I am a bit confused, if GCC is not complaining for arg3, then what is
> the following error message for:
>
> > In file included from rijndael.c:33:
> > .../xen/include/crypto/rijndael.h:55:53: note: previously declared as
> 'const unsigned char[]'
> > 55 | voidrijndaelEncrypt(const unsigned int [], int, const
> unsigned char [],
> >|
> ^~
Hmm, good point. I didn't observe this myself, and simply copied the
part of the error message that I was handed. I suppose there was an
"error: argument 3 of type ..." there then as well. Charles - any
chance you could confirm this, and perhaps even quote the full set
of error messages in our internal patch?
I'll adjust the wording of the description in any event.
>> There are quite a few more issues with gcc11, but from my brief initial
>> inspection I'm suspecting (hoping) it'll rather be the compiler which
>> will get further changed by the time their release gets finalized. Just
>> one example:
>>
>> .../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified
>> bound 4 exceeds source size 0 [-Werror=stringop-overread]
>>101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
>>| ^~~
>> mpparse.c:722:13: note: in expansion of macro 'memcmp'
>>722 | if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
>>| ^~
>>
>> Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
>
> It looks like there is a report on the redhat bug tracker for it [1]. Do
> you know if there is a bug report on the GCC tracker as well?
I have no idea, to be honest.
Jan
