[ovmf test] 177239: all pass - PUSHED
flight 177239 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/177239/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 540522fec06b87bf11ad5624abe23b515f282d60 baseline version: ovmf b3f321f2d7871868951cf73edb8fa4d5a88854a5 Last test of basis 177223 2023-02-14 00:11:46 Z0 days Testing same since 177239 2023-02-14 03:48:16 Z0 days1 attempts People who touched revisions under test: Michael Kubacki jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 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/osstest/ovmf.git b3f321f2d7..540522fec0 540522fec06b87bf11ad5624abe23b515f282d60 -> xen-tested-master
[PATCH 2/2] xen/misra: add entries to exclude-list.json
Add entries to the exclude-list.json for those files that need to be excluded from the analysis scan. Signed-off-by: Luca Fancellu Signed-off-by: Michal Orzel --- This list is originated from Michal's work here: https://patchwork.kernel.org/project/xen-devel/patch/20221116092032.4423-1-michal.or...@amd.com/#25123099 the only files removed are the *arm/smmu* that, if I understand correctly, we would like to include in the scan. --- --- docs/misra/exclude-list.json | 207 ++- 1 file changed, 206 insertions(+), 1 deletion(-) diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json index 1fb0ac67747b..3a6dc0809af5 100644 --- a/docs/misra/exclude-list.json +++ b/docs/misra/exclude-list.json @@ -1,4 +1,209 @@ { "version": "1.0", -"content": [] +"content": [ +{ +"rel_path": "arch/arm/arm64/cpufeature.c" +}, +{ +"rel_path": "arch/arm/arm64/insn.c" +}, +{ +"rel_path": "arch/arm/arm64/lib/find_next_bit.c" +}, +{ +"rel_path": "arch/x86/acpi/boot.c" +}, +{ +"rel_path": "arch/x86/acpi/cpu_idle.c" +}, +{ +"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c" +}, +{ +"rel_path": "arch/x86/acpi/cpuidle_menu.c" +}, +{ +"rel_path": "arch/x86/acpi/lib.c" +}, +{ +"rel_path": "arch/x86/acpi/power.c" +}, +{ +"rel_path": "arch/x86/cpu/amd.c" +}, +{ +"rel_path": "arch/x86/cpu/centaur.c" +}, +{ +"rel_path": "arch/x86/cpu/common.c" +}, +{ +"rel_path": "arch/x86/cpu/hygon.c" +}, +{ +"rel_path": "arch/x86/cpu/intel.c" +}, +{ +"rel_path": "arch/x86/cpu/intel_cacheinfo.c" +}, +{ +"rel_path": "arch/x86/cpu/mcheck/mce-apei.c" +}, +{ +"rel_path": "arch/x86/cpu/mcheck/non-fatal.c" +}, +{ +"rel_path": "arch/x86/cpu/mtrr/*" +}, +{ +"rel_path": "arch/x86/cpu/mwait-idle.c" +}, +{ +"rel_path": "arch/x86/delay.c" +}, +{ +"rel_path": "arch/x86/dmi_scan.c" +}, +{ +"rel_path": "arch/x86/domain.c" +}, +{ +"rel_path": "arch/x86/genapic/*" +}, +{ +"rel_path": "arch/x86/i387.c" +}, +{ +"rel_path": "arch/x86/irq.c" +}, +{ +"rel_path": "arch/x86/mpparse.c" +}, +{ +"rel_path": "arch/x86/srat.c" +}, +{ +"rel_path": "arch/x86/time.c" +}, +{ +"rel_path": "arch/x86/traps.c" +}, +{ +"rel_path": "arch/x86/usercopy.c" +}, +{ +"rel_path": "arch/x86/x86_64/mmconf-fam10h.c" +}, +{ +"rel_path": "common/bitmap.c" +}, +{ +"rel_path": "common/bunzip2.c" +}, +{ +"rel_path": "common/cpu.c" +}, +{ +"rel_path": "common/earlycpio.c" +}, +{ +"rel_path": "common/inflate.c" +}, +{ +"rel_path": "common/libfdt/*" +}, +{ +"rel_path": "common/lz4/decompress.c" +}, +{ +"rel_path": "common/notifier.c" +}, +{ +"rel_path": "common/radix-tree.c" +}, +{ +"rel_path": "common/rcupdate.c" +}, +{ +"rel_path": "common/softirq.c" +}, +{ +"rel_path": "common/tasklet.c" +}, +{ +"rel_path": "common/ubsan/ubsan.c" +}, +{ +"rel_path": "common/un*.c" +}, +{ +"rel_path": "common/vsprintf.c" +}, +{ +"rel_path": "common/xz/*" +}, +{ +"rel_path": "common/zstd/*" +}, +{ +"rel_path": "crypto/rijndael.c" +}, +{ +"rel_path": "crypto/vmac.c" +}, +{ +"rel_path": "drivers/acpi/apei/*" +}, +{ +"rel_path": "drivers/acpi/hwregs.c" +}, +{ +"rel_path": "drivers/acpi/numa.c" +}, +{ +"rel_path": "drivers/acpi/osl.c" +}, +{ +"rel_path": "drivers/acpi/reboot.c" +}, +{ +"rel_path": "drivers/acpi/tables.c" +}, +{ +"rel_path": "drivers/acpi/tables/*" +}, +{ +"rel_path": "drivers/acpi/utilities/*" +}, +{ +"rel_path": "drivers/char/ehci-dbgp.c" +
[PATCH 1/2] xen/cppcheck: add a way to exclude files from the scan
Add a way to exclude files from the scan, in this way we can skip some findings from the report on those files that Xen doesn't own. To do that, introduce the exclude-list.json file under docs/misra, this file will be populated with relative path to the files/folder to be excluded. Introduce a new module, exclusion_file_list.py, to deal with the exclusion list file and use the new module in cppcheck_analysis.py to take a list of excluded paths to update the suppression list of cppcheck. Modified --suppress flag for cppcheck tool to remove unmatchedSuppression findings for those external file that are listed but for example not built for the current architecture. Add documentation for the file. Signed-off-by: Luca Fancellu --- docs/misra/exclude-list.json | 4 + docs/misra/exclude-list.rst | 44 +++ xen/scripts/xen_analysis/cppcheck_analysis.py | 21 - .../xen_analysis/exclusion_file_list.py | 79 +++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 docs/misra/exclude-list.json create mode 100644 docs/misra/exclude-list.rst create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json new file mode 100644 index ..1fb0ac67747b --- /dev/null +++ b/docs/misra/exclude-list.json @@ -0,0 +1,4 @@ +{ +"version": "1.0", +"content": [] +} diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst new file mode 100644 index ..969539c46beb --- /dev/null +++ b/docs/misra/exclude-list.rst @@ -0,0 +1,44 @@ +.. SPDX-License-Identifier: CC-BY-4.0 + +Exclude file list for xen-analysis script += + +The code analysis is performed on the Xen codebase for both MISRA checkers and +static analysis checkers, there are some files however that needs to be removed +from the findings report because they are not owned by Xen and they must be kept +in sync with their origin (completely or even partially), hence we can't easily +fix findings or deviate from them. + +For this reason the file docs/misra/exclude-list.json is used to exclude every +entry listed in that file from the final report. +Currently only the cppcheck analysis will use this file. + +Here is an example of the exclude-list.json file:: + +|{ +|"version": "1.0", +|"content": [ +|{ +|"rel_path": "relative/path/from/xen/file", +|"comment": "This file is originated from ..." +|}, +|{ +|"rel_path": "relative/path/from/xen/folder/*", +|"comment": "This folder is a library" +|}, +|{ +|"rel_path": "relative/path/from/xen/mem*.c", +|"comment": "memcpy.c, memory.c and memcmp.c are from the outside" +|} +|] +|} + +Here is an explanation of the fields inside an object of the "content" array: + - rel_path: it is the relative path from the Xen folder to the file/folder that + needs to be excluded from the analysis report, it can contain a wildcard to + match more than one file/folder at the time. This field is mandatory. + - comment: an optional comment to explain why the file is removed from the + analysis. + +To ease the review and the modifications of the entries, they shall be listed in +alphabetical order referring to the rel_path field. diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py index cc1f403d315e..b681dca90836 100644 --- a/xen/scripts/xen_analysis/cppcheck_analysis.py +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py @@ -1,7 +1,8 @@ #!/usr/bin/env python3 import os, re, shutil -from . import settings, utils, cppcheck_report_utils +from . import settings, utils, cppcheck_report_utils, exclusion_file_list +from .exclusion_file_list import ExclusionFileListError class GetMakeVarsPhaseError(Exception): pass @@ -50,6 +51,22 @@ def __generate_suppression_list(out_file): # header for cppcheck supplist_file.write("*:*generated/compiler-def.h\n") +try: +exclusion_file = \ +"{}/docs/misra/exclude-list.json".format(settings.repo_dir) +exclusion_list = \ + exclusion_file_list.load_exclusion_file_list(exclusion_file) +except ExclusionFileListError as e: +raise CppcheckDepsPhaseError( +"Issue with reading file {}: {}" +.format(exclusion_file, e) + ) + +# Append excluded files to the suppression list, using * as error id +# to suppress every findings on that file +for path in exclusion_list: +supplist_file.write("*:{}\n".format(path)) + for entry in headers: filename = entry["file"] try: @@ -128,7
[PATCH 0/2] xen/misra: create exclusion file list
This serie is introducing an exclusion list for the misra analysis, at the moment only cppcheck can benefit from it because it's the tool where we control every step and configuration. Exclude a file from the analysis is the last step we should do, but sometime it is unavoidable because we can't do direct changes to it to fix/deviate from findings. So we declare the file(s) here and we leave the burden of the misra compliance to the final user. Luca Fancellu (2): xen/cppcheck: add a way to exclude files from the scan xen/misra: add entries to exclude-list.json docs/misra/exclude-list.json | 209 ++ docs/misra/exclude-list.rst | 44 xen/scripts/xen_analysis/cppcheck_analysis.py | 21 +- .../xen_analysis/exclusion_file_list.py | 79 +++ 4 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 docs/misra/exclude-list.json create mode 100644 docs/misra/exclude-list.rst create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py -- 2.25.1
Re: [PATCH 1/2] xen/cppcheck: add a way to exclude files from the scan
On 14.02.2023 09:56, Luca Fancellu wrote: > Add a way to exclude files from the scan, in this way we can skip > some findings from the report on those files that Xen doesn't own. > > To do that, introduce the exclude-list.json file under docs/misra, > this file will be populated with relative path to the files/folder > to be excluded. > Introduce a new module, exclusion_file_list.py, to deal with the > exclusion list file and use the new module in cppcheck_analysis.py > to take a list of excluded paths to update the suppression list of > cppcheck. > Modified --suppress flag for cppcheck tool to remove > unmatchedSuppression findings for those external file that are > listed but for example not built for the current architecture. > > Add documentation for the file. > > Signed-off-by: Luca Fancellu > --- > docs/misra/exclude-list.json | 4 + > docs/misra/exclude-list.rst | 44 +++ > xen/scripts/xen_analysis/cppcheck_analysis.py | 21 - > .../xen_analysis/exclusion_file_list.py | 79 +++ > 4 files changed, 146 insertions(+), 2 deletions(-) > create mode 100644 docs/misra/exclude-list.json > create mode 100644 docs/misra/exclude-list.rst > create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py As I think I have asked for before: Can new files please avoid underscores in their names, when dashes do quite fine? Or does Python have special restrictions? > --- a/xen/scripts/xen_analysis/cppcheck_analysis.py > +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py > @@ -1,7 +1,8 @@ > #!/usr/bin/env python3 > > import os, re, shutil > -from . import settings, utils, cppcheck_report_utils > +from . import settings, utils, cppcheck_report_utils, exclusion_file_list > +from .exclusion_file_list import ExclusionFileListError > > class GetMakeVarsPhaseError(Exception): > pass > @@ -50,6 +51,22 @@ def __generate_suppression_list(out_file): > # header for cppcheck > supplist_file.write("*:*generated/compiler-def.h\n") > > +try: > +exclusion_file = \ > + > "{}/docs/misra/exclude-list.json".format(settings.repo_dir) > +exclusion_list = \ > + > exclusion_file_list.load_exclusion_file_list(exclusion_file) > +except ExclusionFileListError as e: > +raise CppcheckDepsPhaseError( > +"Issue with reading file {}: {}" > +.format(exclusion_file, e) > + ) My Python isn't very good, but isn't indentation one level too deep here? Jan
Re: [PATCH 2/2] xen/misra: add entries to exclude-list.json
On 14.02.2023 09:56, Luca Fancellu wrote: > --- a/docs/misra/exclude-list.json > +++ b/docs/misra/exclude-list.json > @@ -1,4 +1,209 @@ > { > "version": "1.0", > -"content": [] > +"content": [ > +{ > +"rel_path": "arch/arm/arm64/cpufeature.c" > +}, > +{ > +"rel_path": "arch/arm/arm64/insn.c" > +}, > +{ > +"rel_path": "arch/arm/arm64/lib/find_next_bit.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/boot.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/cpu_idle.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/cpuidle_menu.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/lib.c" > +}, > +{ > +"rel_path": "arch/x86/acpi/power.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/amd.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/centaur.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/common.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/hygon.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/intel.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/intel_cacheinfo.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/mcheck/mce-apei.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/mcheck/non-fatal.c" > +}, > +{ > +"rel_path": "arch/x86/cpu/mtrr/*" > +}, > +{ > +"rel_path": "arch/x86/cpu/mwait-idle.c" > +}, > +{ > +"rel_path": "arch/x86/delay.c" > +}, > +{ > +"rel_path": "arch/x86/dmi_scan.c" > +}, > +{ > +"rel_path": "arch/x86/domain.c" > +}, > +{ > +"rel_path": "arch/x86/genapic/*" > +}, > +{ > +"rel_path": "arch/x86/i387.c" > +}, > +{ > +"rel_path": "arch/x86/irq.c" > +}, > +{ > +"rel_path": "arch/x86/mpparse.c" > +}, > +{ > +"rel_path": "arch/x86/srat.c" > +}, > +{ > +"rel_path": "arch/x86/time.c" > +}, > +{ > +"rel_path": "arch/x86/traps.c" > +}, > +{ > +"rel_path": "arch/x86/usercopy.c" > +}, > +{ > +"rel_path": "arch/x86/x86_64/mmconf-fam10h.c" > +}, > +{ > +"rel_path": "common/bitmap.c" > +}, > +{ > +"rel_path": "common/bunzip2.c" > +}, > +{ > +"rel_path": "common/cpu.c" > +}, > +{ > +"rel_path": "common/earlycpio.c" > +}, > +{ > +"rel_path": "common/inflate.c" > +}, > +{ > +"rel_path": "common/libfdt/*" > +}, > +{ > +"rel_path": "common/lz4/decompress.c" > +}, > +{ > +"rel_path": "common/notifier.c" > +}, > +{ > +"rel_path": "common/radix-tree.c" > +}, > +{ > +"rel_path": "common/rcupdate.c" > +}, > +{ > +"rel_path": "common/softirq.c" > +}, > +{ > +"rel_path": "common/tasklet.c" > +}, > +{ > +"rel_path": "common/ubsan/ubsan.c" > +}, > +{ > +"rel_path": "common/un*.c" > +}, > +{ > +"rel_path": "common/vsprintf.c" > +}, > +{ > +"rel_path": "common/xz/*" > +}, > +{ > +"rel_path": "common/zstd/*" > +}, > +{ > +"rel_path": "crypto/rijndael.c" > +}, > +{ > +"rel_path": "crypto/vmac.c" > +}, > +{ > +"rel_path": "drivers/acpi/apei/*" > +}, > +{ > +"rel_path": "drivers/acpi/hwregs.c" > +}, > +{ > +"rel_path": "drivers/acpi/numa.c" > +}, > +{ > +"rel_path": "drivers/acpi/osl.c" > +}, > +{ > +"rel_path": "drivers/acpi/reboot.c" > +}, > +{ > +"rel_path": "drivers/acpi/tables.c" > +}, > +{ > +"rel_path": "drivers/acpi/tables/*" > +}, > +{ > +"rel_path": "drivers/acpi/utilities/*" > +}, > +{ > +"rel_path": "drivers/char/ehci-dbgp.c" > +}, > +{ > +"rel_path": "drivers/char/xhci-dbc.c" > +}, > +{ > +"rel_path": "drivers/cpufreq/*" > +}, > +{ > +"rel_path": "drivers/video/font_*" > +}, > +{ > +"rel_
RE: [PATCH v1 01/13] xen/arm: re-arrange the static shared memory region
> -Original Message- > From: Stewart Hildebrand > Sent: Wednesday, February 8, 2023 4:55 AM > To: Penny Zheng ; xen-devel@lists.xenproject.org > Cc: Wei Chen ; Stefano Stabellini > ; Julien Grall ; Bertrand Marquis > ; Volodymyr Babchuk > > Subject: Re: [PATCH v1 01/13] xen/arm: re-arrange the static shared memory > region > > Hi Penny, > Hi, Stewart Sorry for the late response, got sidetracked by a few internal projects > On 11/14/22 21:52, Penny Zheng wrote: > > ... > > diff --git a/xen/arch/arm/include/asm/setup.h > > b/xen/arch/arm/include/asm/setup.h > > index fdbf68aadc..2d4ae0f00a 100644 > > --- a/xen/arch/arm/include/asm/setup.h > > +++ b/xen/arch/arm/include/asm/setup.h > > @@ -50,10 +50,6 @@ struct membank { > > paddr_t start; > > paddr_t size; > > enum membank_type type; > > -#ifdef CONFIG_STATIC_SHM > > -char shm_id[MAX_SHM_ID_LENGTH]; > > -unsigned int nr_shm_borrowers; > > -#endif > > }; > > > > struct meminfo { > > @@ -61,6 +57,17 @@ struct meminfo { > > struct membank bank[NR_MEM_BANKS]; }; > > > > +struct shm_membank { > > +char shm_id[MAX_SHM_ID_LENGTH]; > > +unsigned int nr_shm_borrowers; > > +struct membank *membank; > > +}; > > + > > +struct shm_meminfo { > > +unsigned int nr_banks; > > +struct shm_membank bank[NR_MEM_BANKS]; }; > > + > > /* > > * The domU flag is set for kernels and ramdisks of "xen,domain" nodes. > > * The purpose of the domU flag is to avoid getting confused in @@ > > -105,6 +112,7 @@ struct bootinfo { > > struct meminfo acpi; > > #endif > > bool static_heap; > > +struct shm_meminfo shm_mem; > > }; > > > > struct map_range_data > > The reduction in the sizeof struct membank is a welcome improvement, in > my opinion, because it is multiplied by NR_MEM_BANKS, and IIRC we only > have 32k of boot stack to play with. > > Before this patch: > sizeof(struct kernel_info): 20648 > sizeof(struct meminfo): 10248 > sizeof(struct shm_meminfo): 10248 > > When building with EXTRA_CFLAGS_XEN_CORE="Wstack-usage=4096 -Wno- > error=stack-usage=": Learnt! Thx! > arch/arm/domain_build.c: In function ‘construct_domU’: > arch/arm/domain_build.c:3747:19: warning: stack usage is 20720 bytes [- > Wstack-usage=] > 3747 | static int __init construct_domU(struct domain *d, > | ^~ > arch/arm/domain_build.c: In function ‘construct_dom0’: > arch/arm/domain_build.c:3979:19: warning: stack usage is 20688 bytes [- > Wstack-usage=] > 3979 | static int __init construct_dom0(struct domain *d) > | ^~ > > > > After this patch: > sizeof(struct kernel_info): 14504 > sizeof(struct meminfo): 6152 > sizeof(struct shm_meminfo): 8200 > > arch/arm/domain_build.c: In function ‘construct_domU’: > arch/arm/domain_build.c:3754:19: warning: stack usage is 14576 bytes [- > Wstack-usage=] > 3754 | static int __init construct_domU(struct domain *d, > | ^~ > arch/arm/domain_build.c: In function ‘construct_dom0’: > arch/arm/domain_build.c:3986:19: warning: stack usage is 14544 bytes [- > Wstack-usage=] > 3986 | static int __init construct_dom0(struct domain *d) > | ^~ > > A later patch in this series will increase it again slightly. Note that I'm > not > expecting this series to address these particular warnings, I'm merely > pointing out the (positive) impact of the change. I agreed that NR_MEM_BANKS could be a large multiplier, and if we make "struct shm_meminfo" like "struct meminfo", to have a array of NR_MEM_BANKS items, it will make Xen binary exceed 20MB... We have discussed it here[1]. However, I'm afraid that dynamic allocation is also not a preferred option here, where to free the data could be a problem. So in next serie, which will come very soon, I'll introduce: ``` #define NR_SHM_BANKS 16 /* * A static shared memory node could be banked with a statically * configured host memory bank, or a set of arbitrary host memory * banks allocated from heap. */ struct shm_meminfo { unsigned int nr_banks; struct membank bank[NR_SHM_BANKS]; paddr_t tot_size; }; ``` Taking your previous instructions, compiling with "EXTRA_CFLAGS_XEN_CORE="-Wstack-usage=4096 -Wno-error=stack-usage=", boot stack usage for "construct_domU" and " construct_dom0" will be like: " arch/arm/domain_build.c: In function ‘construct_domU’: arch/arm/domain_build.c:4127:19: warning: stack usage is 16800 bytes [-Wstack-usage=] 4127 | static int __init construct_domU(struct domain *d, | ^~ arch/arm/domain_build.c: In function ‘construct_dom0’: arch/arm/domain_build.c:4359:19: warning: stack usage is 16640 bytes [-Wstack-usage=] 4359 | static int __init construct_dom0(struct domain *d) | ^~ " [1] https://lists.xenproject.org/archives/html/xen-devel/2023-01/msg00449.html Cheers, --- Penny Zheng
Re: [PATCH 1/2] xen/cppcheck: add a way to exclude files from the scan
> On 14 Feb 2023, at 09:51, Jan Beulich wrote: > > On 14.02.2023 09:56, Luca Fancellu wrote: >> Add a way to exclude files from the scan, in this way we can skip >> some findings from the report on those files that Xen doesn't own. >> >> To do that, introduce the exclude-list.json file under docs/misra, >> this file will be populated with relative path to the files/folder >> to be excluded. >> Introduce a new module, exclusion_file_list.py, to deal with the >> exclusion list file and use the new module in cppcheck_analysis.py >> to take a list of excluded paths to update the suppression list of >> cppcheck. >> Modified --suppress flag for cppcheck tool to remove >> unmatchedSuppression findings for those external file that are >> listed but for example not built for the current architecture. >> >> Add documentation for the file. >> >> Signed-off-by: Luca Fancellu >> --- >> docs/misra/exclude-list.json | 4 + >> docs/misra/exclude-list.rst | 44 +++ >> xen/scripts/xen_analysis/cppcheck_analysis.py | 21 - >> .../xen_analysis/exclusion_file_list.py | 79 +++ >> 4 files changed, 146 insertions(+), 2 deletions(-) >> create mode 100644 docs/misra/exclude-list.json >> create mode 100644 docs/misra/exclude-list.rst >> create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py > > As I think I have asked for before: Can new files please avoid underscores > in their names, when dashes do quite fine? Or does Python have special > restrictions? From the python coding style: Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged. In this case I think it improves readability > >> --- a/xen/scripts/xen_analysis/cppcheck_analysis.py >> +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py >> @@ -1,7 +1,8 @@ >> #!/usr/bin/env python3 >> >> import os, re, shutil >> -from . import settings, utils, cppcheck_report_utils >> +from . import settings, utils, cppcheck_report_utils, exclusion_file_list >> +from .exclusion_file_list import ExclusionFileListError >> >> class GetMakeVarsPhaseError(Exception): >> pass >> @@ -50,6 +51,22 @@ def __generate_suppression_list(out_file): >> # header for cppcheck >> supplist_file.write("*:*generated/compiler-def.h\n") >> >> +try: >> +exclusion_file = \ >> + >> "{}/docs/misra/exclude-list.json".format(settings.repo_dir) >> +exclusion_list = \ >> + >> exclusion_file_list.load_exclusion_file_list(exclusion_file) >> +except ExclusionFileListError as e: >> +raise CppcheckDepsPhaseError( >> +"Issue with reading file {}: {}" >> +.format(exclusion_file, e) >> + ) > > My Python isn't very good, but isn't indentation one level too deep > here? Good catch, I’ll fix it > > Jan
Re: [PATCH] automation: Add container and build jobs to run cppcheck analysis
On 13/02/2023 2:23 pm, Michal Orzel wrote: > Add a debian container with cppcheck installation routine inside, > capable of performing cppcheck analysis on Xen-only build including > cross-builds for arm32 and arm64. > > Populate build jobs making use of that container to run cppcheck > analysis to produce a text report (xen-cppcheck.txt) containing the list > of all the findings. > > This patch does not aim at performing any sort of bisection. Cppcheck is > imperfect and for now, our goal is to at least be aware of its reports, > so that we can compare them with the ones produced by better tools and > to be able to see how these reports change as a result of further > infrastructure improvements (e.g. exception list, rules exclusion). > > Signed-off-by: Michal Orzel > --- > For those interested in, here is a sample pipeline: > https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/775769167 > --- > .../build/debian/unstable-cppcheck.dockerfile | 37 + > automation/gitlab-ci/build.yaml | 40 +++ > automation/scripts/build | 11 - I'm afraid that I'm going to start pushing back on any more x86 jobs. We're already at several hours to get a run out of Gitlab CI, and that's assuming none of them hit networking issues, and outside of the typical European working day, when patchew is busy churning and not reporting the status back. Right now, there is vastly more ARM test resource than x86 resource, as evidence by the fact that you're never waiting more than a few minutes for the actually-ARM tests to complete, so adding more x86 cross compiles is compounding the problem. We need less x86 testing, or more x86 resource. Probably both, because its now so long that even I'm not using it as a pre-push gate on all changes. ~Andrew
[xen-unstable test] 177229: tolerable trouble: fail/pass/starved - PUSHED
flight 177229 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/177229/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop fail blocked in 177171 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 177171 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 177171 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 177171 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 177171 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 177171 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 177171 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 177171 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 177171 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-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-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 9b70bc6d9678142a40e6c1c6934a32c7a0966e38 baseline version: xen f4f498d08d50259b9f25c274fd7b1e8ccf5693af Last test of basis 177171 2023-02-13 12:37:06 Z0 days Testing same since 177229 2023-02-14 01:42:49 Z0 days1 attempts People who touched revisions under test: Jan Beulich Xenia Ragiadakou jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pas
Re: [PATCH v1 2/4] livepatch-build: Allow a patch to introduce new subdirs
> From: Xen-devel on behalf of Mihails > Strasuns > Sent: Thursday, January 19, 2023 10:13 AM > To: xen-devel@lists.xenproject.org > Cc: mstra...@amazon.com ; Raphael Ning > ; Bjoern Doebel ; Martin Pohlack > > Subject: [PATCH v1 2/4] livepatch-build: Allow a patch to introduce new > subdirs > > From: Raphael Ning > > Fix a bug in create_patch() where cp, strip, etc. will fail if the new > object file introduced by the patch is located in a new subdirectory: > > DEBUG: cp: cannot create regular file `output/xen/common/lu/lu.o': No such > file or directory > DEBUG: strip: 'output/xen/common/lu/lu.o': No such file > > In this example, xen/common/lu/ does not exist in the original > (unpatched) Xen source tree. It needs to be created in output/ as well. > > Signed-off-by: Raphael Ning > Reviewed-by: Bjoern Doebel > Reviewed-by: Martin Pohlack > --- > livepatch-build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/livepatch-build b/livepatch-build > index f7d6471..444daa9 100755 > --- a/livepatch-build > +++ b/livepatch-build > @@ -232,6 +232,7 @@ function create_patch() > > NEW_FILES=$(comm -23 <(cd patched/xen && find . -type f -name '*.o' | > sort) <(cd original/xen && find . -type f -name '*.o' | sort)) > for i in $NEW_FILES; do > + mkdir -p "output/$(dirname "$i")" > cp "patched/$i" "output/$i" > [[ $STRIP -eq 1 ]] && strip --strip-unneeded "output/$i" > CHANGED=1 Reviewed-by: Ross Lagerwall
Re: [PATCH v1 3/4] livepatch-gcc: Ignore buildid.o
> From: Xen-devel on behalf of Mihails > Strasuns > Sent: Thursday, January 19, 2023 10:13 AM > To: xen-devel@lists.xenproject.org > Cc: mstra...@amazon.com ; Raphael Ning > ; Bjoern Doebel ; Martin Pohlack > > Subject: [PATCH v1 3/4] livepatch-gcc: Ignore buildid.o > > From: Raphael Ning > > Not all .o files generated by the Xen build need to be passed to > create-diff-object for analysis. The latest example is: > > Run create-diff-object on xen/arch/x86/efi/buildid.o > Open base > /usr/libexec/livepatch-build-tools/create-diff-object: ERROR: buildid.o: > kpatch_create_section_list: 77: elf_getshdrnum > > This file is special, as it does not contain any sections. It is > generated by objcopy from a magic string of bytes (see Xen commit > eee5909e9d1e x86/EFI: use less crude a way of generating the build ID), > which probably will never change. Therefore, livepatch-gcc should not > copy it to the output directory. > > Signed-off-by: Raphael Ning > Reviewed-by: Bjoern Doebel > Reviewed-by: Martin Pohlack > Reviewed-by: Ross Lagerwall
Re: [PATCH] automation: Add container and build jobs to run cppcheck analysis
Hi Andrew, On 14/02/2023 12:00, Andrew Cooper wrote: > > > On 13/02/2023 2:23 pm, Michal Orzel wrote: >> Add a debian container with cppcheck installation routine inside, >> capable of performing cppcheck analysis on Xen-only build including >> cross-builds for arm32 and arm64. >> >> Populate build jobs making use of that container to run cppcheck >> analysis to produce a text report (xen-cppcheck.txt) containing the list >> of all the findings. >> >> This patch does not aim at performing any sort of bisection. Cppcheck is >> imperfect and for now, our goal is to at least be aware of its reports, >> so that we can compare them with the ones produced by better tools and >> to be able to see how these reports change as a result of further >> infrastructure improvements (e.g. exception list, rules exclusion). >> >> Signed-off-by: Michal Orzel >> --- >> For those interested in, here is a sample pipeline: >> https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/775769167 >> --- >> .../build/debian/unstable-cppcheck.dockerfile | 37 + >> automation/gitlab-ci/build.yaml | 40 +++ >> automation/scripts/build | 11 - > > I'm afraid that I'm going to start pushing back on any more x86 jobs. > > We're already at several hours to get a run out of Gitlab CI, and that's > assuming none of them hit networking issues, and outside of the typical > European working day, when patchew is busy churning and not reporting > the status back. > > Right now, there is vastly more ARM test resource than x86 resource, as > evidence by the fact that you're never waiting more than a few minutes > for the actually-ARM tests to complete, so adding more x86 cross > compiles is compounding the problem. > > We need less x86 testing, or more x86 resource. Probably both, because > its now so long that even I'm not using it as a pre-push gate on all > changes. I'm aware of the problem you described. AFAICT there is nothing stopping us from switching completely the arm32 cross builds from x86 to arm64 container. It is just a matter of creating identical container to unstable-arm32-gcc e.g. unstable-arm64v8-arm32-gcc and using FROM arm64v8/debian:unstable. We need to keep the old container for backwards compatibility. This way, x86 runners will only do x86 stuff + riscv64. Are you aware of anything preventing us to do so? If not, I will push a prereq patch to switch the arm32 cross build to arm64. > > ~Andrew ~Michal
Re: [PATCH] automation: Add container and build jobs to run cppcheck analysis
On 14/02/2023 11:45 am, Michal Orzel wrote: > Hi Andrew, > > On 14/02/2023 12:00, Andrew Cooper wrote: >> >> On 13/02/2023 2:23 pm, Michal Orzel wrote: >>> Add a debian container with cppcheck installation routine inside, >>> capable of performing cppcheck analysis on Xen-only build including >>> cross-builds for arm32 and arm64. >>> >>> Populate build jobs making use of that container to run cppcheck >>> analysis to produce a text report (xen-cppcheck.txt) containing the list >>> of all the findings. >>> >>> This patch does not aim at performing any sort of bisection. Cppcheck is >>> imperfect and for now, our goal is to at least be aware of its reports, >>> so that we can compare them with the ones produced by better tools and >>> to be able to see how these reports change as a result of further >>> infrastructure improvements (e.g. exception list, rules exclusion). >>> >>> Signed-off-by: Michal Orzel >>> --- >>> For those interested in, here is a sample pipeline: >>> https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/775769167 >>> --- >>> .../build/debian/unstable-cppcheck.dockerfile | 37 + >>> automation/gitlab-ci/build.yaml | 40 +++ >>> automation/scripts/build | 11 - >> I'm afraid that I'm going to start pushing back on any more x86 jobs. >> >> We're already at several hours to get a run out of Gitlab CI, and that's >> assuming none of them hit networking issues, and outside of the typical >> European working day, when patchew is busy churning and not reporting >> the status back. >> >> Right now, there is vastly more ARM test resource than x86 resource, as >> evidence by the fact that you're never waiting more than a few minutes >> for the actually-ARM tests to complete, so adding more x86 cross >> compiles is compounding the problem. >> >> We need less x86 testing, or more x86 resource. Probably both, because >> its now so long that even I'm not using it as a pre-push gate on all >> changes. > I'm aware of the problem you described. AFAICT there is nothing stopping us > from switching completely the arm32 cross builds from x86 to arm64 container. > It is just a matter of creating identical container to unstable-arm32-gcc > e.g. unstable-arm64v8-arm32-gcc and using FROM arm64v8/debian:unstable. > We need to keep the old container for backwards compatibility. > > This way, x86 runners will only do x86 stuff + riscv64. > > Are you aware of anything preventing us to do so? > If not, I will push a prereq patch to switch the arm32 cross build to arm64. No issues that I can see - I think that would be a good move in the short term. And it's also something that should be backported to alleviate pressure there. On the x86 side, we also desperately need to prune some legacy things. Guess I'll get around to that is some copious free never. ~Andrew
[libvirt test] 177242: tolerable trouble: pass/starved - PUSHED
flight 177242 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/177242/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 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-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 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-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: libvirt c433c2434c0459df98ed3355ef615e341acd9009 baseline version: libvirt 092176e5ec2f45d3b56a9e131c5237c52712d804 Last test of basis 176926 2023-02-11 04:20:15 Z3 days Testing same since 177242 2023-02-14 04:18:49 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Peter Krempa Piotr Drąg jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt starved test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 starved test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw starved test-amd64-i386-libvirt-raw pass test-amd64-amd64-libvirt-vhd 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;
Re: [PATCH] xen: speed up grant-table reclaim
On Tue, Feb 14, 2023 at 08:51:09AM +0100, Juergen Gross wrote: > On 13.02.23 22:01, Demi Marie Obenour wrote: > > On Mon, Feb 13, 2023 at 10:26:11AM +0100, Juergen Gross wrote: > > > On 07.02.23 03:10, Demi Marie Obenour wrote: > > > > When a grant entry is still in use by the remote domain, Linux must put > > > > it on a deferred list. Normally, this list is very short, because > > > > the PV network and block protocols expect the backend to unmap the grant > > > > first. However, Qubes OS's GUI protocol is subject to the constraints > > > > of the X Window System, and as such winds up with the frontend unmapping > > > > the window first. As a result, the list can grow very large, resulting > > > > in a massive memory leak and eventual VM freeze. > > > > > > > > Fix this problem by bumping the number of entries that the VM will > > > > attempt to free at each iteration to 1. This is an ugly hack that > > > > may well make a denial of service easier, but for Qubes OS that is less > > > > bad than the problem Qubes OS users are facing today. > > > > > > > There really > > > > needs to be a way for a frontend to be notified when the backend has > > > > unmapped the grants. > > > > > > Please remove this sentence from the commit message, or move it below the > > > "---" marker. > > > > Will fix in v2. > > > > > There are still some flag bits unallocated in struct grant_entry_v1 or > > > struct grant_entry_header. You could suggest some patches for Xen to use > > > one of the bits as a marker to get an event from the hypervisor if a > > > grant with such a bit set has been unmapped. > > > > That is indeed a good idea. There are other problems with the grant > > interface as well, but those can be dealt with later. > > > > > I have no idea, whether such an interface would be accepted by the > > > maintainers, though. > > > > > > > Additionally, a module parameter is provided to > > > > allow tuning the reclaim speed. > > > > > > > > The code previously used printk(KERN_DEBUG) whenever it had to defer > > > > reclaiming a page because the grant was still mapped. This resulted in > > > > a large volume of log messages that bothered users. Use pr_debug > > > > instead, which suppresses the messages by default. Developers can > > > > enable them using the dynamic debug mechanism. > > > > > > > > Fixes: QubesOS/qubes-issues#7410 (memory leak) > > > > Fixes: QubesOS/qubes-issues#7359 (excessive logging) > > > > Fixes: 569ca5b3f94c ("xen/gnttab: add deferred freeing logic") > > > > Cc: sta...@vger.kernel.org > > > > Signed-off-by: Demi Marie Obenour > > > > --- > > > > Anyone have suggestions for improving the grant mechanism? Argo isn't > > > > a good option, as in the GUI protocol there are substantial performance > > > > wins to be had by using true shared memory. Resending as I forgot the > > > > Signed-off-by on the first submission. Sorry about that. > > > > > > > >drivers/xen/grant-table.c | 2 +- > > > >1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > > > > index 5c83d41..2c2faa7 100644 > > > > --- a/drivers/xen/grant-table.c > > > > +++ b/drivers/xen/grant-table.c > > > > @@ -355,14 +355,20 @@ > > > >static void gnttab_handle_deferred(struct timer_list *); > > > >static DEFINE_TIMER(deferred_timer, gnttab_handle_deferred); > > > > +static atomic64_t deferred_count; > > > > +static atomic64_t leaked_count; > > > > +static unsigned int free_per_iteration = 1; > > > > > > As you are adding a kernel parameter to change this value, please set the > > > default to a value not potentially causing any DoS problems. Qubes OS can > > > still use a higher value then. > > > > Do you have any suggestions? I don’t know if this is actually a DoS > > concern anymore. Shrinking the interval between iterations would be. > > Why don't you use today's value of 10 for the default? Will do. I now remember that the DoS concern is that the kernel could be made to use excess CPU trying and failing to reclaim memory. > > > > + > > > >static void gnttab_handle_deferred(struct timer_list *unused) > > > >{ > > > > - unsigned int nr = 10; > > > > + unsigned int nr = READ_ONCE(free_per_iteration); > > > > > > I don't see why you are needing READ_ONCE() here. > > > > free_per_iteration can be concurrently modified via sysfs. > > My remark was based on the wrong assumption that ignore_limit could be > dropped. Even if ignore_limit could not be dropped, READ_ONCE is still necessary to avoid a data race. -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab signature.asc Description: PGP signature
Re: linux-next: duplicate patches in the xen-tip tree
On Tue, Feb 14, 2023 at 12:47:00PM +1100, Stephen Rothwell wrote: > Hi all, > > The following commits are also in the tip tree as different commits > (but the same patches): > > 415dab3c1796 ("drivers/xen/hypervisor: Expose Xen SIF flags to userspace") > 336f560a8917 ("x86/xen: don't let xen_pv_play_dead() return") > f697cb00afa9 ("x86/xen: mark xen_pv_play_dead() as __noreturn") > > These are commits > > 859761e770f8 ("drivers/xen/hypervisor: Expose Xen SIF flags to userspace") > 076cbf5d2163 ("x86/xen: don't let xen_pv_play_dead() return") > 1aff0d2658e5 ("x86/xen: mark xen_pv_play_dead() as __noreturn") > > in the tip tree. This was intentional (dependencies) and the plan is to only offer the tip branch for merge after the Xen tree goes in. signature.asc Description: PGP signature
[PATCH v2 5/5] automation: Add a true dom0less test on arm32
Add a new test job qemu-smoke-dom0less-arm32-gcc-without-dom0 in debug and non-debug variant that will execute qemu-smoke-dom0less-arm32.sh script to test dom0less domU boot without dom0 (i.e. true dom0less configuration). By passing "without-dom0" as a test variant, the test will clear the dom0 prompt that we grep for as a last step and remove all the DOM0 related options in ImageBuilder configuration. Signed-off-by: Michal Orzel --- Changes in v2: - new patch created as a result of deciding to keep dom0 by default. We still need to test true dom0less configuration, hence added a new test. --- automation/gitlab-ci/test.yaml | 16 automation/scripts/qemu-smoke-dom0less-arm32.sh | 13 + 2 files changed, 29 insertions(+) diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index be7a55d89708..c0b4a90e0d29 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -258,6 +258,22 @@ qemu-smoke-dom0less-arm32-gcc-debug-gzip: - *arm32-test-needs - debian-unstable-gcc-arm32-debug +qemu-smoke-dom0less-arm32-gcc-without-dom0: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh without-dom0 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32 + +qemu-smoke-dom0less-arm32-gcc-debug-without-dom0: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh without-dom0 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32-debug + qemu-alpine-x86_64-gcc: extends: .qemu-x86-64 script: diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh b/automation/scripts/qemu-smoke-dom0less-arm32.sh index c2e331451d99..cc91238f4222 100755 --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh @@ -42,6 +42,15 @@ echo \"${passed}\" " fi +if [[ "${test_variant}" == "without-dom0" ]]; then +# Clear dom0 prompt +dom0_prompt="" +passed="${test_variant} test passed" +domU_check=" +echo \"${passed}\" +" +fi + # dom0/domU rootfs # We are using the same rootfs for dom0 and domU. The only difference is # that for the former, we set explictly rdinit to /bin/sh, whereas for the @@ -102,6 +111,10 @@ if [[ "${test_variant}" == "gzip" ]]; then sed -i 's/DOMU_KERNEL\[0\]=.*/DOMU_KERNEL\[0\]="vmlinuz.gz"/' config fi +if [[ "${test_variant}" == "without-dom0" ]]; then +sed -i '/^DOM0/d' config +fi + rm -rf imagebuilder git clone https://gitlab.com/ViryaOS/imagebuilder bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config -- 2.25.1
[PATCH v2 4/5] automation: Add a gzip compressed kernel image test on arm32
Xen supports booting gzip compressed kernels, therefore add a new test job qemu-smoke-dom0less-arm32-gcc-gzip in debug and non-debug variant that will execute qemu-smoke-dom0less-arm32.sh script to test booting a domU with a gzip compressed kernel image (in our case zImage). By passing "gzip" as a test variant, the test will call gzip command to compress kernel image and use it in ImageBuilder configuration for domU1. No need for a specific check to be executed from domU. Being able to see a test message from a boot log is sufficient to mark a test as passed. Signed-off-by: Michal Orzel --- Changes in v2: - take into account dom0 presence - drop Rb as a result of code changes --- automation/gitlab-ci/test.yaml | 16 automation/scripts/qemu-smoke-dom0less-arm32.sh | 13 + 2 files changed, 29 insertions(+) diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index c2bcc5d4d3e5..be7a55d89708 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -242,6 +242,22 @@ qemu-smoke-dom0less-arm32-gcc-debug-staticmem: - *arm32-test-needs - debian-unstable-gcc-arm32-debug-staticmem +qemu-smoke-dom0less-arm32-gcc-gzip: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh gzip 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32 + +qemu-smoke-dom0less-arm32-gcc-debug-gzip: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh gzip 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32-debug + qemu-alpine-x86_64-gcc: extends: .qemu-x86-64 script: diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh b/automation/scripts/qemu-smoke-dom0less-arm32.sh index bd89a3f8b45e..c2e331451d99 100755 --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh @@ -33,6 +33,15 @@ fi " fi +if [[ "${test_variant}" == "gzip" ]]; then +# Compress kernel image with gzip (keep unmodified one for dom0) +gzip -k vmlinuz +passed="${test_variant} test passed" +domU_check=" +echo \"${passed}\" +" +fi + # dom0/domU rootfs # We are using the same rootfs for dom0 and domU. The only difference is # that for the former, we set explictly rdinit to /bin/sh, whereas for the @@ -89,6 +98,10 @@ if [[ "${test_variant}" == "static-mem" ]]; then echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >> config fi +if [[ "${test_variant}" == "gzip" ]]; then +sed -i 's/DOMU_KERNEL\[0\]=.*/DOMU_KERNEL\[0\]="vmlinuz.gz"/' config +fi + rm -rf imagebuilder git clone https://gitlab.com/ViryaOS/imagebuilder bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config -- 2.25.1
[PATCH v2 3/5] automation: Add a static memory allocation test on arm32
Add a new test job qemu-smoke-dom0less-arm32-gcc-staticmem in debug and non-debug variant that will execute qemu-smoke-dom0less-arm32.sh script to test static memory allocation feature. The test case itself is directly taken from dom0less arm64 testing. Populate build jobs to compile Xen with config options necessary to enable static memory feature. Populate test jobs passing "static-mem" as a test variant. The test configures domU with a static memory region (direct-mapped) and adds a check using /proc/iomem to determine the memory region marked as "System RAM". Signed-off-by: Michal Orzel --- Changes in v2: - take into account new container for arm32 cross builds - drop Rb as a result of code changes --- automation/gitlab-ci/build.yaml | 20 +++ automation/gitlab-ci/test.yaml| 16 +++ .../scripts/qemu-smoke-dom0less-arm32.sh | 17 3 files changed, 53 insertions(+) diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index f8e156e0a994..079e9b73f659 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -565,6 +565,26 @@ debian-unstable-gcc-arm32-debug-randconfig: HYPERVISOR_ONLY: y RANDCONFIG: y +debian-unstable-gcc-arm32-staticmem: + extends: .gcc-arm32-cross-build + variables: +CONTAINER: debian:unstable-arm64v8-arm32-gcc +HYPERVISOR_ONLY: y +EXTRA_XEN_CONFIG: | + CONFIG_EXPERT=y + CONFIG_UNSUPPORTED=y + CONFIG_STATIC_MEMORY=y + +debian-unstable-gcc-arm32-debug-staticmem: + extends: .gcc-arm32-cross-build-debug + variables: +CONTAINER: debian:unstable-arm64v8-arm32-gcc +HYPERVISOR_ONLY: y +EXTRA_XEN_CONFIG: | + CONFIG_EXPERT=y + CONFIG_UNSUPPORTED=y + CONFIG_STATIC_MEMORY=y + # Arm builds debian-unstable-gcc-arm64: diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 84ab1fee50a4..c2bcc5d4d3e5 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -226,6 +226,22 @@ qemu-smoke-dom0less-arm32-gcc-debug: - *arm32-test-needs - debian-unstable-gcc-arm32-debug +qemu-smoke-dom0less-arm32-gcc-staticmem: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh static-mem 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32-staticmem + +qemu-smoke-dom0less-arm32-gcc-debug-staticmem: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh static-mem 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32-debug-staticmem + qemu-alpine-x86_64-gcc: extends: .qemu-x86-64 script: diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh b/automation/scripts/qemu-smoke-dom0less-arm32.sh index e3f2b28f3f89..bd89a3f8b45e 100755 --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh @@ -20,6 +20,19 @@ echo \"${passed}\" " fi +if [[ "${test_variant}" == "static-mem" ]]; then +# Memory range that is statically allocated to domU1 +domu_base="0x5000" +domu_size="0x2000" +passed="${test_variant} test passed" +domU_check=" +mem_range=$(printf \"%08x-%08x\" ${domu_base} $(( ${domu_base} + ${domu_size} - 1 ))) +if grep -q -x \"\${mem_range} : System RAM\" /proc/iomem; then +echo \"${passed}\" +fi +" +fi + # dom0/domU rootfs # We are using the same rootfs for dom0 and domU. The only difference is # that for the former, we set explictly rdinit to /bin/sh, whereas for the @@ -72,6 +85,10 @@ BOOT_CMD="bootm" UBOOT_SOURCE="boot.source" UBOOT_SCRIPT="boot.scr"' > config +if [[ "${test_variant}" == "static-mem" ]]; then +echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >> config +fi + rm -rf imagebuilder git clone https://gitlab.com/ViryaOS/imagebuilder bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config -- 2.25.1
[PATCH v2 1/5] automation: Switch arm32 cross builds to run on arm64
Due to the limited x86 CI resources slowing down the whole pipeline, switch the arm32 cross builds to be executed on arm64 which is much more capable. For that, rename the existing debian container dockerfile from unstable-arm32-gcc to unstable-arm64v8-arm32-gcc and use arm64v8/debian:unstable as an image. Note, that we cannot use the same container name as we have to keep the backwards compatibility. Take the opportunity to remove extra empty line at the end of a file. Modify the tag of .arm32-cross-build-tmpl to arm64 and update the build jobs accordingly. Signed-off-by: Michal Orzel --- Changes in v2: - new patch For convenience and own testing, I built and pushed the new container to registry. --- ...ockerfile => unstable-arm64v8-arm32-gcc.dockerfile} | 3 +-- automation/gitlab-ci/build.yaml| 10 +- 2 files changed, 6 insertions(+), 7 deletions(-) rename automation/build/debian/{unstable-arm32-gcc.dockerfile => unstable-arm64v8-arm32-gcc.dockerfile} (94%) diff --git a/automation/build/debian/unstable-arm32-gcc.dockerfile b/automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile similarity index 94% rename from automation/build/debian/unstable-arm32-gcc.dockerfile rename to automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile index b41a57f19729..11860425a6a2 100644 --- a/automation/build/debian/unstable-arm32-gcc.dockerfile +++ b/automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile @@ -1,4 +1,4 @@ -FROM debian:unstable +FROM arm64v8/debian:unstable LABEL maintainer.name="The Xen Project" \ maintainer.email="xen-devel@lists.xenproject.org" @@ -21,4 +21,3 @@ RUN apt-get update && \ apt-get autoremove -y && \ apt-get clean && \ rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* - diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index a053c5c7325d..f8e156e0a994 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -123,7 +123,7 @@ variables: XEN_TARGET_ARCH: arm32 tags: -- x86_64 +- arm64 .arm32-cross-build: extends: .arm32-cross-build-tmpl @@ -542,26 +542,26 @@ alpine-3.12-clang-debug: debian-unstable-gcc-arm32: extends: .gcc-arm32-cross-build variables: -CONTAINER: debian:unstable-arm32-gcc +CONTAINER: debian:unstable-arm64v8-arm32-gcc HYPERVISOR_ONLY: y debian-unstable-gcc-arm32-debug: extends: .gcc-arm32-cross-build-debug variables: -CONTAINER: debian:unstable-arm32-gcc +CONTAINER: debian:unstable-arm64v8-arm32-gcc HYPERVISOR_ONLY: y debian-unstable-gcc-arm32-randconfig: extends: .gcc-arm32-cross-build variables: -CONTAINER: debian:unstable-arm32-gcc +CONTAINER: debian:unstable-arm64v8-arm32-gcc HYPERVISOR_ONLY: y RANDCONFIG: y debian-unstable-gcc-arm32-debug-randconfig: extends: .gcc-arm32-cross-build-debug variables: -CONTAINER: debian:unstable-arm32-gcc +CONTAINER: debian:unstable-arm64v8-arm32-gcc HYPERVISOR_ONLY: y RANDCONFIG: y -- 2.25.1
[PATCH v2 2/5] automation: Add arm32 dom0less testing
At the moment, we only perform a single arm32 test in our CI, checking whether dom0 boots successfully or not. This is mostly because we do not have any arm32 runners and we only execute a hypervisor only build. In order not to leave the arm32 testing in such a poor state, add a script qemu-smoke-dom0less-arm32.sh to start testing dom0less configuration, while keeping dom0 to make the test more interesting. The script is mostly based on the one used for dom0 arm32 testing as well as the one used for dom0less arm64 testing. We obtain Debian Bullseye kernel and Alpine Linux busybox-based rootfs. Depending on the test variant, we prepare a test case contained within domU_check variable, that will be executed as part of /init script from domU rootfs. Signed-off-by: Michal Orzel --- Changes in v2: - keep dom0 around by default to make tests more interesting --- automation/gitlab-ci/test.yaml| 16 +++ .../scripts/qemu-smoke-dom0less-arm32.sh | 99 +++ 2 files changed, 115 insertions(+) create mode 100755 automation/scripts/qemu-smoke-dom0less-arm32.sh diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index ce543ef5c0ef..84ab1fee50a4 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -210,6 +210,22 @@ qemu-smoke-dom0-arm32-gcc-debug: - *arm32-test-needs - debian-unstable-gcc-arm32-debug +qemu-smoke-dom0less-arm32-gcc: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32 + +qemu-smoke-dom0less-arm32-gcc-debug: + extends: .qemu-arm32 + script: +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh 2>&1 | tee ${LOGFILE} + needs: +- *arm32-test-needs +- debian-unstable-gcc-arm32-debug + qemu-alpine-x86_64-gcc: extends: .qemu-x86-64 script: diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh b/automation/scripts/qemu-smoke-dom0less-arm32.sh new file mode 100755 index ..e3f2b28f3f89 --- /dev/null +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh @@ -0,0 +1,99 @@ +#!/bin/bash + +set -ex + +test_variant=$1 + +# Prompt to grep for to check if dom0 booted successfully +dom0_prompt="^/ #" + +cd binaries +# Use the kernel from Debian +curl --fail --silent --show-error --location --output vmlinuz https://deb.debian.org/debian/dists/bullseye/main/installer-armhf/current/images/netboot/vmlinuz +# Use a tiny initrd based on busybox from Alpine Linux +curl --fail --silent --show-error --location --output initrd.tar.gz https://dl-cdn.alpinelinux.org/alpine/v3.15/releases/armhf/alpine-minirootfs-3.15.1-armhf.tar.gz + +if [ -z "${test_variant}" ]; then +passed="generic test passed" +domU_check=" +echo \"${passed}\" +" +fi + +# dom0/domU rootfs +# We are using the same rootfs for dom0 and domU. The only difference is +# that for the former, we set explictly rdinit to /bin/sh, whereas for the +# latter we rely on using custom /init script with test case inside. +mkdir rootfs +cd rootfs +tar xvzf ../initrd.tar.gz +echo "#!/bin/sh + +mount -t proc proc /proc +mount -t sysfs sysfs /sys +mount -t devtmpfs devtmpfs /dev +${domU_check} +/bin/sh" > init +chmod +x init +find . | cpio -H newc -o | gzip > ../initrd.gz +cd .. + +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded +curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom +./qemu-system-arm \ +-machine virt \ +-machine virtualization=true \ +-smp 4 \ +-m 2048 \ +-serial stdio \ +-monitor none \ +-display none \ +-machine dumpdtb=virt.dtb + +# ImageBuilder +echo 'MEMORY_START="0x4000" +MEMORY_END="0xC000" + +DEVICE_TREE="virt.dtb" +XEN="xen" +XEN_CMD="console=dtuart dom0_mem=512M bootscrub=0" + +DOM0_KERNEL="vmlinuz" +DOM0_RAMDISK="initrd.gz" +DOM0_CMD="console=hvc0 earlyprintk clk_ignore_unused root=/dev/ram0 rdinit=/bin/sh" + +DOMU_KERNEL[0]="vmlinuz" +DOMU_RAMDISK[0]="initrd.gz" +DOMU_MEM[0]="512" +NUM_DOMUS=1 + +LOAD_CMD="tftpb" +BOOT_CMD="bootm" +UBOOT_SOURCE="boot.source" +UBOOT_SCRIPT="boot.scr"' > config + +rm -rf imagebuilder +git clone https://gitlab.com/ViryaOS/imagebuilder +bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config + +# Run the test +rm -f smoke.serial +set +e +echo " virtio scan; dhcp; tftpb 0x4000 boot.scr; source 0x4000"| \ +timeout -k 1 240 \ +./qemu-system-arm \ +-machine virt \ +-machine virtualization=true \ +-smp 4 \ +-m 2048 \ +-serial stdio \ +-monitor none \ +-display none \ +-no-reboot \ +-device virtio-net-pci,netdev=n0 \ +-netdev user,id=n0,tftp=./ \ +-bios /usr/lib/u-boot/qemu_arm/u-boot.bin |& tee smoke.serial + +set -e +(grep -q "${dom0_prompt}" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1 +exit 0 -- 2.25.1
[PATCH v2 0/5] automation: dom0less arm32 testing
This patch series aims at improving the arm32 CI testing by introducing the dom0less-based tests. It creates a foundation for further test expansion. This is particularly important now, when OSSTEST arm32 stuff is down and we need to have at least some coverage in gitlab CI. Note: First patch is added to the series for convenience. It switches the arm32 cross builds to be executed on arm64 instead of x86, as the latter has a lot less resources resulting in slowing down the whole pipeline. CI pipeline performed on top of this series + cppcheck patch: https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/777181033 Michal Orzel (5): automation: Switch arm32 cross builds to run on arm64 automation: Add arm32 dom0less testing automation: Add a static memory allocation test on arm32 automation: Add a gzip compressed kernel image test on arm32 automation: Add a true dom0less test on arm32 ... => unstable-arm64v8-arm32-gcc.dockerfile} | 3 +- automation/gitlab-ci/build.yaml | 30 +++- automation/gitlab-ci/test.yaml| 64 .../scripts/qemu-smoke-dom0less-arm32.sh | 142 ++ 4 files changed, 232 insertions(+), 7 deletions(-) rename automation/build/debian/{unstable-arm32-gcc.dockerfile => unstable-arm64v8-arm32-gcc.dockerfile} (94%) create mode 100755 automation/scripts/qemu-smoke-dom0less-arm32.sh -- 2.25.1
[PATCH v2] automation: Add container and build jobs to run cppcheck analysis
Add a debian container with cppcheck installation routine inside, capable of performing cppcheck analysis on Xen-only build including cross-builds for arm32 and x86_64. Populate build jobs making use of that container to run cppcheck analysis to produce a text report (xen-cppcheck.txt) containing the list of all the findings. This patch does not aim at performing any sort of bisection. Cppcheck is imperfect and for now, our goal is to at least be aware of its reports, so that we can compare them with the ones produced by better tools and to be able to see how these reports change as a result of further infrastructure improvements (e.g. exception list, rules exclusion). Signed-off-by: Michal Orzel --- Changes in v2: - use arm64 container instead of x86 to make pipeline faster - explicitly set HYPERVISOR_ONLY=y for cppcheck jobs For convenience and own testing, I built and pushed the new container to registry. CI pipeline including dom0less series: https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/777181033 --- .../build/debian/unstable-cppcheck.dockerfile | 37 automation/gitlab-ci/build.yaml | 43 +++ automation/scripts/build | 11 - 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 automation/build/debian/unstable-cppcheck.dockerfile diff --git a/automation/build/debian/unstable-cppcheck.dockerfile b/automation/build/debian/unstable-cppcheck.dockerfile new file mode 100644 index ..54b99f87dfec --- /dev/null +++ b/automation/build/debian/unstable-cppcheck.dockerfile @@ -0,0 +1,37 @@ +FROM arm64v8/debian:unstable +LABEL maintainer.name="The Xen Project" \ + maintainer.email="xen-devel@lists.xenproject.org" + +ENV DEBIAN_FRONTEND=noninteractive +ENV CPPCHECK_VERSION=2.7 +ENV USER root + +RUN mkdir /build +WORKDIR /build + +# dependencies for cppcheck and Xen-only build/cross-build +RUN apt-get update && \ +apt-get --quiet --yes install \ +build-essential \ +curl \ +python-is-python3 \ +libpcre3-dev \ +flex \ +bison \ +gcc-arm-linux-gnueabihf \ +gcc-x86-64-linux-gnu + +# cppcheck release build (see cppcheck readme.md) +RUN curl -fsSLO https://github.com/danmar/cppcheck/archive/"$CPPCHECK_VERSION".tar.gz && \ +tar xvzf "$CPPCHECK_VERSION".tar.gz && \ +cd cppcheck-"$CPPCHECK_VERSION" && \ +make install -j$(nproc) \ +MATCHCOMPILER=yes \ +FILESDIR=/usr/share/cppcheck \ +HAVE_RULES=yes CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare -Wno-unused-function" + +# clean +RUN apt-get autoremove -y && \ +apt-get clean && \ +rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* && \ +rm -rf cppcheck-"$CPPCHECK_VERSION"* "$CPPCHECK_VERSION".tar.gz diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index 079e9b73f659..1441b7dbb6fa 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -7,6 +7,7 @@ paths: - binaries/ - xen-config + - xen-cppcheck.txt - '*.log' - '*/*.log' when: always @@ -199,6 +200,23 @@ variables: <<: *gcc +.x86-64-cross-build-tmpl: + <<: *build + variables: +XEN_TARGET_ARCH: x86_64 + tags: +- arm64 + +.x86-64-cross-build: + extends: .x86-64-cross-build-tmpl + variables: +debug: n + +.gcc-x86-64-cross-build: + extends: .x86-64-cross-build + variables: +<<: *gcc + # Jobs below this line archlinux-gcc: @@ -699,6 +717,31 @@ archlinux-current-gcc-riscv64-debug-randconfig: EXTRA_FIXED_RANDCONFIG: CONFIG_COVERAGE=n +# Cppcheck analysis jobs + +debian-unstable-gcc-cppcheck: + extends: .gcc-x86-64-cross-build + variables: +CONTAINER: debian:unstable-cppcheck +CROSS_COMPILE: /usr/bin/x86_64-linux-gnu- +CPPCHECK: y +HYPERVISOR_ONLY: y + +debian-unstable-gcc-arm32-cppcheck: + extends: .gcc-arm32-cross-build + variables: +CONTAINER: debian:unstable-cppcheck +CROSS_COMPILE: /usr/bin/arm-linux-gnueabihf- +CPPCHECK: y +HYPERVISOR_ONLY: y + +debian-unstable-gcc-arm64-cppcheck: + extends: .gcc-arm64-build + variables: +CONTAINER: debian:unstable-cppcheck +CPPCHECK: y +HYPERVISOR_ONLY: y + ## Test artifacts common .test-jobs-artifact-common: diff --git a/automation/scripts/build b/automation/scripts/build index f2f5e55bc04f..7d1b19c4250d 100755 --- a/automation/scripts/build +++ b/automation/scripts/build @@ -38,7 +38,16 @@ cp xen/.config xen-config # Directory for the artefacts to be dumped into mkdir binaries -if [[ "${HYPERVISOR_ONLY}" == "y" ]]; then +if [[ "${CPPCHECK}" == "y" ]] && [[ "${HYPERVISOR_ONLY}" == "y" ]]; then +# Cppcheck analysis invokes Xen-only build. +# Known limitation: cppcheck generates inconsistent reports when running +# in parallel mode, therefore do not specify -j. +xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra + +
[PATCH v4 0/4 + v1 0/1] x86/spec-ctrl: IBPB improvements
Versions of the two final patches were submitted standalone earlier on. The series here tries to carry out a suggestion from Andrew, which the two of us have been discussing. Then said previously posted patches are re-based on top, utilizing the new functionality. Xen: 1: spec-ctrl: add logic to issue IBPB on exit to guest 2: spec-ctrl: defer context-switch IBPB until guest entry 3: limit issuing of IBPB during context switch 4: PV: issue branch prediction barrier when switching 64-bit guest to kernel mode Linux (new): 1: x86/Xen: make use of IBPB controlling VM assist Jan
[PATCH v4 1/4] x86/spec-ctrl: add logic to issue IBPB on exit to guest
In order to be able to defer the context switch IBPB to the last possible point, add logic to the exit-to-guest paths to issue the barrier there, including the "IBPB doesn't flush the RSB/RAS" workaround. Since alternatives, for now at least, can't nest, emit JMP to skip past both constructs where both are needed. This may be more efficient anyway, as the sequence of NOPs is pretty long. As with all other conditional blocks on exit-to-guest paths, no Spectre-v1 protections are necessary as execution will imminently be hitting a serialising event. Signed-off-by: Jan Beulich --- I have to admit that I'm not really certain about the placement of the IBPB wrt the MSR_SPEC_CTRL writes. For now I've simply used "opposite of entry". Since we're going to run out of SCF_* bits soon and since the new flag is meaningful only in struct cpu_info's spec_ctrl_flags, we could choose to widen that field to 16 bits right away and then use bit 8 (or higher) for the purpose here. --- v4: Alter parts of the description. Re-word a comment. Rename flag and feature identifiers. v3: New. --- a/xen/arch/x86/hvm/svm/entry.S +++ b/xen/arch/x86/hvm/svm/entry.S @@ -75,6 +75,12 @@ __UNLIKELY_END(nsvm_hap) .endm ALTERNATIVE "", svm_vmentry_spec_ctrl, X86_FEATURE_SC_MSR_HVM +ALTERNATIVE "jmp 2f", __stringify(DO_SPEC_CTRL_EXIT_IBPB disp=(2f-1f)), \ +X86_FEATURE_NEW_PRED_CTXT_HVM +1: +ALTERNATIVE "", DO_OVERWRITE_RSB, X86_BUG_IBPB_NO_RET +2: + pop %r15 pop %r14 pop %r13 --- a/xen/arch/x86/hvm/vmx/entry.S +++ b/xen/arch/x86/hvm/vmx/entry.S @@ -86,7 +86,8 @@ UNLIKELY_END(realmode) jz .Lvmx_vmentry_restart /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ -/* SPEC_CTRL_EXIT_TO_VMX Req: %rsp=regs/cpuinfo Clob: */ +/* SPEC_CTRL_EXIT_TO_VMX Req: %rsp=regs/cpuinfo Clob: acd */ +ALTERNATIVE "", DO_SPEC_CTRL_EXIT_IBPB, X86_FEATURE_NEW_PRED_CTXT_HVM DO_SPEC_CTRL_COND_VERW mov VCPU_hvm_guest_cr2(%rbx),%rax --- a/xen/arch/x86/include/asm/cpufeatures.h +++ b/xen/arch/x86/include/asm/cpufeatures.h @@ -39,8 +39,10 @@ XEN_CPUFEATURE(XEN_LBR, X86_SY XEN_CPUFEATURE(SC_VERW_IDLE, X86_SYNTH(25)) /* VERW used by Xen for idle */ XEN_CPUFEATURE(XEN_SHSTK, X86_SYNTH(26)) /* Xen uses CET Shadow Stacks */ XEN_CPUFEATURE(XEN_IBT, X86_SYNTH(27)) /* Xen uses CET Indirect Branch Tracking */ -XEN_CPUFEATURE(IBPB_ENTRY_PV, X86_SYNTH(28)) /* MSR_PRED_CMD used by Xen for PV */ -XEN_CPUFEATURE(IBPB_ENTRY_HVM,X86_SYNTH(29)) /* MSR_PRED_CMD used by Xen for HVM */ +XEN_CPUFEATURE(IBPB_ENTRY_PV, X86_SYNTH(28)) /* MSR_PRED_CMD used by Xen when entered from PV */ +XEN_CPUFEATURE(IBPB_ENTRY_HVM,X86_SYNTH(29)) /* MSR_PRED_CMD used by Xen when entered from HVM */ +XEN_CPUFEATURE(NEW_PRED_CTXT_PV, X86_SYNTH(30)) /* issue prediction barrier when exiting to PV */ +XEN_CPUFEATURE(NEW_PRED_CTXT_HVM, X86_SYNTH(31)) /* issue prediction barrier when exiting to HVM */ /* Bug words follow the synthetic words. */ #define X86_NR_BUG 1 --- a/xen/arch/x86/include/asm/current.h +++ b/xen/arch/x86/include/asm/current.h @@ -55,9 +55,13 @@ struct cpu_info { /* See asm/spec_ctrl_asm.h for usage. */ unsigned int shadow_spec_ctrl; +/* + * spec_ctrl_flags is accessed as a 32-bit entity in certain cases. Place + * it accordingly. + */ +uint8_t spec_ctrl_flags; uint8_t xen_spec_ctrl; uint8_t last_spec_ctrl; -uint8_t spec_ctrl_flags; /* * The following field controls copying of the L4 page table of 64-bit --- a/xen/arch/x86/include/asm/spec_ctrl.h +++ b/xen/arch/x86/include/asm/spec_ctrl.h @@ -36,6 +36,8 @@ #define SCF_verw (1 << 3) #define SCF_ist_ibpb (1 << 4) #define SCF_entry_ibpb (1 << 5) +#define SCF_new_pred_ctxt_bit 6 +#define SCF_new_pred_ctxt (1 << SCF_new_pred_ctxt_bit) /* * The IST paths (NMI/#MC) can interrupt any arbitrary context. Some --- a/xen/arch/x86/include/asm/spec_ctrl_asm.h +++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h @@ -117,6 +117,27 @@ .L\@_done: .endm +.macro DO_SPEC_CTRL_EXIT_IBPB disp=0 +/* + * Requires %rsp=regs + * Clobbers %rax, %rcx, %rdx + * + * Conditionally issue IBPB if SCF_new_pred_ctxt is active. The macro + * invocation may be followed by X86_BUG_IBPB_NO_RET workaround code. The + * "disp" argument is to allow invocation sites to pass in the extra amount + * of code which needs skipping in case no action is necessary. + * + * The flag is a "one-shot" indicator, so it is being cleared at the same time. + */ +btrl$SCF_new_pred_ctxt_bit, CPUINFO_spec_ctrl_flags(%rsp) +jnc .L\@_skip + (\disp) +mov $MSR_PRED_CMD, %ecx +mov $PRED_CMD_IBPB, %eax +xor %edx, %edx +wrmsr +.L\@_skip: +.endm + .macro DO_OVERWRITE_RSB tmp=rax /* * Requires nothing @@ -272,6
[PATCH v4 2/4] x86/spec-ctrl: defer context-switch IBPB until guest entry
In order to avoid clobbering Xen's own predictions, defer the barrier as much as possible. Merely mark the CPU as needing a barrier issued the next time we're exiting to guest context. Suggested-by: Andrew Cooper Signed-off-by: Jan Beulich --- I couldn't find any sensible (central/unique) place where to move the comment which is being deleted alongside spec_ctrl_new_guest_context(). (If this patch is to survive in the first place, it was suggested to move to spect_ctrl_asm.h, next to the #define of the controlling bit.) --- v4: Re-base in particular over changes earlier in the series. v3: New. --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -2038,7 +2038,7 @@ void context_switch(struct vcpu *prev, s */ if ( *last_id != next_id ) { -spec_ctrl_new_guest_context(); +info->spec_ctrl_flags |= SCF_new_pred_ctxt; *last_id = next_id; } } --- a/xen/arch/x86/include/asm/spec_ctrl.h +++ b/xen/arch/x86/include/asm/spec_ctrl.h @@ -67,28 +67,6 @@ void init_speculation_mitigations(void); void spec_ctrl_init_domain(struct domain *d); -/* - * Switch to a new guest prediction context. - * - * This flushes all indirect branch predictors (BTB, RSB/RAS), so guest code - * which has previously run on this CPU can't attack subsequent guest code. - * - * As this flushes the RSB/RAS, it destroys the predictions of the calling - * context. For best performace, arrange for this to be used when we're going - * to jump out of the current context, e.g. with reset_stack_and_jump(). - * - * For hardware which mis-implements IBPB, fix up by flushing the RSB/RAS - * manually. - */ -static always_inline void spec_ctrl_new_guest_context(void) -{ -wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB); - -/* (ab)use alternative_input() to specify clobbers. */ -alternative_input("", "DO_OVERWRITE_RSB", X86_BUG_IBPB_NO_RET, - : "rax", "rcx"); -} - extern int8_t opt_ibpb_ctxt_switch; extern bool opt_ssbd; extern int8_t opt_eager_fpu; --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -854,6 +854,11 @@ static void __init ibpb_calculations(voi */ if ( opt_ibpb_ctxt_switch == -1 ) opt_ibpb_ctxt_switch = !(opt_ibpb_entry_hvm && opt_ibpb_entry_pv); +if ( opt_ibpb_ctxt_switch ) +{ +setup_force_cpu_cap(X86_FEATURE_NEW_PRED_CTXT_PV); +setup_force_cpu_cap(X86_FEATURE_NEW_PRED_CTXT_HVM); +} } /* Calculate whether this CPU is vulnerable to L1TF. */
[PATCH v4 3/4] x86: limit issuing of IBPB during context switch
When the outgoing vCPU had IBPB issued and RSB overwritten upon entering Xen, then there's no need for a 2nd barrier during context switch. Note that SCF_entry_ibpb is always clear for the idle domain, so no explicit idle domain check is needed to augment the feature check (which is simply inapplicable to "idle"). Signed-off-by: Jan Beulich --- v4: Tighten the condition. v3: Fold into series. --- I think in principle we could limit the impact from finding the idle domain as "prevd", by having __context_switch() tell us what kind domain's vCPU was switched out (it could still be "idle", but in fewer cases). --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -2005,17 +2005,26 @@ void context_switch(struct vcpu *prev, s } else { +unsigned int feat_sc_rsb = X86_FEATURE_SC_RSB_HVM; + __context_switch(); /* Re-enable interrupts before restoring state which may fault. */ local_irq_enable(); if ( is_pv_domain(nextd) ) +{ load_segments(next); +feat_sc_rsb = X86_FEATURE_SC_RSB_PV; +} + ctxt_switch_levelling(next); -if ( opt_ibpb_ctxt_switch && !is_idle_domain(nextd) ) +if ( opt_ibpb_ctxt_switch && !is_idle_domain(nextd) && + (!(prevd->arch.spec_ctrl_flags & SCF_entry_ibpb) || + /* is_idle_domain(prevd) || */ + !boot_cpu_has(feat_sc_rsb)) ) { static DEFINE_PER_CPU(unsigned int, last); unsigned int *last_id = &this_cpu(last);
[PATCH v4 4/4] x86/PV: issue branch prediction barrier when switching 64-bit guest to kernel mode
Since both kernel and user mode run in ring 3, they run in the same "predictor mode". While the kernel could take care of this itself, doing so would be yet another item distinguishing PV from native. Additionally we're in a much better position to issue the barrier command, and we can save a #GP (for privileged instruction emulation) this way. To allow to recover performance, introduce a new VM assist allowing the guest kernel to suppress this barrier. Make availability of the assist dependent upon the command line control, such that kernels have a way to know whether their request actually took any effect. Note that because of its use in PV64_VM_ASSIST_MASK, the declaration of opt_ibpb_mode_switch can't live in asm/spec_ctrl.h. Signed-off-by: Jan Beulich --- Is the placement of the clearing of opt_ibpb_ctxt_switch correct in parse_spec_ctrl()? Shouldn't it live ahead of the "disable_common" label, as being about guest protection, not Xen's? Adding setting of the variable to the "pv" sub-case in parse_spec_ctrl() didn't seem quite right to me, considering that we default it to the opposite of opt_ibpb_entry_pv. --- v4: Correct the print_details() change. Re-base in particular over changes earlier in the series. v3: Leverage exit-IBPB. Introduce separate command line control. v2: Leverage entry-IBPB. Add VM assist. Re-base. --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -2320,8 +2320,8 @@ By default SSBD will be mitigated at run ### spec-ctrl (x86) > `= List of [ , xen=, {pv,hvm}=, > {msr-sc,rsb,md-clear,ibpb-entry}=|{pv,hvm}=, -> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd,psfd, -> eager-fpu,l1d-flush,branch-harden,srb-lock, +> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ibpb-mode-switch, +> ssbd,psfd,eager-fpu,l1d-flush,branch-harden,srb-lock, > unpriv-mmio}= ]` Controls for speculative execution sidechannel mitigations. By default, Xen @@ -2403,7 +2403,10 @@ default. On hardware supporting IBPB (Indirect Branch Prediction Barrier), the `ibpb=` option can be used to force (the default) or prevent Xen from issuing branch -prediction barriers on vcpu context switches. +prediction barriers on vcpu context switches. On such hardware the +`ibpb-mode-switch` option can be used to control whether, by default, Xen +would issue branch prediction barriers when 64-bit PV guests switch from +user to kernel mode. If enabled, guest kernels can op out of this behavior. On all hardware, the `eager-fpu=` option can be used to force or prevent Xen from using fully eager FPU context switches. This is currently implemented as --- a/xen/arch/x86/include/asm/domain.h +++ b/xen/arch/x86/include/asm/domain.h @@ -742,6 +742,8 @@ static inline void pv_inject_sw_interrup pv_inject_event(&event); } +extern int8_t opt_ibpb_mode_switch; + #define PV32_VM_ASSIST_MASK ((1UL << VMASST_TYPE_4gb_segments)| \ (1UL << VMASST_TYPE_4gb_segments_notify) | \ (1UL << VMASST_TYPE_writable_pagetables) | \ @@ -753,7 +755,9 @@ static inline void pv_inject_sw_interrup * but we can't make such requests fail all of the sudden. */ #define PV64_VM_ASSIST_MASK (PV32_VM_ASSIST_MASK | \ - (1UL << VMASST_TYPE_m2p_strict)) + (1UL << VMASST_TYPE_m2p_strict) | \ + ((opt_ibpb_mode_switch + 0UL) << \ + VMASST_TYPE_mode_switch_no_ibpb)) #define HVM_VM_ASSIST_MASK (1UL << VMASST_TYPE_runstate_update_flag) #define arch_vm_assist_valid_mask(d) \ --- a/xen/arch/x86/pv/domain.c +++ b/xen/arch/x86/pv/domain.c @@ -455,6 +455,7 @@ static void _toggle_guest_pt(struct vcpu void toggle_guest_mode(struct vcpu *v) { const struct domain *d = v->domain; +struct cpu_info *cpu_info = get_cpu_info(); unsigned long gs_base; ASSERT(!is_pv_32bit_vcpu(v)); @@ -467,15 +468,21 @@ void toggle_guest_mode(struct vcpu *v) if ( v->arch.flags & TF_kernel_mode ) v->arch.pv.gs_base_kernel = gs_base; else +{ v->arch.pv.gs_base_user = gs_base; + +if ( opt_ibpb_mode_switch && + !(d->arch.spec_ctrl_flags & SCF_entry_ibpb) && + !VM_ASSIST(d, mode_switch_no_ibpb) ) +cpu_info->spec_ctrl_flags |= SCF_new_pred_ctxt; +} + asm volatile ( "swapgs" ); _toggle_guest_pt(v); if ( d->arch.pv.xpti ) { -struct cpu_info *cpu_info = get_cpu_info(); - cpu_info->root_pgt_changed = true; cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) | (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0); --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -60,6 +60,7 @@ bool __ro_after_init opt_ssbd; int8_t __initdata opt_psfd = -1; int8_t __ro_after_init opt_ibpb_ctxt_s
[PATCH] x86/Xen: make use of IBPB controlling VM assist
If this VM assist is available (to PV guests only), use it to - avoid issuing an IBPB ourselves upon entry from user mode (which the hypervisor would then have to emulate, as the MSR write traps), - suppress the IBPB in the hypervisor if we don't mean to have one issued. As there's no good place to have xen_vm_assist_ibpb() as an inline function, make it an init-only out-of-line one. While adjusting the Xen public header, drop the unused and no longer applicable MAX_VMASST_TYPE (instead of modifying its value). Signed-off-by: Jan Beulich --- a/arch/x86/include/asm/xen/hypervisor.h +++ b/arch/x86/include/asm/xen/hypervisor.h @@ -43,6 +43,8 @@ static inline uint32_t xen_cpuid_base(vo return hypervisor_cpuid_base("XenVMMXenVMM", 2); } +int xen_vm_assist_ibpb(bool enable); + struct pci_dev; #ifdef CONFIG_XEN_PV_DOM0 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -18,6 +18,8 @@ #include #include +#include + #include #include #include @@ -32,6 +34,7 @@ #include #include #include +#include #include #include "cpu.h" @@ -934,7 +937,8 @@ do_cmd_auto: break; case RETBLEED_MITIGATION_IBPB: - setup_force_cpu_cap(X86_FEATURE_ENTRY_IBPB); + if (!xen_pv_domain() || xen_vm_assist_ibpb(true)) + setup_force_cpu_cap(X86_FEATURE_ENTRY_IBPB); mitigate_smt = true; break; --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -294,6 +294,17 @@ int xen_panic_handler_init(void) return 0; } +int __init xen_vm_assist_ibpb(bool enable) +{ + /* +* Note that the VM-assist is a disable, so a request to enable IBPB +* on our behalf needs to turn the functionality off (and vice versa). +*/ + return HYPERVISOR_vm_assist(enable ? VMASST_CMD_disable + : VMASST_CMD_enable, + VMASST_TYPE_mode_switch_no_ibpb); +} + void xen_pin_vcpu(int cpu) { static bool disable_pinning; --- a/arch/x86/xen/setup.c +++ b/arch/x86/xen/setup.c @@ -940,6 +940,13 @@ static void __init xen_pvmmu_arch_setup( HYPERVISOR_vm_assist(VMASST_CMD_enable, VMASST_TYPE_pae_extended_cr3); + /* +* By default suppress the hypervisor issuing IBPB on our behalf. In +* the RETBLEED_MITIGATION_IBPB case the VM assist will be disengaged +* again in retbleed_select_mitigation(). +*/ + xen_vm_assist_ibpb(false); + if (register_callback(CALLBACKTYPE_event, xen_asm_exc_xen_hypervisor_callback) || register_callback(CALLBACKTYPE_failsafe, xen_failsafe_callback)) --- a/include/xen/interface/xen.h +++ b/include/xen/interface/xen.h @@ -413,7 +413,15 @@ DEFINE_GUEST_HANDLE_STRUCT(mmuext_op); */ #define VMASST_TYPE_runstate_update_flag 5 -#define MAX_VMASST_TYPE 5 +/* + * x86-64 guests: Suppress IBPB on guest-user to guest-kernel mode switch. + * + * By default (on affected and capable hardware) as a safety measure Xen, + * to cover for the fact that guest-kernel and guest-user modes are both + * running in ring 3 (and hence share prediction context), would issue a + * barrier for user->kernel mode switches of PV guests. + */ +#define VMASST_TYPE_mode_switch_no_ibpb 33 #ifndef __ASSEMBLY__
Re: [RFC 05/10] x86/traps: guard vmx specific functions with INTEL_VMX
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > The functions vmx_vmcs_enter() and vmx_vmcs_exit() are VT-x specific. > Guard their calls with INTEL_VMX. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou With whatever the final name of the Kconfig control is going to be Acked-by: Jan Beulich Jan
Re: [PATCH v1 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
Hi Jan! Thanks a lot for starting the review of the patch series! On Mon, 2023-02-13 at 13:24 +0100, Jan Beulich wrote: > On 03.02.2023 18:05, Oleksii Kurochko wrote: > > --- a/xen/common/Kconfig > > +++ b/xen/common/Kconfig > > @@ -92,6 +92,12 @@ config STATIC_MEMORY > > > > If unsure, say N. > > > > +config GENERIC_DO_BUG_FRAME > > + bool > > + help > > + Generic do_bug_frame() function is needed to handle the > > type of bug > > + frame and print an information about it. > > Generally help text for prompt-less functions is not very useful. > Assuming > it is put here to inform people actually reading the source file, I'm > okay > for it to be left here, but please at least drop the stray "an". I > also > think this may want moving up in the file, e.g. ahead of all the > HAS_* > controls (which, as you will notice, all have no help text either). > Plus > finally how about shorter and more applicable GENERIC_BUG_FRAME - > after > all what becomes generic is more than just do_bug_frame()? Thanks for the comments. I will take them into account. Right now only do_bug_frame() is expected to be generic. > > > --- /dev/null > > +++ b/xen/common/bug.c > > @@ -0,0 +1,88 @@ > > +#include > > +#include > > +#include > > +#include > > Please order headers alphabetically unless there's anything > preventing > that from being done. Sure, thanks. > > > +#include > > +#include > > + > > +#include > > + > > +int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc) > > I know Arm is using vaddr_t and RISC-V now also has it, but in UNIX- > like > environments that's redundant with "unsigned long", and it's also > redundant with C99's uintptr_t. Hence when seeing the type I always > wonder whether it's really a host virtual address which is meant (and > not perhaps a guest one, which in principle could differ from the > host > one for certain guest types). In any event the existence of this type > should imo not be a prereq to using this generic piece of > infrastructure. > I am OK with changing vaddr_t to 'unsigned long' and agree with your point so if no one is against that I'll change it. > > +{ > > + const struct bug_frame *bug = NULL; > > + const char *prefix = "", *filename, *predicate; > > + unsigned long fixup; > > + int id = -1, lineno; > > For both of these I question them needing to be of a signed type. I took the code from ARM but it looks like the type of id and lineno can be changed to 'unsgined int'. So I'll update the code correspondingly. > > > + const struct virtual_region *region; > > + > > + region = find_text_region(pc); > > + if ( region ) > > + { > > + for ( id = 0; id < BUGFRAME_NR; id++ ) > > + { > > + const struct bug_frame *b; > > + unsigned int i; > > + > > + for ( i = 0, b = region->frame[id].bugs; > > + i < region->frame[id].n_bugs; b++, i++ ) > > + { > > + if ( ((vaddr_t)bug_loc(b)) == pc ) > > Afaics this is the sole use of bug_loc(). If so, better change the > macro > to avoid the need for a cast here: > > #define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp) > Thanks for the recommendation. It makes sense to change the macro so I'll update it too. > > --- /dev/null > > +++ b/xen/include/xen/bug.h > > @@ -0,0 +1,127 @@ > > +#ifndef __XEN_BUG_H__ > > +#define __XEN_BUG_H__ > > + > > +#define BUG_DISP_WIDTH 24 > > +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH) > > +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH) > > + > > +#define BUGFRAME_run_fn 0 > > +#define BUGFRAME_warn 1 > > +#define BUGFRAME_bug 2 > > +#define BUGFRAME_assert 3 > > + > > +#define BUGFRAME_NR 4 > > + > > +#ifndef __ASSEMBLY__ > > + > > +#include > > +#include > > +#include > > +#include > > Again - please sort headers. > > > +#include > > + > > +#ifndef BUG_FRAME_STUFF > > +struct bug_frame { > > Please can we have a blank line between the above two ones and then > similarly > ahead of the #endif? Sure. > > > + signed int loc_disp; /* Relative address to the bug address > > */ > > + signed int file_disp; /* Relative address to the filename */ > > + signed int msg_disp; /* Relative address to the predicate > > (for ASSERT) */ > > + uint16_t line; /* Line number */ > > + uint32_t pad0:16; /* Padding for 8-bytes align */ > > Already the original comment in Arm code is wrong: The padding > doesn't > guarantee 8-byte alignment; it merely guarantees a multiple of 8 > bytes > size. Aiui there's also no need for 8-byte alignment anywhere here > (in > fact there's ".p2align 2" in the generator macros). > > I also wonder why this needs to be a named bitfield: Either make it > plain uint16_t, or if you use a bitfield, then omit the name. > It will better to change it to 'uint16_t' as I don't see any reason to use 'uint32_t' with bitfield here. I'll check if we need the alignment. If
Re: [RFC 06/10] x86/domain: guard svm specific functions with AMD_SVM
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > The functions svm_load_segs() and svm_load_segs_prefetch() are AMD-V specific > so guard their calls in common code with AMD_SVM. > > Since AMD_SVM depends on HVM, it can be used alone. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou With whatever the final name of the Kconfig control is going to be Acked-by: Jan Beulich Thinking about it, both here an in the earlier patch it may be worth considering to switch to use of IS_ENABLED() while making these adjustments. Jan
Re: [RFC 07/10] x86/oprofile: guard svm specific symbols with AMD_SVM
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > The symbol svm_stgi_label is AMD-V specific so guard its usage in common code > with AMD_SVM. > > Since AMD_SVM depends on HVM, it can be used alone. > Also, use #ifdef instead of #if. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou With whatever the final name of the Kconfig control is going to be Acked-by: Jan Beulich Jan
Re: [PATCH v1 2/4] xen/arm: switch ARM to use generic implementation of bug.h
On Mon, 2023-02-13 at 14:02 +0100, Jan Beulich wrote: > On 03.02.2023 18:05, Oleksii Kurochko wrote: > > --- a/xen/arch/arm/Kconfig > > +++ b/xen/arch/arm/Kconfig > > @@ -18,6 +18,7 @@ config ARM > > select HAS_PDX > > select HAS_PMAP > > select IOMMU_FORCE_PT_SHARE > > + select GENERIC_DO_BUG_FRAME > > Please maintain (largely?) alphabetic ordering here. Sure. I'll do that in the next version of the patch series. > > Jan > ~ Oleksii
Re: [RFC 08/10] x86: wire cpu_has_svm/vmx_* to false when svm/vmx not enabled
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > To be able to use cpu_has_svm/vmx_* macros in common code without enclosing Both in the title and here the spelling you use made me first think that you talk about "cpu_has_svm". May I suggest you follow what we typically do and use "cpu_has_{svm,vmx}_*" instead in such a case? However, the title may want changing anyway: > --- a/xen/arch/x86/include/asm/hvm/svm/svm.h > +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h > @@ -80,6 +80,7 @@ extern u32 svm_feature_flags; > #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */ > #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */ > > +#ifdef CONFIG_AMD_SVM > #define cpu_has_svm_feature(f) (svm_feature_flags & (1u << (f))) Why don't you simply provide a 2nd form of this one macro, leaving all others alone? > --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h > +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h > @@ -294,6 +294,7 @@ extern u64 vmx_ept_vpid_cap; > > #define VMX_TSC_MULTIPLIER_MAX 0xULL > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_wbinvd_exiting \ > (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING) > #define cpu_has_vmx_virtualize_apic_accesses \ > @@ -352,6 +353,36 @@ extern u64 vmx_ept_vpid_cap; > (vmx_secondary_exec_control & SECONDARY_EXEC_BUS_LOCK_DETECTION) > #define cpu_has_vmx_notify_vm_exiting \ > (vmx_secondary_exec_control & SECONDARY_EXEC_NOTIFY_VM_EXITING) > +#else > +#define cpu_has_wbinvd_exiting false > +#define cpu_has_vmx_virtualize_apic_accesses false > +#define cpu_has_vmx_tpr_shadow false > +#define cpu_has_vmx_vnmi false > +#define cpu_has_vmx_msr_bitmap false > +#define cpu_has_vmx_secondary_exec_control false > +#define cpu_has_vmx_ept false > +#define cpu_has_vmx_dt_exiting false > +#define cpu_has_vmx_vpid false > +#define cpu_has_monitor_trap_flag false > +#define cpu_has_vmx_pat false > +#define cpu_has_vmx_efer false > +#define cpu_has_vmx_unrestricted_guest false > +#define vmx_unrestricted_guest(v) false > +#define cpu_has_vmx_ple false > +#define cpu_has_vmx_apic_reg_virt false > +#define cpu_has_vmx_virtual_intr_delivery false > +#define cpu_has_vmx_virtualize_x2apic_mode false > +#define cpu_has_vmx_posted_intr_processing false > +#define cpu_has_vmx_vmcs_shadowing false > +#define cpu_has_vmx_vmfunc false > +#define cpu_has_vmx_virt_exceptions false > +#define cpu_has_vmx_pml false > +#define cpu_has_vmx_mpx false > +#define cpu_has_vmx_xsaves false > +#define cpu_has_vmx_tsc_scaling false > +#define cpu_has_vmx_bus_lock_detection false > +#define cpu_has_vmx_notify_vm_exiting false > +#endif /* CONFIG_INTEL_VMX */ For VMX you first of all want to separate out vmx_unrestricted_guest(v). Maybe we can even get away without a 2nd form. Then I think it would be much neater a change if, like we have for SVM, a couple (three I think) helper macros were introduced, which then is all that needs providing a 2nd form for. (Both steps may want doing in separate, prereq patches.) > @@ -374,8 +405,12 @@ extern u64 vmx_ept_vpid_cap; > #define VMX_BASIC_DEFAULT1_ZERO (1ULL << 55) > > extern u64 vmx_basic_msr; > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_ins_outs_instr_info \ > (!!(vmx_basic_msr & VMX_BASIC_INS_OUT_INFO)) > +#else > +#define cpu_has_vmx_ins_outs_instr_info false > +#endif /* CONFIG_INTEL_VMX */ I don't think you need an alternative here - it's used solely in VMX code. If one wanted to this could even be moved to vmcs.c. > --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h > +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h > @@ -289,6 +289,7 @@ typedef union cr_access_qual { > > extern uint8_t posted_intr_vector; > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_ept_exec_only_supported\ > (vmx_ept_vpid_cap & VMX_EPT_EXEC_ONLY_SUPPORTED) > > @@ -301,6 +302,17 @@ extern uint8_t posted_intr_vector; > #define cpu_has_vmx_ept_ad(vmx_ept_vpid_cap & VMX_EPT_AD_BIT) > #define cpu_has_vmx_ept_invept_single_context \ > (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT) > +#else > +#define cpu_has_vmx_ept_exec_only_supported false > + > +#define cpu_has_vmx_ept_wl4_supported false > +#define cpu_has_vmx_ept_mt_uc false > +#define cpu_has_vmx_ept_mt_wb false > +#define cpu_has_vmx_ept_2mb false > +#define cpu_has_vmx_ept_1gb false > +#define cpu_has_vmx_ept_ad false > +#define cpu_has_vmx_ept_invept_single_context false > +#endif /* CONFIG_INTEL_VMX */ Same suggestion for introducing a helper macro here, which would then also be usable ... > @@ -310,12 +322,18 @@ extern uint8_t posted_intr_vector; > #define INVEPT_SINGLE_CONTEXT 1 > #define INVEPT_ALL_CONTEXT 2 > > +#ifdef CONFIG_INTEL_VMX > #define cpu_has_vmx_vpid_invvpid_individual_addr\ > (vmx_ept_vpid_cap & VMX_VPID_INVVPID_INDIVIDUAL_ADDR) > #define cpu_has_vmx_vpid_invvpid_single_context \ > (vmx_ept_vpi
Re: [PATCH v1 3/4] xen/x86: switch x86 to use generic implemetation of bug.h
On Mon, 2023-02-13 at 14:10 +0100, Jan Beulich wrote: > On 03.02.2023 18:05, Oleksii Kurochko wrote: > > Signed-off-by: Oleksii Kurochko > > Is there anything keeping x86 from also using the generic > do_bug_frame()? > If not, switching over would then likely mean no need for the new > Kconfig > control. > Actually, it seems that it is possible to re-use bug_frame in x86 code too. Looking at lines 1188 - 1264 [1] they are mostly the same [2] except for updating of eip [3], processing of BUGFRAME_bug - was added debugger_trap_fatal [4] and multiple usages of fixup_exception_return() [5]. But all this stuff can be processed outside do_bug_frame() function... [1] https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1188 [2] https://gitlab.com/xen-project/people/olkur/xen/-/blob/generic-bug-h/xen/common/bug.c#L10 [3]https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1211 [4] https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1244 [5] https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1217 > Jan
Re: [PATCH v1 4/4] xen: change to
On Mon, 2023-02-13 at 14:13 +0100, Jan Beulich wrote: > On 03.02.2023 18:05, Oleksii Kurochko wrote: > > Since the generic version of bug.h stuff was introduced it > > is necessary to rename all uses of to > > > > Signed-off-by: Oleksii Kurochko > > Doesn't this change need to come ahead of at least what currently is > patch 3? > Or else why do you say "necessary" (I take this to mean the build > otherwise > is broken)? If the build works after patch 3, the change here may > want to be > drop the unnecessary asm/bug.h includes instead. > I'll double-check if the build works after patch 3 and fix the comment. Thanks for the comment. > Jan ~Oleksii
Re: [RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX
On 13.02.2023 15:57, Xenia Ragiadakou wrote: > VIO_realmode_completion is specific to vmx realmode, so guard the completion > handling code with INTEL_VMX. While it means slightly bigger a code change, personally I'd prefer if VIO_realmode_completion simply didn't exist as enumerator when !VMX. Besides ... > --- a/xen/arch/x86/hvm/ioreq.c > +++ b/xen/arch/x86/hvm/ioreq.c > @@ -44,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion > completion) > { > switch ( completion ) > { > +#ifdef CONFIG_INTEL_VMX > case VIO_realmode_completion: > { > struct hvm_emulate_ctxt ctxt; > @@ -54,6 +55,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion > completion) > > break; > } > +#endif ... this use there's exactly one further one which would need #ifdef-ing. I think that's tolerable. Thinking further (not necessarily for you to carry out) the x86 version is identical to Arm's when !VMX, so it would look better to me if the entire x86 function became conditional, the Arm one was dropped, and common/ioreq.c provided a fallback for the case where no arch one exists. Jan
Re: [RFC 08/10] x86: wire cpu_has_svm/vmx_* to false when svm/vmx not enabled
On 2/14/23 18:36, Jan Beulich wrote: On 13.02.2023 15:57, Xenia Ragiadakou wrote: To be able to use cpu_has_svm/vmx_* macros in common code without enclosing Both in the title and here the spelling you use made me first think that you talk about "cpu_has_svm". May I suggest you follow what we typically do and use "cpu_has_{svm,vmx}_*" instead in such a case? However, the title may want changing anyway: Ok, I will use the conventional way from now on. --- a/xen/arch/x86/include/asm/hvm/svm/svm.h +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h @@ -80,6 +80,7 @@ extern u32 svm_feature_flags; #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */ #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */ +#ifdef CONFIG_AMD_SVM #define cpu_has_svm_feature(f) (svm_feature_flags & (1u << (f))) Why don't you simply provide a 2nd form of this one macro, leaving all others alone? You are right. That way there will be less changes. --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h @@ -294,6 +294,7 @@ extern u64 vmx_ept_vpid_cap; #define VMX_TSC_MULTIPLIER_MAX 0xULL +#ifdef CONFIG_INTEL_VMX #define cpu_has_wbinvd_exiting \ (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING) #define cpu_has_vmx_virtualize_apic_accesses \ @@ -352,6 +353,36 @@ extern u64 vmx_ept_vpid_cap; (vmx_secondary_exec_control & SECONDARY_EXEC_BUS_LOCK_DETECTION) #define cpu_has_vmx_notify_vm_exiting \ (vmx_secondary_exec_control & SECONDARY_EXEC_NOTIFY_VM_EXITING) +#else +#define cpu_has_wbinvd_exiting false +#define cpu_has_vmx_virtualize_apic_accesses false +#define cpu_has_vmx_tpr_shadow false +#define cpu_has_vmx_vnmi false +#define cpu_has_vmx_msr_bitmap false +#define cpu_has_vmx_secondary_exec_control false +#define cpu_has_vmx_ept false +#define cpu_has_vmx_dt_exiting false +#define cpu_has_vmx_vpid false +#define cpu_has_monitor_trap_flag false +#define cpu_has_vmx_pat false +#define cpu_has_vmx_efer false +#define cpu_has_vmx_unrestricted_guest false +#define vmx_unrestricted_guest(v) false +#define cpu_has_vmx_ple false +#define cpu_has_vmx_apic_reg_virt false +#define cpu_has_vmx_virtual_intr_delivery false +#define cpu_has_vmx_virtualize_x2apic_mode false +#define cpu_has_vmx_posted_intr_processing false +#define cpu_has_vmx_vmcs_shadowing false +#define cpu_has_vmx_vmfunc false +#define cpu_has_vmx_virt_exceptions false +#define cpu_has_vmx_pml false +#define cpu_has_vmx_mpx false +#define cpu_has_vmx_xsaves false +#define cpu_has_vmx_tsc_scaling false +#define cpu_has_vmx_bus_lock_detection false +#define cpu_has_vmx_notify_vm_exiting false +#endif /* CONFIG_INTEL_VMX */ For VMX you first of all want to separate out vmx_unrestricted_guest(v). Maybe we can even get away without a 2nd form. Then I think it would be much neater a change if, like we have for SVM, a couple (three I think) helper macros were introduced, which then is all that needs providing a 2nd form for. (Both steps may want doing in separate, prereq patches.) Ok will do. @@ -374,8 +405,12 @@ extern u64 vmx_ept_vpid_cap; #define VMX_BASIC_DEFAULT1_ZERO (1ULL << 55) extern u64 vmx_basic_msr; +#ifdef CONFIG_INTEL_VMX #define cpu_has_vmx_ins_outs_instr_info \ (!!(vmx_basic_msr & VMX_BASIC_INS_OUT_INFO)) +#else +#define cpu_has_vmx_ins_outs_instr_info false +#endif /* CONFIG_INTEL_VMX */ I don't think you need an alternative here - it's used solely in VMX code. If one wanted to this could even be moved to vmcs.c. Ok. I ll take a closer look at this one. --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h @@ -289,6 +289,7 @@ typedef union cr_access_qual { extern uint8_t posted_intr_vector; +#ifdef CONFIG_INTEL_VMX #define cpu_has_vmx_ept_exec_only_supported\ (vmx_ept_vpid_cap & VMX_EPT_EXEC_ONLY_SUPPORTED) @@ -301,6 +302,17 @@ extern uint8_t posted_intr_vector; #define cpu_has_vmx_ept_ad(vmx_ept_vpid_cap & VMX_EPT_AD_BIT) #define cpu_has_vmx_ept_invept_single_context \ (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT) +#else +#define cpu_has_vmx_ept_exec_only_supported false + +#define cpu_has_vmx_ept_wl4_supported false +#define cpu_has_vmx_ept_mt_uc false +#define cpu_has_vmx_ept_mt_wb false +#define cpu_has_vmx_ept_2mb false +#define cpu_has_vmx_ept_1gb false +#define cpu_has_vmx_ept_ad false +#define cpu_has_vmx_ept_invept_single_context false +#endif /* CONFIG_INTEL_VMX */ Same suggestion for introducing a helper macro here, which would then also be usable ... @@ -310,12 +322,18 @@ extern uint8_t posted_intr_vector; #define INVEPT_SINGLE_CONTEXT 1 #define INVEPT_ALL_CONTEXT 2 +#ifdef CONFIG_INTEL_VMX #define cpu_has_vmx_vpid_invvpid_individual_addr\ (vmx_ept_vpid_cap & VMX_VPID_INVVPID_INDIVIDUAL_ADDR) #define cpu_has_vmx
Re: [PATCH v1 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
On 14.02.2023 17:22, Oleksii wrote: > On Mon, 2023-02-13 at 13:24 +0100, Jan Beulich wrote: >> On 03.02.2023 18:05, Oleksii Kurochko wrote: >>> --- a/xen/common/Kconfig >>> +++ b/xen/common/Kconfig >>> @@ -92,6 +92,12 @@ config STATIC_MEMORY >>> >>> If unsure, say N. >>> >>> +config GENERIC_DO_BUG_FRAME >>> + bool >>> + help >>> + Generic do_bug_frame() function is needed to handle the >>> type of bug >>> + frame and print an information about it. >> >> Generally help text for prompt-less functions is not very useful. >> Assuming >> it is put here to inform people actually reading the source file, I'm >> okay >> for it to be left here, but please at least drop the stray "an". I >> also >> think this may want moving up in the file, e.g. ahead of all the >> HAS_* >> controls (which, as you will notice, all have no help text either). >> Plus >> finally how about shorter and more applicable GENERIC_BUG_FRAME - >> after >> all what becomes generic is more than just do_bug_frame()? > Thanks for the comments. I will take them into account. > Right now only do_bug_frame() is expected to be generic. Hmm, do you mean to undo part of what you've done? I didn't think you would. Yet in what you've done so far, the struct an several macros are also generalized. Hence the "DO" in the name is too narrow from my pov. >>> --- /dev/null >>> +++ b/xen/include/xen/bug.h >>> @@ -0,0 +1,127 @@ >>> +#ifndef __XEN_BUG_H__ >>> +#define __XEN_BUG_H__ >>> + >>> +#define BUG_DISP_WIDTH 24 >>> +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH) >>> +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH) >>> + >>> +#define BUGFRAME_run_fn 0 >>> +#define BUGFRAME_warn 1 >>> +#define BUGFRAME_bug 2 >>> +#define BUGFRAME_assert 3 >>> + >>> +#define BUGFRAME_NR 4 >>> + >>> +#ifndef __ASSEMBLY__ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >> >> Again - please sort headers. >> >>> +#include >>> + >>> +#ifndef BUG_FRAME_STUFF >>> +struct bug_frame { >> >> Please can we have a blank line between the above two ones and then >> similarly >> ahead of the #endif? > Sure. > >> >>> + signed int loc_disp; /* Relative address to the bug address >>> */ >>> + signed int file_disp; /* Relative address to the filename */ >>> + signed int msg_disp; /* Relative address to the predicate >>> (for ASSERT) */ >>> + uint16_t line; /* Line number */ >>> + uint32_t pad0:16; /* Padding for 8-bytes align */ >> >> Already the original comment in Arm code is wrong: The padding >> doesn't >> guarantee 8-byte alignment; it merely guarantees a multiple of 8 >> bytes >> size. Aiui there's also no need for 8-byte alignment anywhere here >> (in >> fact there's ".p2align 2" in the generator macros). >> >> I also wonder why this needs to be a named bitfield: Either make it >> plain uint16_t, or if you use a bitfield, then omit the name. >> > It will better to change it to 'uint16_t' as I don't see any reason to > use 'uint32_t' with bitfield here. > I'll check if we need the alignment. If there is '.p2align 2' we > really don't need it. See Julien's replies any my responses: FTAOD I did _not_ ask to remove the field. >>> +}; >>> + >>> +#define bug_loc(b) ((const void *)(b) + (b)->loc_disp) >>> +#define bug_file(b) ((const void *)(b) + (b)->file_disp); >>> +#define bug_line(b) ((b)->line) >>> +#define bug_msg(b) ((const char *)(b) + (b)->msg_disp) >>> +#endif /* BUG_FRAME_STUFF */ >>> + >>> +#ifndef BUG_FRAME >>> +/* Many versions of GCC doesn't support the asm %c parameter which >>> would >>> + * be preferable to this unpleasantness. We use mergeable string >>> + * sections to avoid multiple copies of the string appearing in >>> the >>> + * Xen image. BUGFRAME_run_fn needs to be handled separately. >>> + */ >> >> When generalizing the logic, I wonder in how far the comment doesn't >> want re-wording some. For example, while Arm prefixes constant insn >> operands with # (and x86 uses $), there's no such prefix in RISC-V. >> At >> which point there's no need to use %c in the first place. (Which in >> turn means x86'es more compact representation may also be usable >> there. >> And yet in turn the question arises whether RISC-V wouldn't better >> have >> its own derivation of the machinery, rather than trying to generalize >> things. RISC-V's would then likely be closer to x86'es, just without >> the use of %c on asm() operands. Which might then suggest to rather >> generalize x86'es variant down the road.) > ARM version is more portable because of the '%c' modifier which is not > present everywhere, so I decided to use it as a generic implementation. > Moreover I don't see any reason why we can't switch x86 implementation > to 'generic/ARM'. That would increase data size on x86 for no gain, from all I can tell. >>> + ".hword " __stringify(line) ", >>> 0\n" \ >> >> Hmm, .hword. How generic is support for that in assemblers? I noti
Re: [RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX
On 2/14/23 18:46, Jan Beulich wrote: On 13.02.2023 15:57, Xenia Ragiadakou wrote: VIO_realmode_completion is specific to vmx realmode, so guard the completion handling code with INTEL_VMX. While it means slightly bigger a code change, personally I'd prefer if VIO_realmode_completion simply didn't exist as enumerator when !VMX. Besides ... --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -44,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) { switch ( completion ) { +#ifdef CONFIG_INTEL_VMX case VIO_realmode_completion: { struct hvm_emulate_ctxt ctxt; @@ -54,6 +55,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) break; } +#endif ... this use there's exactly one further one which would need #ifdef-ing. I think that's tolerable. Sure. I will fix it. Thinking further (not necessarily for you to carry out) the x86 version is identical to Arm's when !VMX, so it would look better to me if the entire x86 function became conditional, the Arm one was dropped, and common/ioreq.c provided a fallback for the case where no arch one exists. It may look awkward to have arch_vcpu_ioreq_completion() in VMX guards in common ioreq code because the connection is not obvious immediately. Jan -- Xenia
Re: [RFC 06/10] x86/domain: guard svm specific functions with AMD_SVM
On 2/14/23 18:24, Jan Beulich wrote: On 13.02.2023 15:57, Xenia Ragiadakou wrote: The functions svm_load_segs() and svm_load_segs_prefetch() are AMD-V specific so guard their calls in common code with AMD_SVM. Since AMD_SVM depends on HVM, it can be used alone. No functional change intended. Signed-off-by: Xenia Ragiadakou With whatever the final name of the Kconfig control is going to be Acked-by: Jan Beulich Thinking about it, both here an in the earlier patch it may be worth considering to switch to use of IS_ENABLED() while making these adjustments. Ok will do. Thanks. Jan -- Xenia
Re: [PATCH] x86/hotplug: Remove incorrect comment about mwait_play_dead()
Hi, On 1/27/23 4:37 PM, Srivatsa S. Bhat wrote: > From: "Srivatsa S. Bhat (VMware)" > > The comment that says mwait_play_dead() returns only on failure is a > bit misleading because mwait_play_dead() could actually return for > valid reasons (such as mwait not being supported by the platform) that > do not indicate a failure of the CPU offline operation. So, remove the > comment. > > Suggested-by: Thomas Gleixner > Signed-off-by: Srivatsa S. Bhat (VMware) Gentle ping for review. Thank you! Regards, Srivatsa > --- > arch/x86/kernel/smpboot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index 55cad72715d9..9013bb28255a 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -1833,7 +1833,7 @@ void native_play_dead(void) > play_dead_common(); > tboot_shutdown(TB_SHUTDOWN_WFS); > > - mwait_play_dead(); /* Only returns on failure */ > + mwait_play_dead(); > if (cpuidle_play_dead()) > hlt_play_dead(); > } >
Re: [PATCH v2] automation: Add container and build jobs to run cppcheck analysis
On 14/02/2023 3:39 pm, Michal Orzel wrote: > diff --git a/automation/build/debian/unstable-cppcheck.dockerfile > b/automation/build/debian/unstable-cppcheck.dockerfile > new file mode 100644 > index ..54b99f87dfec > --- /dev/null > +++ b/automation/build/debian/unstable-cppcheck.dockerfile > @@ -0,0 +1,37 @@ > +FROM arm64v8/debian:unstable > +LABEL maintainer.name="The Xen Project" \ > + maintainer.email="xen-devel@lists.xenproject.org" > + > +ENV DEBIAN_FRONTEND=noninteractive > +ENV CPPCHECK_VERSION=2.7 > +ENV USER root > + > +RUN mkdir /build > +WORKDIR /build > + > +# dependencies for cppcheck and Xen-only build/cross-build > +RUN apt-get update && \ > +apt-get --quiet --yes install \ > +build-essential \ > +curl \ > +python-is-python3 \ > +libpcre3-dev \ > +flex \ > +bison \ > +gcc-arm-linux-gnueabihf \ > +gcc-x86-64-linux-gnu > + > +# cppcheck release build (see cppcheck readme.md) > +RUN curl -fsSLO > https://github.com/danmar/cppcheck/archive/"$CPPCHECK_VERSION".tar.gz && \ > +tar xvzf "$CPPCHECK_VERSION".tar.gz && \ > +cd cppcheck-"$CPPCHECK_VERSION" && \ > +make install -j$(nproc) \ > +MATCHCOMPILER=yes \ > +FILESDIR=/usr/share/cppcheck \ > +HAVE_RULES=yes CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare > -Wno-unused-function" I think you want to be using a mutli-FROM dockerfile here, otherwise you're including all the intermediate build artefacts in the final image. See debian/buster-gcc-ibt.dockerfile for an example. That said, I'm not sure we want to be making custom containers for every minor tweak we have on a build environment. What's wrong with just putting CPPCHECK in the normal container? ~Andrew
Re: [PATCH v2] automation: Add container and build jobs to run cppcheck analysis
On Tue, 14 Feb 2023, Andrew Cooper wrote: > On 14/02/2023 3:39 pm, Michal Orzel wrote: > > diff --git a/automation/build/debian/unstable-cppcheck.dockerfile > > b/automation/build/debian/unstable-cppcheck.dockerfile > > new file mode 100644 > > index ..54b99f87dfec > > --- /dev/null > > +++ b/automation/build/debian/unstable-cppcheck.dockerfile > > @@ -0,0 +1,37 @@ > > +FROM arm64v8/debian:unstable > > +LABEL maintainer.name="The Xen Project" \ > > + maintainer.email="xen-devel@lists.xenproject.org" > > + > > +ENV DEBIAN_FRONTEND=noninteractive > > +ENV CPPCHECK_VERSION=2.7 > > +ENV USER root > > + > > +RUN mkdir /build > > +WORKDIR /build > > + > > +# dependencies for cppcheck and Xen-only build/cross-build > > +RUN apt-get update && \ > > +apt-get --quiet --yes install \ > > +build-essential \ > > +curl \ > > +python-is-python3 \ > > +libpcre3-dev \ > > +flex \ > > +bison \ > > +gcc-arm-linux-gnueabihf \ > > +gcc-x86-64-linux-gnu > > + > > +# cppcheck release build (see cppcheck readme.md) > > +RUN curl -fsSLO > > https://github.com/danmar/cppcheck/archive/"$CPPCHECK_VERSION".tar.gz && \ > > +tar xvzf "$CPPCHECK_VERSION".tar.gz && \ > > +cd cppcheck-"$CPPCHECK_VERSION" && \ > > +make install -j$(nproc) \ > > +MATCHCOMPILER=yes \ > > +FILESDIR=/usr/share/cppcheck \ > > +HAVE_RULES=yes CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare > > -Wno-unused-function" > > I think you want to be using a mutli-FROM dockerfile here, otherwise > you're including all the intermediate build artefacts in the final image. > > See debian/buster-gcc-ibt.dockerfile for an example. > > That said, I'm not sure we want to be making custom containers for every > minor tweak we have on a build environment. What's wrong with just > putting CPPCHECK in the normal container? cppcheck is not large but needs to be built from source (as part of the Dockerfile). So we thought it would be best to keep it separate from the regular containers. I don't foresee another case like cppcheck at the moment. Also by having it separate it is clearer that this container is "special". I think it would be preferable to keep it in its own separate container but it would be OK either way.
Xen Security Advisory 426 v1 (CVE-2022-27672) - x86: Cross-Thread Return Address Predictions
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory CVE-2022-27672 / XSA-426 x86: Cross-Thread Return Address Predictions ISSUE DESCRIPTION = It has been discovered that on some AMD CPUs, the RAS (Return Address Stack, also called RAP - Return Address Predictor - in some AMD documentation, and RSB - Return Stack Buffer - in Intel terminology) is dynamically partitioned between non-idle threads. This allows an attacker to control speculative execution on the adjacent thread. For more details, see: https://www.amd.com/en/corporate/product-security/bulletin/amd-sb-1045 IMPACT == An attacker might be able to infer the contents of arbitrary host memory, including memory assigned to other guests. VULNERABLE SYSTEMS == Only AMD CPUs are known to be potentially vulnerable. CPUs from other hardware vendors are not believed to be impacted. Only the Zen1 and Zen2 microarchitectures are believed to be potentially vulnerable. Other microarchitectures are not believed to be vulnerable. Only configurations with SMT activate are potentially vulnerable. If SMT is disabled by the firmware, or at runtime with `smt=0` on Xen's command line, then the platform is not vulnerable. Xen 4.17 and later contains an optimisation, specifically: c/s afab477fba3b ("x86/spec-ctrl: Skip RSB overwriting when safe to do so") which in combination with disabling 32bit PV guests (either at compile time with CONFIG_PV32=n, or at runtime with `pv=no-32` on the command line) renders Xen vulnerable to attack from PV guests. Note: multiple downstreams are known to have backported this optimisation to older versions of Xen. Consult your software vendor documentation. MITIGATION == On otherwise-vulnerable configurations, the issue can be mitigated by booting Xen with `spec-ctrl=rsb`, which will override the aforementioned optimisation. Alternatively, SMT can be disabled either in the firmware, or by booting Xen with `smt=0`. Alternatively, if 32bit PV guests are only runtime disabled in Xen, this issue can also be mitigated by booting Xen with `pv=32` to enable support 32bit PV guests. It is not necessary for a 32bit PV guest to actually be running in order to mitigate the issue. RESOLUTION == Applying the attached patch resolves this issue. Note that patches for released versions are generally prepared to apply to the stable branches, and may not apply cleanly to the most recent release tarball. Downstreams are encouraged to update to the tip of the stable branch before applying these patches. xsa426.patch xen-unstable - Xen 4.17 $ sha256sum xsa426* 425b1d8931e02852afec9fe3d9f1d009f6d8a33c6387b2e8b3896f374732d470 xsa426.patch $ -BEGIN PGP SIGNATURE- iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmPrzJoMHHBncEB4ZW4u b3JnAAoJEIP+FMlX6CvZiqsIALrisP3l7ImoKe49Bmb1blNYmUv6UjYGdVF9acc9 ++QYPLq4Mu+kJuIlgKnT21hj7BFczL4KSi8sVw/nLqU3x8R/ZJ6nxXLlCod6RqGw 4MYd6QmArx8a+hm3LC0288VEFVXFh0WTDA6PK15RkspiwcjsAZ4w7DA7cRk0FLP0 9KJMhSPOAj9wCDhvOckr7DnA+D6gOKjMH83NCL0rg6Xe8+Bv0qTVYe49FqAnbWwc 9RsYOKfRuZUci+Z+mALVRB97R7xvns5D9HnDvs55ADri506JWkxmdp1GvLtjezXV 3Zds6TOrr1i0RQGV9M6aouinrI+DQNrOFR8V6p98KYxAo+Y= =T8Uh -END PGP SIGNATURE- xsa426.patch Description: Binary data
Re: [RFC 01/10] x86: introduce AMD-V and Intel VT-x Kconfig options
On 2/14/23 2:48 AM, Jan Beulich wrote: On 13.02.2023 21:53, Boris Ostrovsky wrote: On 2/13/23 11:41 AM, Jan Beulich wrote: On 13.02.2023 17:30, Xenia Ragiadakou wrote: On 2/13/23 17:11, Jan Beulich wrote: On 13.02.2023 15:57, Xenia Ragiadakou wrote: --- a/xen/arch/x86/cpu/Makefile +++ b/xen/arch/x86/cpu/Makefile @@ -10,4 +10,6 @@ obj-y += intel.o obj-y += intel_cacheinfo.o obj-y += mwait-idle.o obj-y += shanghai.o -obj-y += vpmu.o vpmu_amd.o vpmu_intel.o +obj-y += vpmu.o +obj-$(CONFIG_AMD_SVM) += vpmu_amd.o +obj-$(CONFIG_INTEL_VMX) += vpmu_intel.o This code was moved from hvm/ to cpu/ for the specific reason of also being used by PV guests. (Sadly the comments at the top weren't corrected at that time.) Hmm, the init functions are prefixed with svm/vmx. Does vpmu depend on svm/vmx support? There are interactions, but I don't think there's a dependency. You may want to ask Boris (whom I have now added to Cc). MSR intercept bits need to be manipulated, which is SVM/VMX-specific. But that's "interaction", not "dependency" aiui: The intercept bits aren't relevant for PV guests, are they? Correct, they are not. All accesses to intercept bits are under is_hvm_vcpu(). So vpmu does not depend on presence of vmx/svm support. (And init routines shouldn't be prefixed with those) -boris
[xen-unstable test] 177271: tolerable trouble: fail/pass/starved
flight 177271 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/177271/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 177229 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 177229 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 177229 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 177229 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 177229 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 177229 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 177229 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 177229 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 177229 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-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-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 9b70bc6d9678142a40e6c1c6934a32c7a0966e38 baseline version: xen 9b70bc6d9678142a40e6c1c6934a32c7a0966e38 Last test of basis 177271 2023-02-14 11:20:41 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt
HWP and ACPI workarounds
Hi, Qubes recently incorporated my HWP patches, but there was a report of a laptop, Thinkpad X1 Carbon Gen 4 with a Skylake processor, locking up during boot when HWP is enabled. A user found a kernel bug that seems to be the same issue: https://bugzilla.kernel.org/show_bug.cgi?id=110941. That bug was fixed by Linux commit a21211672c9a ("ACPI / processor: Request native thermal interrupt handling via _OSC"). The commit message has a good summary of the issue and is included at the end of this message. The tl;dr is SMM crashes when it receives thermal interrupts, so Linux calls the ACPI _OSC method to take over interrupt handling. Today, Linux calls the _OSC method when boot_cpu_has(X86_FEATURE_HWP), but that is not exposed to the PV Dom0. As a test, the Qubes user was able to boot with the check expanded to `boot_cpu_has(X86_FEATURE_HWP) || xen_initial_domain()`. We need some way for Xen to indicate the presence and/or use of HWP to Dom0, and Dom0 needs to use that to call _OSC. My first idea is that Dom0 could query Xen's cpufreq driver. However, Xen exposes the cpufreq driver through the unstable sysctl ops, and using an unstable hypercall seems wrong for the kernel. Can we add something to an existing hypercall - maybe platform_op? Or do we need a new stable hypercall? Linux will perform the _OSC calls unilaterally upon seeing FEATURE_HWP and independent of actually using HWP via the intel_pstate driver. However, not using HWP may be an untested configuration in practice. The intel_pstate.c driver will not use HWP when FEATURE_HWP_EPP is not found. So we could potentially cheat and expose only HWP to Dom0. That should trigger the _OSC calls without letting Dom0 think it can use HWP. This is rather fragile though, so a more explicity method seems better. Roger's ACPI Processor patches that add xen_sanitize_pdc calls could be leveraged. On the Xen side, arch_acpi_set_pdc_bits() could be extended to set bit 12, which would then be passed to the evaluate _PDC call. _PDC is the older interface superseded by _OSC, but they can be wrappers around the same implementation. But if linux is just using _OSC, it seems more compatible to follow that implementation. Thoughts? Thanks, Jason commit a21211672c9a1d730a39aa65d4a5b3414700adfb Author: Srinivas Pandruvada Date: Wed Mar 23 21:07:39 2016 -0700 ACPI / processor: Request native thermal interrupt handling via _OSC There are several reports of freeze on enabling HWP (Hardware PStates) feature on Skylake-based systems by the Intel P-states driver. The root cause is identified as the HWP interrupts causing BIOS code to freeze. HWP interrupts use the thermal LVT which can be handled by Linux natively, but on the affected Skylake-based systems SMM will respond to it by default. This is a problem for several reasons: - On the affected systems the SMM thermal LVT handler is broken (it will crash when invoked) and a BIOS update is necessary to fix it. - With thermal interrupt handled in SMM we lose all of the reporting features of the arch/x86/kernel/cpu/mcheck/therm_throt driver. - Some thermal drivers like x86-package-temp depend on the thermal threshold interrupts signaled via the thermal LVT. - The HWP interrupts are useful for debugging and tuning performance (if the kernel can handle them). The native handling of thermal interrupts needs to be enabled because of that. This requires some way to tell SMM that the OS can handle thermal interrupts. That can be done by using _OSC/_PDC in processor scope very early during ACPI initialization. The meaning of _OSC/_PDC bit 12 in processor scope is whether or not the OS supports native handling of interrupts for Collaborative Processor Performance Control (CPPC) notifications. Since on HWP-capable systems CPPC is a firmware interface to HWP, setting this bit effectively tells the firmware that the OS will handle thermal interrupts natively going forward. For details on _OSC/_PDC refer to: http://www.intel.com/content/www/us/en/standards/processor-vendor-specific-acpi-specification.html To implement the _OSC/_PDC handshake as described, introduce a new function, acpi_early_processor_osc(), that walks the ACPI namespace looking for ACPI processor objects and invokes _OSC for them with bit 12 in the capabilities buffer set and terminates the namespace walk on the first success. Also modify intel_thermal_interrupt() to clear HWP status bits in the HWP_STATUS MSR to acknowledge HWP interrupts (which prevents them from firing continuously).
[xen-unstable-smoke test] 177305: tolerable trouble: pass/starved - PUSHED
flight 177305 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/177305/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 63305e5392ec2d17b85e7996a97462744425db80 baseline version: xen 9b70bc6d9678142a40e6c1c6934a32c7a0966e38 Last test of basis 177187 2023-02-13 16:01:58 Z1 days Testing same since 177305 2023-02-14 19:03:50 Z0 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved 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 9b70bc6d96..63305e5392 63305e5392ec2d17b85e7996a97462744425db80 -> smoke
Re: [PATCH 2/2] xen/misra: add entries to exclude-list.json
On Tue, 14 Feb 2023, Jan Beulich wrote: > On 14.02.2023 09:56, Luca Fancellu wrote: > > --- a/docs/misra/exclude-list.json > > +++ b/docs/misra/exclude-list.json > > @@ -1,4 +1,209 @@ > > { > > "version": "1.0", > > -"content": [] > > +"content": [ > > +{ > > +"rel_path": "arch/arm/arm64/cpufeature.c" > > +}, > > +{ > > +"rel_path": "arch/arm/arm64/insn.c" > > +}, > > +{ > > +"rel_path": "arch/arm/arm64/lib/find_next_bit.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/boot.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/cpu_idle.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/cpuidle_menu.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/lib.c" > > +}, > > +{ > > +"rel_path": "arch/x86/acpi/power.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/amd.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/centaur.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/common.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/hygon.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/intel.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/intel_cacheinfo.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/mcheck/mce-apei.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/mcheck/non-fatal.c" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/mtrr/*" > > +}, > > +{ > > +"rel_path": "arch/x86/cpu/mwait-idle.c" > > +}, > > +{ > > +"rel_path": "arch/x86/delay.c" > > +}, > > +{ > > +"rel_path": "arch/x86/dmi_scan.c" > > +}, > > +{ > > +"rel_path": "arch/x86/domain.c" > > +}, > > +{ > > +"rel_path": "arch/x86/genapic/*" > > +}, > > +{ > > +"rel_path": "arch/x86/i387.c" > > +}, > > +{ > > +"rel_path": "arch/x86/irq.c" > > +}, > > +{ > > +"rel_path": "arch/x86/mpparse.c" > > +}, > > +{ > > +"rel_path": "arch/x86/srat.c" > > +}, > > +{ > > +"rel_path": "arch/x86/time.c" > > +}, > > +{ > > +"rel_path": "arch/x86/traps.c" > > +}, > > +{ > > +"rel_path": "arch/x86/usercopy.c" > > +}, > > +{ > > +"rel_path": "arch/x86/x86_64/mmconf-fam10h.c" > > +}, > > +{ > > +"rel_path": "common/bitmap.c" > > +}, > > +{ > > +"rel_path": "common/bunzip2.c" > > +}, > > +{ > > +"rel_path": "common/cpu.c" > > +}, > > +{ > > +"rel_path": "common/earlycpio.c" > > +}, > > +{ > > +"rel_path": "common/inflate.c" > > +}, > > +{ > > +"rel_path": "common/libfdt/*" > > +}, > > +{ > > +"rel_path": "common/lz4/decompress.c" > > +}, > > +{ > > +"rel_path": "common/notifier.c" > > +}, > > +{ > > +"rel_path": "common/radix-tree.c" > > +}, > > +{ > > +"rel_path": "common/rcupdate.c" > > +}, > > +{ > > +"rel_path": "common/softirq.c" > > +}, > > +{ > > +"rel_path": "common/tasklet.c" > > +}, > > +{ > > +"rel_path": "common/ubsan/ubsan.c" > > +}, > > +{ > > +"rel_path": "common/un*.c" > > +}, > > +{ > > +"rel_path": "common/vsprintf.c" > > +}, > > +{ > > +"rel_path": "common/xz/*" > > +}, > > +{ > > +"rel_path": "common/zstd/*" > > +}, > > +{ > > +"rel_path": "crypto/rijndael.c" > > +}, > > +{ > > +"rel_path": "crypto/vmac.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/apei/*" > > +}, > > +{ > > +"rel_path": "drivers/acpi/hwregs.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/numa.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/osl.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/reboot.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/tables.c" > > +}, > > +{ > > +"rel_path": "drivers/acpi/tables/*" > > +}, > > +{ > > +
Re: [PATCH v2 1/5] automation: Switch arm32 cross builds to run on arm64
On Tue, 14 Feb 2023, Michal Orzel wrote: > Due to the limited x86 CI resources slowing down the whole pipeline, > switch the arm32 cross builds to be executed on arm64 which is much more > capable. For that, rename the existing debian container dockerfile > from unstable-arm32-gcc to unstable-arm64v8-arm32-gcc and use > arm64v8/debian:unstable as an image. Note, that we cannot use the same > container name as we have to keep the backwards compatibility. > Take the opportunity to remove extra empty line at the end of a file. > > Modify the tag of .arm32-cross-build-tmpl to arm64 and update the build > jobs accordingly. > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - new patch > > For convenience and own testing, I built and pushed the new container > to registry. Thanks! > --- > ...ockerfile => unstable-arm64v8-arm32-gcc.dockerfile} | 3 +-- > automation/gitlab-ci/build.yaml| 10 +- > 2 files changed, 6 insertions(+), 7 deletions(-) > rename automation/build/debian/{unstable-arm32-gcc.dockerfile => > unstable-arm64v8-arm32-gcc.dockerfile} (94%) > > diff --git a/automation/build/debian/unstable-arm32-gcc.dockerfile > b/automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile > similarity index 94% > rename from automation/build/debian/unstable-arm32-gcc.dockerfile > rename to automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile > index b41a57f19729..11860425a6a2 100644 > --- a/automation/build/debian/unstable-arm32-gcc.dockerfile > +++ b/automation/build/debian/unstable-arm64v8-arm32-gcc.dockerfile > @@ -1,4 +1,4 @@ > -FROM debian:unstable > +FROM arm64v8/debian:unstable > LABEL maintainer.name="The Xen Project" \ >maintainer.email="xen-devel@lists.xenproject.org" > > @@ -21,4 +21,3 @@ RUN apt-get update && \ > apt-get autoremove -y && \ > apt-get clean && \ > rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* > - > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index a053c5c7325d..f8e156e0a994 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -123,7 +123,7 @@ >variables: > XEN_TARGET_ARCH: arm32 >tags: > -- x86_64 > +- arm64 > > .arm32-cross-build: >extends: .arm32-cross-build-tmpl > @@ -542,26 +542,26 @@ alpine-3.12-clang-debug: > debian-unstable-gcc-arm32: >extends: .gcc-arm32-cross-build >variables: > -CONTAINER: debian:unstable-arm32-gcc > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > HYPERVISOR_ONLY: y > > debian-unstable-gcc-arm32-debug: >extends: .gcc-arm32-cross-build-debug >variables: > -CONTAINER: debian:unstable-arm32-gcc > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > HYPERVISOR_ONLY: y > > debian-unstable-gcc-arm32-randconfig: >extends: .gcc-arm32-cross-build >variables: > -CONTAINER: debian:unstable-arm32-gcc > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > HYPERVISOR_ONLY: y > RANDCONFIG: y > > debian-unstable-gcc-arm32-debug-randconfig: >extends: .gcc-arm32-cross-build-debug >variables: > -CONTAINER: debian:unstable-arm32-gcc > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > HYPERVISOR_ONLY: y > RANDCONFIG: y > > -- > 2.25.1 >
Re: [PATCH v2 2/5] automation: Add arm32 dom0less testing
On Tue, 14 Feb 2023, Michal Orzel wrote: > At the moment, we only perform a single arm32 test in our CI, checking > whether dom0 boots successfully or not. This is mostly because we do not > have any arm32 runners and we only execute a hypervisor only build. > > In order not to leave the arm32 testing in such a poor state, add a > script qemu-smoke-dom0less-arm32.sh to start testing dom0less > configuration, while keeping dom0 to make the test more interesting. > > The script is mostly based on the one used for dom0 arm32 testing as well > as the one used for dom0less arm64 testing. We obtain Debian Bullseye > kernel and Alpine Linux busybox-based rootfs. Depending on the test > variant, we prepare a test case contained within domU_check variable, > that will be executed as part of /init script from domU rootfs. > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - keep dom0 around by default to make tests more interesting > --- > automation/gitlab-ci/test.yaml| 16 +++ > .../scripts/qemu-smoke-dom0less-arm32.sh | 99 +++ > 2 files changed, 115 insertions(+) > create mode 100755 automation/scripts/qemu-smoke-dom0less-arm32.sh > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index ce543ef5c0ef..84ab1fee50a4 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -210,6 +210,22 @@ qemu-smoke-dom0-arm32-gcc-debug: > - *arm32-test-needs > - debian-unstable-gcc-arm32-debug > > +qemu-smoke-dom0less-arm32-gcc: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh 2>&1 | tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32 > + > +qemu-smoke-dom0less-arm32-gcc-debug: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh 2>&1 | tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32-debug > + > qemu-alpine-x86_64-gcc: >extends: .qemu-x86-64 >script: > diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh > b/automation/scripts/qemu-smoke-dom0less-arm32.sh > new file mode 100755 > index ..e3f2b28f3f89 > --- /dev/null > +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh > @@ -0,0 +1,99 @@ > +#!/bin/bash > + > +set -ex > + > +test_variant=$1 > + > +# Prompt to grep for to check if dom0 booted successfully > +dom0_prompt="^/ #" > + > +cd binaries > +# Use the kernel from Debian > +curl --fail --silent --show-error --location --output vmlinuz > https://deb.debian.org/debian/dists/bullseye/main/installer-armhf/current/images/netboot/vmlinuz > +# Use a tiny initrd based on busybox from Alpine Linux > +curl --fail --silent --show-error --location --output initrd.tar.gz > https://dl-cdn.alpinelinux.org/alpine/v3.15/releases/armhf/alpine-minirootfs-3.15.1-armhf.tar.gz > + > +if [ -z "${test_variant}" ]; then > +passed="generic test passed" > +domU_check=" > +echo \"${passed}\" > +" > +fi > + > +# dom0/domU rootfs > +# We are using the same rootfs for dom0 and domU. The only difference is > +# that for the former, we set explictly rdinit to /bin/sh, whereas for the > +# latter we rely on using custom /init script with test case inside. > +mkdir rootfs > +cd rootfs > +tar xvzf ../initrd.tar.gz > +echo "#!/bin/sh > + > +mount -t proc proc /proc > +mount -t sysfs sysfs /sys > +mount -t devtmpfs devtmpfs /dev > +${domU_check} > +/bin/sh" > init > +chmod +x init > +find . | cpio -H newc -o | gzip > ../initrd.gz > +cd .. > + > +# XXX QEMU looks for "efi-virtio.rom" even if it is unneeded > +curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom > +./qemu-system-arm \ > +-machine virt \ > +-machine virtualization=true \ > +-smp 4 \ > +-m 2048 \ > +-serial stdio \ > +-monitor none \ > +-display none \ > +-machine dumpdtb=virt.dtb > + > +# ImageBuilder > +echo 'MEMORY_START="0x4000" > +MEMORY_END="0xC000" > + > +DEVICE_TREE="virt.dtb" > +XEN="xen" > +XEN_CMD="console=dtuart dom0_mem=512M bootscrub=0" > + > +DOM0_KERNEL="vmlinuz" > +DOM0_RAMDISK="initrd.gz" > +DOM0_CMD="console=hvc0 earlyprintk clk_ignore_unused root=/dev/ram0 > rdinit=/bin/sh" > + > +DOMU_KERNEL[0]="vmlinuz" > +DOMU_RAMDISK[0]="initrd.gz" > +DOMU_MEM[0]="512" > +NUM_DOMUS=1 > + > +LOAD_CMD="tftpb" > +BOOT_CMD="bootm" > +UBOOT_SOURCE="boot.source" > +UBOOT_SCRIPT="boot.scr"' > config > + > +rm -rf imagebuilder > +git clone https://gitlab.com/ViryaOS/imagebuilder > +bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config > + > +# Run the test > +rm -f smoke.serial > +set +e > +echo " virtio scan; dhcp; tftpb 0x4000 boot.scr; source 0x4000"| \ > +timeout -k 1 240 \ > +./qemu-system-arm \ > +-machine virt \ > +-machine virtualization=true \ > +-smp 4 \ > +-m 2048 \ > +-serial stdio \ > +-monitor none \ > +-di
Re: [PATCH v2 3/5] automation: Add a static memory allocation test on arm32
On Tue, 14 Feb 2023, Michal Orzel wrote: > Add a new test job qemu-smoke-dom0less-arm32-gcc-staticmem in debug > and non-debug variant that will execute qemu-smoke-dom0less-arm32.sh > script to test static memory allocation feature. The test case itself > is directly taken from dom0less arm64 testing. > > Populate build jobs to compile Xen with config options necessary to > enable static memory feature. Populate test jobs passing "static-mem" > as a test variant. The test configures domU with a static memory region > (direct-mapped) and adds a check using /proc/iomem to determine the > memory region marked as "System RAM". > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - take into account new container for arm32 cross builds > - drop Rb as a result of code changes > --- > automation/gitlab-ci/build.yaml | 20 +++ > automation/gitlab-ci/test.yaml| 16 +++ > .../scripts/qemu-smoke-dom0less-arm32.sh | 17 > 3 files changed, 53 insertions(+) > > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index f8e156e0a994..079e9b73f659 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -565,6 +565,26 @@ debian-unstable-gcc-arm32-debug-randconfig: > HYPERVISOR_ONLY: y > RANDCONFIG: y > > +debian-unstable-gcc-arm32-staticmem: > + extends: .gcc-arm32-cross-build > + variables: > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > +HYPERVISOR_ONLY: y > +EXTRA_XEN_CONFIG: | > + CONFIG_EXPERT=y > + CONFIG_UNSUPPORTED=y > + CONFIG_STATIC_MEMORY=y > + > +debian-unstable-gcc-arm32-debug-staticmem: > + extends: .gcc-arm32-cross-build-debug > + variables: > +CONTAINER: debian:unstable-arm64v8-arm32-gcc > +HYPERVISOR_ONLY: y > +EXTRA_XEN_CONFIG: | > + CONFIG_EXPERT=y > + CONFIG_UNSUPPORTED=y > + CONFIG_STATIC_MEMORY=y > + > # Arm builds > > debian-unstable-gcc-arm64: > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index 84ab1fee50a4..c2bcc5d4d3e5 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -226,6 +226,22 @@ qemu-smoke-dom0less-arm32-gcc-debug: > - *arm32-test-needs > - debian-unstable-gcc-arm32-debug > > +qemu-smoke-dom0less-arm32-gcc-staticmem: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh static-mem 2>&1 | > tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32-staticmem > + > +qemu-smoke-dom0less-arm32-gcc-debug-staticmem: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh static-mem 2>&1 | > tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32-debug-staticmem > + > qemu-alpine-x86_64-gcc: >extends: .qemu-x86-64 >script: > diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh > b/automation/scripts/qemu-smoke-dom0less-arm32.sh > index e3f2b28f3f89..bd89a3f8b45e 100755 > --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh > +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh > @@ -20,6 +20,19 @@ echo \"${passed}\" > " > fi > > +if [[ "${test_variant}" == "static-mem" ]]; then > +# Memory range that is statically allocated to domU1 > +domu_base="0x5000" > +domu_size="0x2000" > +passed="${test_variant} test passed" > +domU_check=" > +mem_range=$(printf \"%08x-%08x\" ${domu_base} $(( ${domu_base} + > ${domu_size} - 1 ))) > +if grep -q -x \"\${mem_range} : System RAM\" /proc/iomem; then > +echo \"${passed}\" > +fi > +" > +fi > + > # dom0/domU rootfs > # We are using the same rootfs for dom0 and domU. The only difference is > # that for the former, we set explictly rdinit to /bin/sh, whereas for the > @@ -72,6 +85,10 @@ BOOT_CMD="bootm" > UBOOT_SOURCE="boot.source" > UBOOT_SCRIPT="boot.scr"' > config > > +if [[ "${test_variant}" == "static-mem" ]]; then > +echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >> config > +fi > + > rm -rf imagebuilder > git clone https://gitlab.com/ViryaOS/imagebuilder > bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config > -- > 2.25.1 >
Re: [PATCH v2 4/5] automation: Add a gzip compressed kernel image test on arm32
On Tue, 14 Feb 2023, Michal Orzel wrote: > Xen supports booting gzip compressed kernels, therefore add a new test > job qemu-smoke-dom0less-arm32-gcc-gzip in debug and non-debug variant > that will execute qemu-smoke-dom0less-arm32.sh script to test booting > a domU with a gzip compressed kernel image (in our case zImage). > > By passing "gzip" as a test variant, the test will call gzip command > to compress kernel image and use it in ImageBuilder configuration for > domU1. No need for a specific check to be executed from domU. Being able > to see a test message from a boot log is sufficient to mark a test as > passed. > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - take into account dom0 presence > - drop Rb as a result of code changes > --- > automation/gitlab-ci/test.yaml | 16 > automation/scripts/qemu-smoke-dom0less-arm32.sh | 13 + > 2 files changed, 29 insertions(+) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index c2bcc5d4d3e5..be7a55d89708 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -242,6 +242,22 @@ qemu-smoke-dom0less-arm32-gcc-debug-staticmem: > - *arm32-test-needs > - debian-unstable-gcc-arm32-debug-staticmem > > +qemu-smoke-dom0less-arm32-gcc-gzip: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh gzip 2>&1 | tee > ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32 > + > +qemu-smoke-dom0less-arm32-gcc-debug-gzip: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh gzip 2>&1 | tee > ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32-debug > + > qemu-alpine-x86_64-gcc: >extends: .qemu-x86-64 >script: > diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh > b/automation/scripts/qemu-smoke-dom0less-arm32.sh > index bd89a3f8b45e..c2e331451d99 100755 > --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh > +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh > @@ -33,6 +33,15 @@ fi > " > fi > > +if [[ "${test_variant}" == "gzip" ]]; then > +# Compress kernel image with gzip (keep unmodified one for dom0) > +gzip -k vmlinuz > +passed="${test_variant} test passed" > +domU_check=" > +echo \"${passed}\" > +" > +fi > + > # dom0/domU rootfs > # We are using the same rootfs for dom0 and domU. The only difference is > # that for the former, we set explictly rdinit to /bin/sh, whereas for the > @@ -89,6 +98,10 @@ if [[ "${test_variant}" == "static-mem" ]]; then > echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >> config > fi > > +if [[ "${test_variant}" == "gzip" ]]; then > +sed -i 's/DOMU_KERNEL\[0\]=.*/DOMU_KERNEL\[0\]="vmlinuz.gz"/' config > +fi > + > rm -rf imagebuilder > git clone https://gitlab.com/ViryaOS/imagebuilder > bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config > -- > 2.25.1 >
Re: [PATCH v2 5/5] automation: Add a true dom0less test on arm32
On Tue, 14 Feb 2023, Michal Orzel wrote: > Add a new test job qemu-smoke-dom0less-arm32-gcc-without-dom0 in debug > and non-debug variant that will execute qemu-smoke-dom0less-arm32.sh > script to test dom0less domU boot without dom0 (i.e. true dom0less > configuration). > > By passing "without-dom0" as a test variant, the test will clear the > dom0 prompt that we grep for as a last step and remove all the DOM0 > related options in ImageBuilder configuration. > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - new patch created as a result of deciding to keep dom0 by default. We still >need to test true dom0less configuration, hence added a new test. > --- > automation/gitlab-ci/test.yaml | 16 > automation/scripts/qemu-smoke-dom0less-arm32.sh | 13 + > 2 files changed, 29 insertions(+) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index be7a55d89708..c0b4a90e0d29 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -258,6 +258,22 @@ qemu-smoke-dom0less-arm32-gcc-debug-gzip: > - *arm32-test-needs > - debian-unstable-gcc-arm32-debug > > +qemu-smoke-dom0less-arm32-gcc-without-dom0: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh without-dom0 2>&1 | > tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32 > + > +qemu-smoke-dom0less-arm32-gcc-debug-without-dom0: > + extends: .qemu-arm32 > + script: > +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh without-dom0 2>&1 | > tee ${LOGFILE} > + needs: > +- *arm32-test-needs > +- debian-unstable-gcc-arm32-debug > + > qemu-alpine-x86_64-gcc: >extends: .qemu-x86-64 >script: > diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh > b/automation/scripts/qemu-smoke-dom0less-arm32.sh > index c2e331451d99..cc91238f4222 100755 > --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh > +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh > @@ -42,6 +42,15 @@ echo \"${passed}\" > " > fi > > +if [[ "${test_variant}" == "without-dom0" ]]; then > +# Clear dom0 prompt > +dom0_prompt="" > +passed="${test_variant} test passed" > +domU_check=" > +echo \"${passed}\" > +" > +fi > + > # dom0/domU rootfs > # We are using the same rootfs for dom0 and domU. The only difference is > # that for the former, we set explictly rdinit to /bin/sh, whereas for the > @@ -102,6 +111,10 @@ if [[ "${test_variant}" == "gzip" ]]; then > sed -i 's/DOMU_KERNEL\[0\]=.*/DOMU_KERNEL\[0\]="vmlinuz.gz"/' config > fi > > +if [[ "${test_variant}" == "without-dom0" ]]; then > +sed -i '/^DOM0/d' config > +fi > + > rm -rf imagebuilder > git clone https://gitlab.com/ViryaOS/imagebuilder > bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config > -- > 2.25.1 >
Re: [PATCH v2] automation: Add container and build jobs to run cppcheck analysis
On Tue, 14 Feb 2023, Michal Orzel wrote: > Add a debian container with cppcheck installation routine inside, > capable of performing cppcheck analysis on Xen-only build including > cross-builds for arm32 and x86_64. > > Populate build jobs making use of that container to run cppcheck > analysis to produce a text report (xen-cppcheck.txt) containing the list > of all the findings. > > This patch does not aim at performing any sort of bisection. Cppcheck is > imperfect and for now, our goal is to at least be aware of its reports, > so that we can compare them with the ones produced by better tools and > to be able to see how these reports change as a result of further > infrastructure improvements (e.g. exception list, rules exclusion). > > Signed-off-by: Michal Orzel Reviewed-by: Stefano Stabellini > --- > Changes in v2: > - use arm64 container instead of x86 to make pipeline faster > - explicitly set HYPERVISOR_ONLY=y for cppcheck jobs > > For convenience and own testing, I built and pushed the new container > to registry. CI pipeline including dom0less series: > https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/777181033 > --- > .../build/debian/unstable-cppcheck.dockerfile | 37 > automation/gitlab-ci/build.yaml | 43 +++ > automation/scripts/build | 11 - > 3 files changed, 90 insertions(+), 1 deletion(-) > create mode 100644 automation/build/debian/unstable-cppcheck.dockerfile > > diff --git a/automation/build/debian/unstable-cppcheck.dockerfile > b/automation/build/debian/unstable-cppcheck.dockerfile > new file mode 100644 > index ..54b99f87dfec > --- /dev/null > +++ b/automation/build/debian/unstable-cppcheck.dockerfile > @@ -0,0 +1,37 @@ > +FROM arm64v8/debian:unstable > +LABEL maintainer.name="The Xen Project" \ > + maintainer.email="xen-devel@lists.xenproject.org" > + > +ENV DEBIAN_FRONTEND=noninteractive > +ENV CPPCHECK_VERSION=2.7 > +ENV USER root > + > +RUN mkdir /build > +WORKDIR /build > + > +# dependencies for cppcheck and Xen-only build/cross-build > +RUN apt-get update && \ > +apt-get --quiet --yes install \ > +build-essential \ > +curl \ > +python-is-python3 \ > +libpcre3-dev \ > +flex \ > +bison \ > +gcc-arm-linux-gnueabihf \ > +gcc-x86-64-linux-gnu > + > +# cppcheck release build (see cppcheck readme.md) > +RUN curl -fsSLO > https://github.com/danmar/cppcheck/archive/"$CPPCHECK_VERSION".tar.gz && \ > +tar xvzf "$CPPCHECK_VERSION".tar.gz && \ > +cd cppcheck-"$CPPCHECK_VERSION" && \ > +make install -j$(nproc) \ > +MATCHCOMPILER=yes \ > +FILESDIR=/usr/share/cppcheck \ > +HAVE_RULES=yes CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare > -Wno-unused-function" > + > +# clean > +RUN apt-get autoremove -y && \ > +apt-get clean && \ > +rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* && \ > +rm -rf cppcheck-"$CPPCHECK_VERSION"* "$CPPCHECK_VERSION".tar.gz > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index 079e9b73f659..1441b7dbb6fa 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -7,6 +7,7 @@ > paths: >- binaries/ >- xen-config > + - xen-cppcheck.txt >- '*.log' >- '*/*.log' > when: always > @@ -199,6 +200,23 @@ >variables: > <<: *gcc > > +.x86-64-cross-build-tmpl: > + <<: *build > + variables: > +XEN_TARGET_ARCH: x86_64 > + tags: > +- arm64 > + > +.x86-64-cross-build: > + extends: .x86-64-cross-build-tmpl > + variables: > +debug: n > + > +.gcc-x86-64-cross-build: > + extends: .x86-64-cross-build > + variables: > +<<: *gcc > + > # Jobs below this line > > archlinux-gcc: > @@ -699,6 +717,31 @@ archlinux-current-gcc-riscv64-debug-randconfig: > EXTRA_FIXED_RANDCONFIG: >CONFIG_COVERAGE=n > > +# Cppcheck analysis jobs > + > +debian-unstable-gcc-cppcheck: > + extends: .gcc-x86-64-cross-build > + variables: > +CONTAINER: debian:unstable-cppcheck > +CROSS_COMPILE: /usr/bin/x86_64-linux-gnu- > +CPPCHECK: y > +HYPERVISOR_ONLY: y > + > +debian-unstable-gcc-arm32-cppcheck: > + extends: .gcc-arm32-cross-build > + variables: > +CONTAINER: debian:unstable-cppcheck > +CROSS_COMPILE: /usr/bin/arm-linux-gnueabihf- > +CPPCHECK: y > +HYPERVISOR_ONLY: y > + > +debian-unstable-gcc-arm64-cppcheck: > + extends: .gcc-arm64-build > + variables: > +CONTAINER: debian:unstable-cppcheck > +CPPCHECK: y > +HYPERVISOR_ONLY: y > + > ## Test artifacts common > > .test-jobs-artifact-common: > diff --git a/automation/scripts/build b/automation/scripts/build > index f2f5e55bc04f..7d1b19c4250d 100755 > --- a/automation/scripts/build > +++ b/automation/scripts/build > @@ -38,7 +38,16 @@ cp xen/.config xen-config > # Directory for the artefacts to be dumped into > mkdir
Re: [RFC PATCH 01/10] xen: pci: add per-domain pci list lock
Hello Stefano, Stefano Stabellini writes: > On Wed, 31 Aug 2022, Volodymyr Babchuk wrote: >> domain->pdevs_lock protects access to domain->pdev_list. >> As this, it should be used when we are adding, removing on enumerating >> PCI devices assigned to a domain. >> >> This enables more granular locking instead of one huge pcidevs_lock that >> locks entire PCI subsystem. Please note that pcidevs_lock() is still >> used, we are going to remove it in subsequent patches. >> >> Signed-off-by: Volodymyr Babchuk > > I reviewed the patch, and made sure to pay extra attention to: Thank you for doing this. > - error paths > - missing locks > - lock ordering > - interruptions > Here is what I found: > > > 1) iommu.c:reassign_device_ownership and pci_amd_iommu.c:reassign_device > Both functions without any pdevs_lock locking do: > list_move(&pdev->domain_list, &target->pdev_list); > > It seems to be it would need pdevs_lock. Maybe we need to change > list_move into list_del (protected by the pdevs_lock of the old domain) > and list_add (protected by the pdev_lock of the new domain). Yes, I did as you suggested. But this leads to another potential issue. I'll describe it below, in deassign_device() part. [...] >> +spin_lock(&d->pdevs_lock); >> list_for_each_entry_safe ( pdev, tmp, &d->pdev_list, domain_list ) >> { >> bus = pdev->bus; >> devfn = pdev->devfn; >> ret = deassign_device(d, pdev->seg, bus, devfn) ?: ret; > > This causes pdevs_lock to be taken twice. deassign_device also takes a > pdevs_lock. Probably we need to change all the > spin_lock(&d->pdevs_lock) into spin_lock_recursive. You are right, I missed that deassign_device() causes iommu*_reassign_device() call. But there lies the issue: with recursive locks, reassign_device() will not be able to unlock source->pdevs_lock, but will try to take target->pdevs_lock also. This potentially might lead to deadlock, if another call to reassign_device() moves some other pdev in the opposite way at the same time. This is why I want to avoid recursive spinlocks if possible. So, I am thinking: why does IOMMU code move a pdev across domains in the first place? We are making IOMMU driver responsible of managing domain's pdevs, which does not look right, as this is the responsibility of pci.c I want to propose another approach: implement deassign_device() function in IOMMU drivers. Then, instead of calling to reassign_device() we might do the following: 1. deassign_device() 2. remove pdev from source->pdev_list 3. add pdef to target->pdev_list 4. assign_device() [...] >> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >> index 1cf629e7ec..0775228ba9 100644 >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -457,6 +457,7 @@ struct domain >> >> #ifdef CONFIG_HAS_PCI >> struct list_head pdev_list; >> +spinlock_t pdevs_lock; > > I think it would be better called "pdev_lock" but OK either way As Jan pointed out, we are locking devices as in plural. On other hand, I can rename pdev_list to pdevs_lists to make this consistent. > > >> #endif >> >> #ifdef CONFIG_HAS_PASSTHROUGH >> -- >> 2.36.1 >>
Re: [PATCH] x86/Xen: make use of IBPB controlling VM assist
On 2/14/23 11:13 AM, Jan Beulich wrote: --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -18,6 +18,8 @@ #include #include +#include + #include #include #include @@ -32,6 +34,7 @@ #include #include #include +#include #include #include "cpu.h" @@ -934,7 +937,8 @@ do_cmd_auto: break; case RETBLEED_MITIGATION_IBPB: - setup_force_cpu_cap(X86_FEATURE_ENTRY_IBPB); + if (!xen_pv_domain() || xen_vm_assist_ibpb(true)) Is this going to compile without CONFIG_XEN? I also think these two conditions should be wrapped into something to limit exposure of non-Xen code to Xen-specific primitives. -boris + setup_force_cpu_cap(X86_FEATURE_ENTRY_IBPB); mitigate_smt = true; break;
Re: [PATCH] x86/Xen: make use of IBPB controlling VM assist
On 2/14/23 6:53 PM, Boris Ostrovsky wrote: On 2/14/23 11:13 AM, Jan Beulich wrote: --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -18,6 +18,8 @@ #include #include +#include + #include #include #include @@ -32,6 +34,7 @@ #include #include #include +#include #include #include "cpu.h" @@ -934,7 +937,8 @@ do_cmd_auto: break; case RETBLEED_MITIGATION_IBPB: - setup_force_cpu_cap(X86_FEATURE_ENTRY_IBPB); + if (!xen_pv_domain() || xen_vm_assist_ibpb(true)) Is this going to compile without CONFIG_XEN? I also think these two conditions should be wrapped into something to limit exposure of non-Xen code to Xen-specific primitives. Oh, and this needs x86 maintainers. -boris
[PATCH 0/3] automation: add arm32 xl domU creation test
Hi all, This patch series add a domU creation test based on xl for arm32. To do that, it reuses the existing arm32 dom0 test, and also reuses the Yocto qemuarm build output. Pipeline (with reduced amount of jobs): https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/80349 Cheers, Stefano Stefano Stabellini (3): automation: move yocto jobs to build stage automation: add binaries/ to artifacts for Yocto jobs automation: expand arm32 dom0 test adding xl domain creation automation/build/yocto/build-yocto.sh | 6 automation/gitlab-ci/build.yaml | 52 + automation/gitlab-ci/test.yaml | 46 + automation/scripts/qemu-smoke-dom0-arm32.sh | 50 --- 4 files changed, 97 insertions(+), 57 deletions(-)
[PATCH 1/3] automation: move yocto jobs to build stage
From: Stefano Stabellini We are going to use artifacts produced by the Yocto builds in test jobs. Signed-off-by: Stefano Stabellini --- automation/gitlab-ci/build.yaml | 51 + automation/gitlab-ci/test.yaml | 45 - 2 files changed, 51 insertions(+), 45 deletions(-) diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index a053c5c732..f62cf21f45 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -199,6 +199,41 @@ variables: <<: *gcc +.yocto-test: + stage: build + image: registry.gitlab.com/xen-project/xen/${CONTAINER} + except: +- master +- smoke +- /^coverity-tested\/.*/ +- /^stable-.*/ + script: +- ./automation/build/yocto/build-yocto.sh -v --log-dir=./logs --xen-dir=`pwd` ${YOCTO_BOARD} + variables: +YOCTO_VERSION: kirkstone +CONTAINER: yocto:${YOCTO_VERSION}-${YOCTO_BOARD}-${YOCTO_HOST} + artifacts: +paths: + - 'logs/*' +when: always + needs: [] + +.yocto-test-arm64: + extends: .yocto-test + variables: +YOCTO_HOST: arm64v8 + tags: +- arm64 + +# This is not used by any test job as we only run Yocto on arm based machines. +# Keep it here so that someone having x86 hardware can easily add jobs. +.yocto-test-x86-64: + extends: .yocto-test + variables: +YOCTO_HOST: amd64 + tags: +- x86_64 + # Jobs below this line archlinux-gcc: @@ -679,6 +714,22 @@ archlinux-current-gcc-riscv64-debug-randconfig: EXTRA_FIXED_RANDCONFIG: CONFIG_COVERAGE=n +# Yocto test jobs +yocto-qemuarm64: + extends: .yocto-test-arm64 + variables: +YOCTO_BOARD: qemuarm64 + +yocto-qemuarm: + extends: .yocto-test-arm64 + variables: +YOCTO_BOARD: qemuarm + +yocto-qemux86-64: + extends: .yocto-test-arm64 + variables: +YOCTO_BOARD: qemux86-64 + ## Test artifacts common .test-jobs-artifact-common: diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index ce543ef5c0..9570085a60 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -67,35 +67,6 @@ tags: - x86_64 -.yocto-test: - extends: .test-jobs-common - script: -- ./automation/build/yocto/build-yocto.sh -v --log-dir=./logs --xen-dir=`pwd` ${YOCTO_BOARD} - variables: -YOCTO_VERSION: kirkstone -CONTAINER: yocto:${YOCTO_VERSION}-${YOCTO_BOARD}-${YOCTO_HOST} - artifacts: -paths: - - 'logs/*' -when: always - needs: [] - -.yocto-test-arm64: - extends: .yocto-test - variables: -YOCTO_HOST: arm64v8 - tags: -- arm64 - -# This is not used by any test job as we only run Yocto on arm based machines. -# Keep it here so that someone having x86 hardware can easily add jobs. -.yocto-test-x86-64: - extends: .yocto-test - variables: -YOCTO_HOST: amd64 - tags: -- x86_64 - # Test jobs build-each-commit-gcc: extends: .test-jobs-common @@ -253,19 +224,3 @@ qemu-smoke-riscv64-gcc: - ./automation/scripts/qemu-smoke-riscv64.sh 2>&1 | tee ${LOGFILE} needs: - archlinux-current-gcc-riscv64-debug - -# Yocto test jobs -yocto-qemuarm64: - extends: .yocto-test-arm64 - variables: -YOCTO_BOARD: qemuarm64 - -yocto-qemuarm: - extends: .yocto-test-arm64 - variables: -YOCTO_BOARD: qemuarm - -yocto-qemux86-64: - extends: .yocto-test-arm64 - variables: -YOCTO_BOARD: qemux86-64 -- 2.25.1
[PATCH 2/3] automation: add binaries/ to artifacts for Yocto jobs
From: Stefano Stabellini Copy the build output of Yocto builds to binaries/ and export binaries/ among the jobs artifacts so that they can be reused by other jobs. Signed-off-by: Stefano Stabellini --- automation/build/yocto/build-yocto.sh | 6 ++ automation/gitlab-ci/build.yaml | 1 + 2 files changed, 7 insertions(+) diff --git a/automation/build/yocto/build-yocto.sh b/automation/build/yocto/build-yocto.sh index 3601cebc3c..d0fcaacf06 100755 --- a/automation/build/yocto/build-yocto.sh +++ b/automation/build/yocto/build-yocto.sh @@ -166,6 +166,10 @@ function project_build() { source "${YOCTODIR}/poky/oe-init-build-env" "${destdir}" bitbake "${build_image}" || exit 1 +mkdir -p $OUTPUT +cp $BUILDDIR/tmp/deploy/images/qemuarm/zImage $OUTPUT +cp $BUILDDIR/tmp/deploy/images/qemuarm/xen-qemuarm $OUTPUT +cp $BUILDDIR/tmp/deploy/images/qemuarm/xen-image-minimal-qemuarm.tar.bz2 $OUTPUT ) || return 1 } @@ -238,6 +242,8 @@ Options: EOF } +OUTPUT=`pwd`/binaries + for OPTION in "$@" do case ${OPTION} in diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index f62cf21f45..d4a2aa9a5b 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -215,6 +215,7 @@ artifacts: paths: - 'logs/*' + - binaries/ when: always needs: [] -- 2.25.1
[PATCH 3/3] automation: expand arm32 dom0 test adding xl domain creation
From: Stefano Stabellini As part of the arm32 dom0 test, also create a simple domU using xl. To do that, we need the toolstack installed in the dom0 rootfs. We switch to using the kernel and rootfs built by the Yocto arm32 job. Remove the PCI node from the host device tree: it is unused but causes a Linux hang at boot. Use xen-watchdog to trigger the domU creation for convience (/etc/local.d is not handled by rootfs.) Signed-off-by: Stefano Stabellini --- automation/gitlab-ci/test.yaml | 1 + automation/scripts/qemu-smoke-dom0-arm32.sh | 50 - 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 9570085a60..7bfdd02e64 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -172,6 +172,7 @@ qemu-smoke-dom0-arm32-gcc: needs: - *arm32-test-needs - debian-unstable-gcc-arm32 +- yocto-qemuarm qemu-smoke-dom0-arm32-gcc-debug: extends: .qemu-arm32 diff --git a/automation/scripts/qemu-smoke-dom0-arm32.sh b/automation/scripts/qemu-smoke-dom0-arm32.sh index 98e4d481f6..7a748bdf23 100755 --- a/automation/scripts/qemu-smoke-dom0-arm32.sh +++ b/automation/scripts/qemu-smoke-dom0-arm32.sh @@ -3,14 +3,37 @@ set -ex cd binaries -# Use the kernel from Debian -curl --fail --silent --show-error --location --output vmlinuz http://http.us.debian.org/debian/dists/bullseye/main/installer-armhf/current/images/netboot/vmlinuz -# Use a tiny initrd based on busybox from Alpine Linux -curl --fail --silent --show-error --location --output initrd.tar.gz https://dl-cdn.alpinelinux.org/alpine/v3.15/releases/armhf/alpine-minirootfs-3.15.1-armhf.tar.gz +mkdir rootfs +cd rootfs +tar xvf ../xen-image-minimal-qemuarm.tar.bz2 +mkdir -p ./root +echo "name=\"test\" +memory=400 +vcpus=1 +kernel=\"/root/zImage\" +ramdisk=\"/root/initrd.cpio.gz\" +extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\" +" > root/test.cfg +echo "#!/bin/bash + +xl list + +xl create -c /root/test.cfg + +" > ./root/xen.start +echo "bash /root/xen.start" >> ./etc/init.d/xen-watchdog + +curl --fail --silent --show-error --location --output initrd.tar.gz https://dl-cdn.alpinelinux.org/alpine/v3.15/releases/armhf/alpine-minirootfs-3.15.1-armhf.tar.gz mkdir rootfs cd rootfs tar xvzf ../initrd.tar.gz +find . | cpio -H newc -o | gzip > ../root/initrd.cpio.gz +cd .. +rm -rf rootfs +rm initrd.tar.gz + +cp ../zImage ./root find . | cpio -H newc -o | gzip > ../initrd.gz cd .. @@ -20,22 +43,25 @@ curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom -machine virt \ -machine virtualization=true \ -smp 4 \ - -m 1024 \ + -m 2048 \ -serial stdio \ -monitor none \ -display none \ -machine dumpdtb=virt.dtb +# XXX disable pci to avoid Linux hang +fdtput virt.dtb -p -t s /pcie@1000 status disabled + # ImageBuilder echo 'MEMORY_START="0x4000" -MEMORY_END="0x8000" +MEMORY_END="0xC000" DEVICE_TREE="virt.dtb" -XEN="xen" -DOM0_KERNEL="vmlinuz" +XEN="xen-qemuarm" +DOM0_KERNEL="zImage" DOM0_RAMDISK="initrd.gz" -DOM0_CMD="console=hvc0 earlyprintk clk_ignore_unused root=/dev/ram0 rdinit=/bin/sh" -XEN_CMD="console=dtuart dom0_mem=512M bootscrub=0" +DOM0_CMD="console=hvc0 earlyprintk clk_ignore_unused root=/dev/ram0 rdinit=/sbin/init" +XEN_CMD="console=dtuart dom0_mem=1024M bootscrub=0" NUM_DOMUS=0 @@ -51,12 +77,12 @@ bash imagebuilder/scripts/uboot-script-gen -t tftp -d . -c config rm -f smoke.serial set +e echo " virtio scan; dhcp; tftpb 0x4000 boot.scr; source 0x4000"| \ -timeout -k 1 240 \ +timeout -k 1 720 \ ./qemu-system-arm \ -machine virt \ -machine virtualization=true \ -smp 4 \ - -m 1024 \ + -m 2048 \ -serial stdio \ -monitor none \ -display none \ -- 2.25.1
Re: [PATCH 2/3] automation: add binaries/ to artifacts for Yocto jobs
On Tue, 14 Feb 2023, Stefano Stabellini wrote: > From: Stefano Stabellini > > Copy the build output of Yocto builds to binaries/ and export binaries/ > among the jobs artifacts so that they can be reused by other jobs. > > Signed-off-by: Stefano Stabellini > --- > automation/build/yocto/build-yocto.sh | 6 ++ > automation/gitlab-ci/build.yaml | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/automation/build/yocto/build-yocto.sh > b/automation/build/yocto/build-yocto.sh > index 3601cebc3c..d0fcaacf06 100755 > --- a/automation/build/yocto/build-yocto.sh > +++ b/automation/build/yocto/build-yocto.sh > @@ -166,6 +166,10 @@ function project_build() { > source "${YOCTODIR}/poky/oe-init-build-env" "${destdir}" > > bitbake "${build_image}" || exit 1 > +mkdir -p $OUTPUT > +cp $BUILDDIR/tmp/deploy/images/qemuarm/zImage $OUTPUT > +cp $BUILDDIR/tmp/deploy/images/qemuarm/xen-qemuarm $OUTPUT > +cp > $BUILDDIR/tmp/deploy/images/qemuarm/xen-image-minimal-qemuarm.tar.bz2 $OUTPUT In this form this is a mistake as it only works for the arm32 Yocto job, while build-yocto.sh is common for all of them. We could either check if this is the arm32 job before copying, or move the copy to build.yaml. I'll fix this in the next version. > ) || return 1 > } > > @@ -238,6 +242,8 @@ Options: > EOF > } > > +OUTPUT=`pwd`/binaries > + > for OPTION in "$@" > do > case ${OPTION} in > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index f62cf21f45..d4a2aa9a5b 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -215,6 +215,7 @@ >artifacts: > paths: >- 'logs/*' > + - binaries/ > when: always >needs: [] > > -- > 2.25.1 >
[linux-linus test] 177298: tolerable trouble: fail/pass/starved - PUSHED
flight 177298 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/177298/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 177222 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 177222 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 177222 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 177222 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 177222 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-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-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 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: linux82eac0c830b7d917bd2a8806eb6ed21ef1e0f84e baseline version: linuxf6feea56f66d34259c4222fa02e8171c4f2673d1 Last test of basis 177222 2023-02-14 00:11:11 Z1 days Testing same since 177298 2023-02-14 17:43:56 Z0 days1 attempts People who touched revisions under test: Linus Torvalds Paolo Bonzini Tom Lendacky jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-coresched-amd64-xlpass test-arm64-arm64-xl pa
[ovmf test] 177328: all pass - PUSHED
flight 177328 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/177328/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 1b5420e8071b4f9ba13136f19365542dfe66bf04 baseline version: ovmf 540522fec06b87bf11ad5624abe23b515f282d60 Last test of basis 177239 2023-02-14 03:48:16 Z0 days Testing same since 177328 2023-02-15 00:12:28 Z0 days1 attempts People who touched revisions under test: Dionna Glaze jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 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/osstest/ovmf.git 540522fec0..1b5420e807 1b5420e8071b4f9ba13136f19365542dfe66bf04 -> xen-tested-master
[xen-4.17-testing test] 177300: tolerable trouble: fail/pass/starved - PUSHED
flight 177300 xen-4.17-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/177300/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 176804 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 176804 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 176804 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 176804 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 176804 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 176804 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 176804 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 176804 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 176804 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-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-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-thunderx 15 migrate-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-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 3685e754e6017c616769b28133286d06bf07b613 baseline version: xen 587823eca162d063027faf1826ec3544f0a06e78 Last test of basis 176804 2023-02-10 01:58:49 Z5 days Testing same since 177300 2023-02-14 18:07:07 Z0 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf
[ovmf test] 177343: all pass - PUSHED
flight 177343 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/177343/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 090642db7ac124c336da72e1954e1fb09e816fb0 baseline version: ovmf 1b5420e8071b4f9ba13136f19365542dfe66bf04 Last test of basis 177328 2023-02-15 00:12:28 Z0 days Testing same since 177343 2023-02-15 03:35:26 Z0 days1 attempts People who touched revisions under test: Abner Chang de...@edk2.groups.io Jeff Brasen Jiangang He Michael D Kinney jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 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/osstest/ovmf.git 1b5420e807..090642db7a 090642db7ac124c336da72e1954e1fb09e816fb0 -> xen-tested-master