[ovmf test] 181127: all pass - PUSHED
flight 181127 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/181127/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf c1dd400a13d1a5e862809c55f6760ba3a944a609 baseline version: ovmf 8fbf857a0b42697c22ec03abda9a2101b2870812 Last test of basis 181117 2023-06-02 16:44:01 Z0 days Testing same since 181127 2023-06-03 00:10:43 Z0 days1 attempts People who touched revisions under test: Gerd Hoffmann Mikolaj Lisik Mikolaj Lisik via groups.io Tom Lendacky 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 8fbf857a0b..c1dd400a13 c1dd400a13d1a5e862809c55f6760ba3a944a609 -> xen-tested-master
[linux-linus test] 181115: regressions - FAIL
flight 181115 linux-linus real [real] flight 181129 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/181115/ http://logs.test-lab.xenproject.org/osstest/logs/181129/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 180278 test-amd64-amd64-freebsd11-amd64 21 guest-start/freebsd.repeat fail REGR. vs. 180278 Tests which did not succeed, but are not blocking: test-armhf-armhf-examine 8 reboot fail like 180278 test-armhf-armhf-xl-multivcpu 8 xen-boot fail like 180278 test-armhf-armhf-xl-arndale 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-credit2 8 xen-boot fail like 180278 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278 test-armhf-armhf-libvirt 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-raw 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-qcow2 8 xen-bootfail like 180278 test-armhf-armhf-xl 8 xen-boot fail like 180278 test-armhf-armhf-xl-vhd 8 xen-boot fail like 180278 test-armhf-armhf-xl-rtds 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-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-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 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-raw 14 migrate-support-checkfail never pass version targeted for testing: linux921bdc72a0d68977092d6a64855a1b8967acc1d9 baseline version: linux6c538e1adbfc696ac4747fb10d63e704344f763d Last test of basis 180278 2023-04-16 19:41:46 Z 47 days Failing since180281 2023-04-17 06:24:36 Z 46 days 88 attempts Testing same since 181115 2023-06-02 15:11:42 Z0 days1 attempts 2596 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass
[qemu-mainline test] 181119: regressions - FAIL
flight 181119 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/181119/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 180691 test-amd64-amd64-libvirt-pair 30 leak-check/check/src_host fail REGR. vs. 180691 test-amd64-amd64-libvirt-pair 31 leak-check/check/dst_host fail REGR. vs. 180691 test-amd64-i386-libvirt 23 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-xsm 23 leak-check/check fail REGR. vs. 180691 build-arm64-xsm 6 xen-buildfail REGR. vs. 180691 build-arm64 6 xen-buildfail REGR. vs. 180691 test-amd64-amd64-libvirt 23 leak-check/check fail REGR. vs. 180691 test-amd64-i386-libvirt-xsm 23 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 21 leak-check/check fail REGR. vs. 180691 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 21 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-vhd 22 leak-check/check fail REGR. vs. 180691 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail REGR. vs. 180691 test-amd64-i386-libvirt-raw 22 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-xl-qcow224 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt 21 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt-qcow2 20 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt-raw 20 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-xl-vhd 20 leak-check/check fail REGR. vs. 180691 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-thunderx 1 build-check(1) blocked n/a test-arm64-arm64-xl-vhd 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180691 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180691 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180691 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180691 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180691 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180691 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180691 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180691 test-amd64-i386-xl-pvshim14 guest-start fail 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-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-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-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm
[xen-unstable-smoke test] 181122: tolerable all pass - PUSHED
flight 181122 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/181122/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 67fdffef9246c82cecd8db28838ed09e79e2528a baseline version: xen 71226054f28ad98ab214b56a15899e8f62a7bca8 Last test of basis 181093 2023-06-01 21:00:28 Z1 days Testing same since 181122 2023-06-02 21:00:24 Z0 days1 attempts People who touched revisions under test: Marek Marczykowski-Górecki Stefano Stabellini jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 71226054f2..67fdffef92 67fdffef9246c82cecd8db28838ed09e79e2528a -> smoke
[xen-unstable test] 181109: tolerable FAIL - PUSHED
flight 181109 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/181109/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181095 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181095 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181095 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181095 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181095 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181095 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181095 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181095 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181095 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181095 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181095 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181095 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail 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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 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-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 71226054f28ad98ab214b56a15899e8f62a7bca8 baseline version: xen 59d0bf62861f5c9b317ccf89f8b5c8b4d19927ad Last test of basis 181095 2023-06-01 22:07:06 Z0 days Testing same since 181109 2023-06-02 09:47:57 Z0 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-amd64-xsm
Re: [PATCH] x86/microcode: Prevent attempting updates known to fail
On 01/06/2023 11:54 am, Andrew Cooper wrote: > Instead, I recommend the following: > > 1) One patch moving the early-cpuid/msr read from tsx_init() into > early_microcode_init(), adjusting the comment as it goes. No point > duplicating that logic, and we need it earlier on boot now. > 2) This patch, adjusting early_microcode_init() only. Have a printk() > saying "microcode loading disabled by firmware" and avoid filling in > ucode_ops. Every other part of ucode handling understands "loading not > available". So, having fallen over "x86/ucode: Exit early from early_update_cache() if loading not available" for other reasons, I've realised that this isn't a completely sensible suggestion. By not filling in ucode_ops, nothing ever calls collect_info(), meaning that external components which peek at this_cpu(cpu_sig).rev get 0's back in place of the actual microcode revision. That's probably the best we can do for genuinely no ucode facilities available. But there's another case we ought to cope with. Some hypervisors deliberately report a microcode revision of ~0, and we should take to mean "no microcode loading available" too. For this MCU_CONTROL_DIS_MCU_LOAD case, we don't want to be trying to load new microcode because that's a waste of time, but we absolutely should query the current microcode revision. It is frequently relevant for security reasons. So I think we want to fine-grain things a little, and separate the concepts of "ucode info available" and "ucode loading available". Per the current mechanism, that would involve supporting a case where ucode_ops.collect_cpu_info() is available but ucode_ops.apply_microcode() is not. ~Andrew P.S. also in our copious free time, we need to start supporting the Intel min_rev field, which is more complicated than it sounds. min_rev is vaguely defined as being relevant to block updates "after you've evaluated CPUID and made decisions based on it", but here in Xen we do also do livepatching and late loading to explicitly make use of newly enumerated features. So we need a way of xen-ucode saying "please really do load this, because I as the admin think it will be fine in combination with the livepatch I'm about to apply". My best idea for this is to have a `--force` option to pass to Xen to skip the revision checks, which will require either a new hypercall, or perhaps borrowing a high bit from the size field in the current hypercall. With a force option in place, the boot time ucode=allow-same can go away. It has become distinctly less useful now that we were forced do this unilaterally on AMD CPUs, and separating "allow same because of HW bugs" from "the Admin promised they knew what they were doing" would be better for testing.
Re: [PATCH] automation: zen3 dom0pvh test
On Wed, May 31, 2023 at 04:49:21PM -0700, Stefano Stabellini wrote:
> From: Stefano Stabellini
>
> Add a PVH Dom0 test for the zen3 runner.
>
> Signed-off-by: Stefano Stabellini
Acked-by: Marek Marczykowski-Górecki
> ---
> automation/gitlab-ci/test.yaml | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index fbe2c0589a..d5cb238b0a 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -202,6 +202,14 @@ zen3p-smoke-x86-64-gcc-debug:
> - *x86-64-test-needs
> - alpine-3.12-gcc-debug
>
> +zen3p-smoke-x86-64-dom0pvh-gcc-debug:
> + extends: .zen3p-x86-64
> + script:
> +- ./automation/scripts/qubes-x86-64.sh dom0pvh 2>&1 | tee ${LOGFILE}
> + needs:
> +- *x86-64-test-needs
> +- alpine-3.12-gcc-debug
> +
> zen3p-pci-hvm-x86-64-gcc-debug:
>extends: .zen3p-x86-64
>script:
> --
> 2.25.1
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
signature.asc
Description: PGP signature
[ovmf test] 181117: all pass - PUSHED
flight 181117 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/181117/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 8fbf857a0b42697c22ec03abda9a2101b2870812 baseline version: ovmf 4354c22f38778a2bb4644d1f1f43a47e4313fe42 Last test of basis 18 2023-06-02 11:40:43 Z0 days Testing same since 181117 2023-06-02 16:44:01 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel 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 4354c22f38..8fbf857a0b 8fbf857a0b42697c22ec03abda9a2101b2870812 -> xen-tested-master
Re: [BROKEN] Re: [PATCH v9 0/5] enable MMU for RISC-V
On Wed, 2023-05-31 at 12:45 +0100, Andrew Cooper wrote: > On 25/05/2023 4:28 pm, Oleksii Kurochko wrote: > > Oleksii Kurochko (5): > > xen/riscv: add VM space layout > > xen/riscv: introduce setup_initial_pages > > xen/riscv: align __bss_start > > xen/riscv: setup initial pagetables > > xen/riscv: remove dummy_bss variable > > These have just been committed. > > But as I fed back to early drafts of this series, patch 2 is > sufficiently fragile and unwise as to be unacceptable in this form. > > enable_mmu() is unsafe in multiple ways, from the compiler reordering > statements (the label needs to be in the asm statement for that to > work > correctly), and because it * depends* on hooking all exceptions and > pagefault. > > Any exception other than pagefault, or not taking a pagefault causes > it > to malfunction, which means you will fail to boot depending on where > Xen > was loaded into memory. It may not explode inside Qemu right now, > but > it will not function reliably in the general case. Linux RISC-V has the similar approach and it doesn't explode but if it will be better to use identity map then I'll re-write it. > > Furthermore, a combination of patch 2 and 4 breaks the CI integration > of > looking for "All set up" at the end of start_xen(). It's not ok, > from a > code quality point of view, to defer 99% of start_xen()'s > functionality > into unrelated function. > > > Please do not do anything else until you've addressed these issues. > enable_mmu() needs to return normally, cont_after_mmu_is_enabled() > needs > deleting entirely, and there needs to be an identity page for Xen to > land on so it isn't jumping into the void and praying not to explode. In this case enable_mmu() should be called before start_xen() function because if start_xen() has local variables then after jumping to VA and removing identity page we will have an issue with stack pointer as it will contain addresses where Xen was loaded. One more enable_mmu() can't have local variable too ( as stack pointer can be somewhere outside one page used for identity mapping ) and it will cause an issue when execute epilogue of enable_mmu() function. Or am I missing something? > > Other minor issues include page.h not having __ASSEMBLY__ guards, > mm.c > locally externing cpu0_boot_stack[] from setup.c when the declaration > needs to be in a header file somewhere, and SPDX tags. Thanks. I'll take it into account. ~ Oleksii
Re: [PULL 05/27] hw/xen: Watches on XenStore transactions
On Tue, 2 May 2023 at 18:08, Peter Maydell wrote:
>
> On Tue, 7 Mar 2023 at 18:27, David Woodhouse wrote:
> >
> > From: David Woodhouse
> >
> > Firing watches on the nodes that still exist is relatively easy; just
> > walk the tree and look at the nodes with refcount of one.
> >
> > Firing watches on *deleted* nodes is more fun. We add 'modified_in_tx'
> > and 'deleted_in_tx' flags to each node. Nodes with those flags cannot
> > be shared, as they will always be unique to the transaction in which
> > they were created.
> >
> > When xs_node_walk would need to *create* a node as scaffolding and it
> > encounters a deleted_in_tx node, it can resurrect it simply by clearing
> > its deleted_in_tx flag. If that node originally had any *data*, they're
> > gone, and the modified_in_tx flag will have been set when it was first
> > deleted.
> >
> > We then attempt to send appropriate watches when the transaction is
> > committed, properly delete the deleted_in_tx nodes, and remove the
> > modified_in_tx flag from the others.
> >
> > Signed-off-by: David Woodhouse
> > Reviewed-by: Paul Durrant
>
> Hi; Coverity's "is there missing error handling?"
> heuristic fired for a change in this code (CID 1508359):
>
> > static int transaction_commit(XenstoreImplState *s, XsTransaction *tx)
> > {
> > +struct walk_op op;
> > +XsNode **n;
> > +
> > if (s->root_tx != tx->base_tx) {
> > return EAGAIN;
> > }
> > @@ -720,10 +861,18 @@ static int transaction_commit(XenstoreImplState *s,
> > XsTransaction *tx)
> > s->root_tx = tx->tx_id;
> > s->nr_nodes = tx->nr_nodes;
> >
> > +init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n);
>
> This is the only call to init_walk_op() which ignores its
> return value. Intentional, or missing error handling?
Hi -- I was going through the unclassified Coverity issues
again today, and this one's still on the list. Is this a
bug, or intentional?
thanks
-- PMM
Re: [PATCH] x86/microcode: Prevent attempting updates known to fail
On 02/06/2023 2:19 pm, Alejandro Vallejo wrote: > On Thu, Jun 01, 2023 at 11:54:31AM +0100, Andrew Cooper wrote: >> I had something else in mind here. Right now, this will read >> MSR_MCU_CONTROL and emit a printk() on every microcode load, which will >> be every AP, and every time the user uses the xen-ucode tool. > Not every AP. The hypercall would return with an error before the APs are > brought in. It is true that the error on dmesg would appear on every > microcode load attempt though. I meant every AP on boot, where Xen initiates the ucode load. > >> Instead, I recommend the following: >> >> 1) One patch moving the early-cpuid/msr read from tsx_init() into >> early_microcode_init(), adjusting the comment as it goes. No point >> duplicating that logic, and we need it earlier on boot now. >> 2) This patch, adjusting early_microcode_init() only. Have a printk() >> saying "microcode loading disabled by firmware" and avoid filling in >> ucode_ops. Every other part of ucode handling understands "loading not >> available". > Sure. Going on a tangent though, I do wonder why tsx_init() is preceding > identify_cpu(). It's reading cpuid leaf 7d0 simply because it hasn't been > read yet, but it's not obvious why this rush in invoking tsx_init(). I > can't see any obvious marker that affect the following identify_cpu() call, > and swapping them gets rid of the cpuid read. In __start_xen(), tsx_init(); /* Needs microcode. May change HLE/RTM feature bits. */ If you were to test such a patch, the test-tsx ought to fail on SKL/KBL amongst others. One of the things that tsx_init() does is select TSX_CTRL_CPUID_CLEAR and/or TSX_CPUID_CLEAR, which hides the HLE and RTM bits in regular CPUID, so wants to run before the general CPUID scan. This matters for guest performance - if TSX is actually always aborting, but reported to the guest, then any library using RTM will be less performant than using the non-transactional path. Conversely if the user wants to explicitly re-activate TSX despite the firmware defaults, those bits need clearing before the CPUID scan for anything to work. ~Andrew
[qemu-mainline test] 181103: regressions - FAIL
flight 181103 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/181103/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 180691 test-amd64-amd64-libvirt-pair 30 leak-check/check/src_host fail REGR. vs. 180691 test-amd64-amd64-libvirt-pair 31 leak-check/check/dst_host fail REGR. vs. 180691 test-amd64-i386-libvirt 23 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-xsm 23 leak-check/check fail REGR. vs. 180691 build-arm64-xsm 6 xen-buildfail REGR. vs. 180691 build-arm64 6 xen-buildfail REGR. vs. 180691 test-amd64-amd64-libvirt 23 leak-check/check fail REGR. vs. 180691 test-amd64-i386-libvirt-xsm 23 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 21 leak-check/check fail REGR. vs. 180691 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 21 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-libvirt-vhd 22 leak-check/check fail REGR. vs. 180691 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail REGR. vs. 180691 test-amd64-i386-libvirt-raw 22 leak-check/check fail REGR. vs. 180691 test-amd64-amd64-xl-qcow224 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt 21 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt-qcow2 20 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-libvirt-raw 20 leak-check/check fail REGR. vs. 180691 test-armhf-armhf-xl-vhd 20 leak-check/check fail REGR. vs. 180691 test-amd64-i386-xl-qemuu-win7-amd64 12 windows-install fail REGR. vs. 180691 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-thunderx 1 build-check(1) blocked n/a test-arm64-arm64-xl-vhd 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180691 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180691 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180691 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180691 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180691 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180691 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180691 test-amd64-i386-xl-pvshim14 guest-start fail 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-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-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-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 02/06/2023 5:08 pm, Alejandro Vallejo wrote: > On Fri, Jun 02, 2023 at 03:22:20PM +0100, Andrew Cooper wrote: >> Linux deals with this in verify_cpu() (early asm) along with a FMS check >> protecting the access to MSR_MISC_ENABLE, rather than using rdmsr_safe() >> and catching the #GP. > On a related note, we don't use rdmsr_safe() either. We just hope it exists > on any Intel CPU. It fortunately does on any Intel CPU we care about > because it was introduced shortly before Pentium 4 (Netburst), so we're > fine since we mandate long mode. Oh, good point. Yeah, that's fine, but only try reading it in the case that we've found LM, not NX, and GenuineIntel. There are old versions of Xen which don't emulate the MSR at all, and the only reason Xen does emulate it in all guests is for a CPUID-faulting corner case. The same assumptions are unlikely to hold for other virtualised cases. Failing with a clear "NX not available" is strictly preferable to triple faulting. ~Andrew
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On Fri, Jun 02, 2023 at 03:22:20PM +0100, Andrew Cooper wrote: > On 01/06/2023 6:43 pm, Alejandro Vallejo wrote: > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > > index 09bebf8635..8414266281 100644 > > --- a/xen/arch/x86/boot/head.S > > +++ b/xen/arch/x86/boot/head.S > > @@ -647,11 +653,18 @@ trampoline_setup: > > cpuid > > 1: mov %edx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM) + > > sym_esi(boot_cpu_data) > > > > -/* Check for NX. Adjust EFER setting if available. */ > > +/* > > + * Check for NX: > > + * - If Xen was compiled requiring it simply assert it's > > + * supported. The trampoline already has the right constant. > > + * - Otherwise, update the trampoline EFER mask accordingly. > > + */ > > bt $cpufeat_bit(X86_FEATURE_NX), %edx > > -jnc 1f > > +jnc no_nx_bit > > +#if !IS_ENABLED(CONFIG_REQUIRE_NX_BIT) > > orb $EFER_NXE >> 8, 1 + sym_esi(trampoline_efer) > > -1: > > +no_nx_bit: > > +#endif > > It occurs to me... This will prevent Xen booting in firmware > configurations where XD-Disable is active, despite Xen having > unconditional logic to turn XD off later in boot. In practice setting/clearing that bit was done through a BIOS configuration knob AFAIR, so I wouldn't be too worried about forcing it open. > > Linux deals with this in verify_cpu() (early asm) along with a FMS check > protecting the access to MSR_MISC_ENABLE, rather than using rdmsr_safe() > and catching the #GP. > On a related note, we don't use rdmsr_safe() either. We just hope it exists on any Intel CPU. It fortunately does on any Intel CPU we care about because it was introduced shortly before Pentium 4 (Netburst), so we're fine since we mandate long mode. > > In terms of which CPUs are a problem, we almost got very lucky. NX is > part of the AMD64 spec, and all AMD, VIA, Centaur and Intel Atoms have > this property. 64bit and XD were both added midway through the Pentium > 4 era, and appear in the Prescott E0 stepping. > > However, it appears that the prior stepping, D0, had 64bit but was only > unlocked for certain OEMs. (At the time, Intel were still trying to > push Itaniaum as the future, and were trying hard not to go the x86_64 > route.) > > Specifically, > https://en.wikipedia.org/wiki/List_of_Intel_Xeon_processors_(NetBurst-based)#%22Nocona%22_(90_nm) > is the suspected problem set. > > > So, I think this does want to turn into a series, with the first patch > moving the XD-disable logic into this path, I agree. Will do. > after which I think there is > a strong case to be made for defaulting CONFIG_REQUIRE_NX to yes. > > ~Andrew A machine with long mode and no NX would be exceedingly rare, that's for sure. Alejandro
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
Hi Roger, On 02/06/2023 16:24, Roger Pau Monné wrote: On Fri, Jun 02, 2023 at 02:46:03PM +0100, Ayan Kumar Halder wrote: Hi Roger, On 02/06/2023 12:43, Roger Pau Monné wrote: On Fri, Jun 02, 2023 at 09:48:48AM +0100, Ayan Kumar Halder wrote: Hi Xen developers, We are trying to better document xen project development processes and related tools. At present, we are targeting **x86 and Arm** only. These tools range from bug/change request tracking means, compilers, infra, editors, code-review tools, etc which is connected in some way to the Xen development and is being currently used by xen-devel community. What is the end goal of this? We are trying to do an initial assesment of the requirements for Xen functional safety. As a first step, I am trying to make a list tools which are in someways related to Xen development/testing/deployment. I'm kind of unsure why do you care about which editor I use to generate my code, that's up to the developer. I agree that editor, email-clients are something that are an individual developer's choice. However as it is related to Xen development, we want to atleast put down some of the commonly used tools. At a later state when (and if) we go through the list with a safety assessor, we might prune some of these items. I have very little idea about what's required for a safety assessor, sorry. Same here :) Will this have an impact on what tools are allowed to be used when working with certain parts of Xen? (the safety certifiable parts I would assume) At the moment, I am not very sure. At the least, our first step is to gather the list of existing tools. I appreciate if you can let me know anything I missed or mistaken and the version currently being used (for some of the tools). 1. Code management portal - xenbits (https://xenbits.xenproject.org), gitlab (https://gitlab.com/xen-project/xen) 2. Project description - wiki.xenproject.org 3. Project management - gitlab 4. Code review - text based email clients (mutt, thunderbird), git-email, b4 5. Text Editors such as vim, emacs 6. Code review history - xen-devel mail archives 7. Code revision management - git 8. Xen coding language - C89, C99, Kconfig assembly (gas), python, perl, shell, Makefile, bison, flex, ocaml, go... Likely more that I've missed. Ack 9. Testing tools for Arm64 in gitlab CI compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) binutils - GNU Binutils for Debian) 2.38.9 emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) dom0/domU kernel - kernel-5.19.0 rootfs - alpine-3.12-arm64-rootfs firmware - U-Boot 2022.10 10. Testing tools for Arm in gitlab CI compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, arm-linux-gnueabihf-gcc (Debian 12.2.0-14) 12.2.0 (most commonly used versions) emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from Debian) rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz firmware - U-Boot 2022.10 11. Testing tools for x86 compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, clang (from Debian) (most commonly used version) binutils - GNU ld (GNU Binutils for Debian) 2.40) emulator/hardware - Qubes HW (**need details regarding machine, firmware, etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) dom0/domU kernel - kernel 6.1.19 rootfs - alpine-3.12-rootfs firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by EDK II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB 2.06~rc1 I do use an LLVM based toolstack, so that's usually latest LLVM import on FreeBSD. We do also test this on the cirrus-ci, see: https://github.com/royger/xen/runs/5334480206 Thanks, this is interesting info. For the moment, I am ignoring the downstream forks of Xen. That's not a fork of Xen, just plain Xen hosted on my personal github repo. Ok I am only considering the tools used by the upstream Xen and the associated CI/CD. Gitlab CI does test with LLVM toolchain also. osstest does test FreeBSD guests, but no FreeBSD dom0. Ok I_n any case I think the scope to some of the questions is unknown, it's not feasible to expect to list every possible combination of Linux versions vs Xen version vs whatever guests versions a given developer might be running. I agree . That is the reason I am picking up the compiler, linux, binutils, firmware, etc versions from our gitlab CI. It also acts as a proof that we are testing Xen against a known set of compiler, linux versions, etc. OK, so the question is not what every developers uses, but you trying to narrow down the scope to a specific environment? Yes, I am trying to do 3 things here :- 1. Gather the most commonly used developer tools - For eg vim, emacs, mutt, thunderbird 2. Gather the current tools used in any existing upstream Xen testing - For eg gitlab tests, OS
Re: [PATCH v2 3/3] x86/cpu-policy: Derive RSBA/RRSBA for guest policies
On 02/06/2023 4:29 pm, Andrew Cooper wrote: > On 02/06/2023 11:20 am, Jan Beulich wrote: >> On 01.06.2023 16:48, Andrew Cooper wrote: >> What about a tool stack request leading to us setting the 2nd of the two >> bits here, while the other was already set? IOW wouldn't we better clear >> the other bit explicitly? (Due to the EIBRS dependency or RRSBA I think >> this can really only happen when the tool stack requests RSBA+EIBRS, as >> the deep deps clearing doesn't know the concept of "negative" >> dependencies.) > Hmm - I think there is a bug here, but it's not this simple. I think > the only reasonable thing we can do is start rejecting bad input because > I don't think Xen can fix up safely. > > Xen must not ever clear RSBA, or we've potentially made the VM unsafe > behind the toolstack's back. > > If EIBRS != RRSBA, the toolstack has made a mistake. Equally too for > RSBA && EIBRS. > > I think this is going to take more coffee to solve... Actually, no. I'm going to delete the hunk modifying recalculate_cpuid(), and move this patch back to the meaning it had in v1 which is just "get the policies looking correct". It's still not supported for the toolstack to request ARCH_CAPS (the "a" marking), and the safely logic for that can come in a subsequent series along with the unit(ish) testing I was already planning to do. ~Andrew
Re: [PATCH v2 3/3] x86/cpu-policy: Derive RSBA/RRSBA for guest policies
On 02/06/2023 11:20 am, Jan Beulich wrote:
> On 01.06.2023 16:48, Andrew Cooper wrote:
>> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
>> predictors when empty. From a practical point of view, this mean "Retpoline
>> not safe".
>>
>> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
>> statement that IBRS is implemented in hardware (as opposed to the form
>> retrofitted to existing CPUs in microcode).
>>
>> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
>> property that predictions are tagged with the mode in which they were learnt.
>> Therefore, it means "when eIBRS is active, the RSB may fall back to
>> alternative predictors but restricted to the current prediction mode". As
>> such, it's stronger statement than RSBA, but still means "Retpoline not
>> safe".
>>
>> CPUs are not expected to enumerate both RSBA and RRSBA.
>>
>> Add feature dependencies for EIBRS and RRSBA. While technically they're not
>> linked, absolutely nothing good can of letting the guest see RRSBA without
> Nit: missing "come"?
Yes. Will fix.
>> @@ -786,6 +825,20 @@ void recalculate_cpuid_policy(struct domain *d)
>>
>> sanitise_featureset(fs);
>>
>> +/*
>> + * If the host suffers from RSBA of any form, and the guest can see
>> + * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the
>> guest
>> + * depending on the visibility of eIBRS.
>> + */
>> +if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
>> + (cpu_has_rsba || cpu_has_rrsba) )
>> +{
>> +bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
>> +
>> +__set_bit(eibrs ? X86_FEATURE_RRSBA
>> +: X86_FEATURE_RSBA, fs);
>> +}
> Now that we have the same code and comment even a 3rd time, surely this
> wants to be put in a helper?
I did consider that, and chose not to in this case.
One of these is going to disappear again in due course, when we start
handing errors back to the toolstack instead of fixing up behind it.
The requirement to be after sanitise_featureset() is critically
important here for safety, and out-of-lining makes that connection less
obvious.
I considered having guest_common_default_late_feature_adjustments(), but
that name is getting silly and it's already somewhat hard to navigate.
There's quite a bit of other cleanup which ought to be done, like
uniformly adding new bits first, then taking bits away (I suffered two
bugs in init_dom0_cpuid_policy() getting this wrong during development),
so I was planning to leave any decisions until then.
> What about a tool stack request leading to us setting the 2nd of the two
> bits here, while the other was already set? IOW wouldn't we better clear
> the other bit explicitly? (Due to the EIBRS dependency or RRSBA I think
> this can really only happen when the tool stack requests RSBA+EIBRS, as
> the deep deps clearing doesn't know the concept of "negative"
> dependencies.)
Hmm - I think there is a bug here, but it's not this simple. I think
the only reasonable thing we can do is start rejecting bad input because
I don't think Xen can fix up safely.
Xen must not ever clear RSBA, or we've potentially made the VM unsafe
behind the toolstack's back.
If EIBRS != RRSBA, the toolstack has made a mistake. Equally too for
RSBA && EIBRS.
I think this is going to take more coffee to solve...
> Similarly what about a tool stack (and ultimately admin) request setting
> both RSBA and RRSBA? Wouldn't we better clear the respectively wrong bit
> then? People may do this in their guest configs "just to be on the safe
> side" (once expressing this in guest configs is possible, of course),
> and due to the max policies having both bits set this also won't occur
> "automatically".
The only reason this series doesn't have a final patch turning
ARCH_CAPS's "a" into "A" is because libxl can't currently operate these
bits at all, let alone safely. Roger is kindly looking into that side
of things.
It is an error to be modifying bits behind the toolstack's back to start
with. We get away with it previously because hiding bits that the
toolstack thinks the guest saw is goes in the safe direction WRT
migrate. But no more with the semantics of RSBA/RRSBA.
I explicitly don't care about people wanting to set RSBA && RRSBA "just
in case" - this is too complicated already. The only non-default thing
an admin needs to be able to express is +rsba,-eibrs,-rrsba to mean
"please be compatible with pre-EIBRS hardware". (In reality, there will
also need to be some FOO_NO bits taken out too, depending on the CPUs in
question.)
~Andrew
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
On Fri, Jun 02, 2023 at 02:46:03PM +0100, Ayan Kumar Halder wrote: > Hi Roger, > > On 02/06/2023 12:43, Roger Pau Monné wrote: > > On Fri, Jun 02, 2023 at 09:48:48AM +0100, Ayan Kumar Halder wrote: > > > Hi Xen developers, > > > > > > We are trying to better document xen project development processes and > > > related tools. At present, we are targeting **x86 and Arm** only. > > > > > > These tools range from bug/change request tracking means, compilers, > > > infra, > > > editors, code-review tools, etc which is connected in some way to the Xen > > > development and is being currently used by xen-devel community. > > What is the end goal of this? > > We are trying to do an initial assesment of the requirements for Xen > functional safety. > > As a first step, I am trying to make a list tools which are in someways > related to Xen development/testing/deployment. > > > > > I'm kind of unsure why do you care about which editor I use to > > generate my code, that's up to the developer. > > I agree that editor, email-clients are something that are an individual > developer's choice. > > However as it is related to Xen development, we want to atleast put down > some of the commonly used tools. > > At a later state when (and if) we go through the list with a safety > assessor, we might prune some of these items. I have very little idea about what's required for a safety assessor, sorry. Will this have an impact on what tools are allowed to be used when working with certain parts of Xen? (the safety certifiable parts I would assume) > > > > > I appreciate if you can let me know anything I missed or mistaken and the > > > version currently being used (for some of the tools). > > > > > > > > > 1. Code management portal - xenbits (https://xenbits.xenproject.org), > > > gitlab > > > (https://gitlab.com/xen-project/xen) > > > > > > 2. Project description - wiki.xenproject.org > > > > > > 3. Project management - gitlab > > > > > > 4. Code review - text based email clients (mutt, thunderbird), git-email, > > > b4 > > > > > > 5. Text Editors such as vim, emacs > > > > > > 6. Code review history - xen-devel mail archives > > > > > > 7. Code revision management - git > > > > > > 8. Xen coding language - C89, C99, Kconfig > > assembly (gas), python, perl, shell, Makefile, bison, flex, ocaml, > > go... > > > > Likely more that I've missed. > Ack > > > > > 9. Testing tools for Arm64 in gitlab CI > > > > > > compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) > > > > > > binutils - GNU Binutils for Debian) 2.38.9 > > > > > > emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky > > > disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) > > > > > > dom0/domU kernel - kernel-5.19.0 > > > > > > rootfs - alpine-3.12-arm64-rootfs > > > > > > firmware - U-Boot 2022.10 > > > > > > 10. Testing tools for Arm in gitlab CI > > > > > > compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, > > > arm-linux-gnueabihf-gcc > > > (Debian 12.2.0-14) 12.2.0 (most commonly used versions) > > > > > > emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) > > > > > > dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from > > > Debian) > > > > > > rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz > > > > > > firmware - U-Boot 2022.10 > > > > > > 11. Testing tools for x86 > > > > > > compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, > > > clang (from Debian) (most commonly used version) > > > > > > binutils - GNU ld (GNU Binutils for Debian) 2.40) > > > > > > emulator/hardware - Qubes HW (**need details regarding machine, firmware, > > > etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) > > > > > > dom0/domU kernel - kernel 6.1.19 > > > > > > rootfs - alpine-3.12-rootfs > > > > > > firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by > > > EDK > > > II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB > > > 2.06~rc1 > > I do use an LLVM based toolstack, so that's usually latest LLVM import > > on FreeBSD. We do also test this on the cirrus-ci, see: > > > > https://github.com/royger/xen/runs/5334480206 > > Thanks, this is interesting info. > > For the moment, I am ignoring the downstream forks of Xen. That's not a fork of Xen, just plain Xen hosted on my personal github repo. > I am only considering the tools used by the upstream Xen and the associated > CI/CD. Gitlab CI does test with LLVM toolchain also. osstest does test FreeBSD guests, but no FreeBSD dom0. > > > > I_n any case I think the scope to some of the questions is unknown, > > it's not feasible to expect to list every possible combination of > > Linux versions vs Xen version vs whatever guests versions a given > > developer might be running. > > I agree . That is the reason I am picking up the compiler, linux, binutils, > firmware, etc versions from our gitlab CI. > > It also acts as a proof that we ar
Re: [RFC PATCH v1 0/9] Hypervisor-Enforced Kernel Integrity
On 31/05/2023 22:24, Sean Christopherson wrote: On Tue, May 30, 2023, Rick P Edgecombe wrote: On Fri, 2023-05-26 at 17:22 +0200, Micka�l Sala�n wrote: Can the guest kernel ask the host VMM's emulated devices to DMA into the protected data? It should go through the host userspace mappings I think, which don't care about EPT permissions. Or did I miss where you are protecting that another way? There are a lot of easy ways to ask the host to write to guest memory that don't involve the EPT. You probably need to protect the host userspace mappings, and also the places in KVM that kmap a GPA provided by the guest. Good point, I'll check this confused deputy attack. Extended KVM protections should indeed handle all ways to map guests' memory. I'm wondering if current VMMs would gracefully handle such new restrictions though. I guess the host could map arbitrary data to the guest, so that need to be handled, but how could the VMM (not the host kernel) bypass/update EPT initially used for the guest (and potentially later mapped to the host)? Well traditionally both QEMU and KVM accessed guest memory via host mappings instead of the EPT.�So I'm wondering what is stopping the guest from passing a protected gfn when setting up the DMA, and QEMU being enticed to write to it? The emulator as well would use these host userspace mappings and not consult the EPT IIRC. I think Sean was suggesting host userspace should be more involved in this process, so perhaps it could protect its own alias of the protected memory, for example mprotect() it as read-only. Ya, though "suggesting" is really "demanding, unless someone provides super strong justification for handling this directly in KVM". It's basically the same argument that led to Linux Security Modules: I'm all for KVM providing the framework and plumbing, but I don't want KVM to get involved in defining policy, thread models, etc. I agree that KVM should not provide its own policy but only the building blocks to enforce one. There is two complementary points: - policy definition by the guest, provided to KVM and the host; - policy enforcement by KVM and the host. A potential extension of this framework could be to enable the host to define it's own policy for guests, but this would be a different threat model. To avoid too much latency because of the host being involved in policy enforcement, I'd like to explore an asynchronous approach that would especially fit well for dynamic restrictions.
[linux-linus test] 181100: regressions - FAIL
flight 181100 linux-linus real [real] flight 181113 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/181100/ http://logs.test-lab.xenproject.org/osstest/logs/181113/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 180278 test-amd64-amd64-xl-vhd 21 guest-start/debian.repeat fail REGR. vs. 180278 Tests which are failing intermittently (not blocking): test-amd64-amd64-dom0pvh-xl-intel 20 guest-localmigrate/x10 fail pass in 181113-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-examine 8 reboot fail like 180278 test-armhf-armhf-xl-arndale 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-credit2 8 xen-boot fail like 180278 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-multivcpu 8 xen-boot fail like 180278 test-armhf-armhf-libvirt 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-raw 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-qcow2 8 xen-bootfail like 180278 test-armhf-armhf-xl 8 xen-boot fail like 180278 test-armhf-armhf-xl-vhd 8 xen-boot fail like 180278 test-armhf-armhf-xl-rtds 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-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-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 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-raw 14 migrate-support-checkfail never pass version targeted for testing: linux9e87b63ed37e202c77aa17d4112da6ae0c7c097c baseline version: linux6c538e1adbfc696ac4747fb10d63e704344f763d Last test of basis 180278 2023-04-16 19:41:46 Z 46 days Failing since180281 2023-04-17 06:24:36 Z 46 days 87 attempts Testing same since 181100 2023-06-02 03:25:50 Z0 days1 attempts 2584 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass buil
[ovmf test] 181111: all pass - PUSHED
flight 18 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/18/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 4354c22f38778a2bb4644d1f1f43a47e4313fe42 baseline version: ovmf 78262899d225eb30e5fbe6a88e85a4b1d8c04a61 Last test of basis 181106 2023-06-02 07:40:45 Z0 days Testing same since 18 2023-06-02 11:40:43 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Gerd Hoffmann Marcin Juszkiewicz 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 78262899d2..4354c22f38 4354c22f38778a2bb4644d1f1f43a47e4313fe42 -> xen-tested-master
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 01/06/2023 6:43 pm, Alejandro Vallejo wrote: > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index 09bebf8635..8414266281 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -647,11 +653,18 @@ trampoline_setup: > cpuid > 1: mov %edx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM) + > sym_esi(boot_cpu_data) > > -/* Check for NX. Adjust EFER setting if available. */ > +/* > + * Check for NX: > + * - If Xen was compiled requiring it simply assert it's > + * supported. The trampoline already has the right constant. > + * - Otherwise, update the trampoline EFER mask accordingly. > + */ > bt $cpufeat_bit(X86_FEATURE_NX), %edx > -jnc 1f > +jnc no_nx_bit > +#if !IS_ENABLED(CONFIG_REQUIRE_NX_BIT) > orb $EFER_NXE >> 8, 1 + sym_esi(trampoline_efer) > -1: > +no_nx_bit: > +#endif It occurs to me... This will prevent Xen booting in firmware configurations where XD-Disable is active, despite Xen having unconditional logic to turn XD off later in boot. Linux deals with this in verify_cpu() (early asm) along with a FMS check protecting the access to MSR_MISC_ENABLE, rather than using rdmsr_safe() and catching the #GP. In terms of which CPUs are a problem, we almost got very lucky. NX is part of the AMD64 spec, and all AMD, VIA, Centaur and Intel Atoms have this property. 64bit and XD were both added midway through the Pentium 4 era, and appear in the Prescott E0 stepping. However, it appears that the prior stepping, D0, had 64bit but was only unlocked for certain OEMs. (At the time, Intel were still trying to push Itaniaum as the future, and were trying hard not to go the x86_64 route.) Specifically, https://en.wikipedia.org/wiki/List_of_Intel_Xeon_processors_(NetBurst-based)#%22Nocona%22_(90_nm) is the suspected problem set. So, I think this does want to turn into a series, with the first patch moving the XD-disable logic into this path, after which I think there is a strong case to be made for defaulting CONFIG_REQUIRE_NX to yes. ~Andrew
[libvirt test] 181102: tolerable all pass - PUSHED
flight 181102 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/181102/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181066 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181066 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181066 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-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-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-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: libvirt 1c7335add9e28637d8a8b5039f487e5dc6a591c2 baseline version: libvirt c47e17689e3309e544b59f5a9eb7b9d668967787 Last test of basis 181066 2023-06-01 04:18:54 Z1 days Testing same since 181102 2023-06-02 04:20:20 Z0 days1 attempts People who touched revisions under test: Jiri Denemark Ján Tomko Michal Privoznik Temuri Doghonadze jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass 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 pass 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 pass test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw pass 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 Explanatio
Re: [PATCH v2 2/3] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate
On 02/06/2023 10:56 am, Jan Beulich wrote:
> On 01.06.2023 16:48, Andrew Cooper wrote:
>> @@ -593,15 +596,85 @@ static bool __init retpoline_calculations(void)
>> return false;
>>
>> /*
>> - * RSBA may be set by a hypervisor to indicate that we may move to a
>> - * processor which isn't retpoline-safe.
>> + * The meaning of the RSBA and RRSBA bits have evolved over time. The
>> + * agreed upon meaning at the time of writing (May 2023) is thus:
>> + *
>> + * - RSBA (RSB Alternative) means that an RSB may fall back to an
>> + * alternative predictor on underflow. Skylake uarch and later all
>> have
>> + * this property. Broadwell too, when running microcode versions
>> prior
>> + * to Jan 2018.
>> + *
>> + * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
>> + * tagging of predictions with the mode in which they were learned.
>> So
>> + * when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
>> + *
>> + * - CPUs are not expected to enumerate both RSBA and RRSBA.
>> + *
>> + * Some parts (Broadwell) are not expected to ever enumerate this
>> + * behaviour directly. Other parts have differing enumeration with
>> + * microcode version. Fix up Xen's idea, so we can advertise them
>> safely
>> + * to guests, and so toolstacks can level a VM safety for migration.
>> + *
>> + * The following states exist:
>> + *
>> + * | | RSBA | EIBRS | RRSBA | Notes | Action|
>> + * |---+--+---+---++---|
>> + * | 1 |0 | 0 | 0 | OK (older parts) | Maybe +RSBA |
>> + * | 2 |0 | 0 | 1 | Broken | +RSBA, -RRSBA |
>> + * | 3 |0 | 1 | 0 | OK (pre-Aug ucode) | +RRSBA|
>> + * | 4 |0 | 1 | 1 | OK | |
>> + * | 5 |1 | 0 | 0 | OK | |
>> + * | 6 |1 | 0 | 1 | Broken | -RRSBA|
>> + * | 7 |1 | 1 | 0 | Broken | -RSBA, +RRSBA |
>> + * | 8 |1 | 1 | 1 | Broken | -RSBA |
>> *
>> + * However, we doesn't need perfect adherence to the spec. Identify the
> Nit: "don't" or "it"?
Oops. This used to read "Xen doesn't need". So much for last minute
changes.
>
>> + * broken cases (so we stand a chance of spotting violated assumptions),
>> + * and fix up Rows 1 and 3 so Xen can use RSBA || RRSBA to identify
>> + * "alternative predictors potentially in use".
> Considering that it's rows 2, 6, 7, and 8 which are broken, I find this
> comment a little misleading. To me it doesn't become clear whether them
> subsequently being left alone (and merely a log message issued) is
> intentional.
It is intentional.
I don't know if these combinations exist in practice anywhere or not.
Intel think they oughtn't to, and it's quite possible that the printk()
is unreachable, but given the complexity and shifting meanings over time
here, I think it would be unwise to simply assume this to be true.
But at the same time, if it is an unreachable code, it would be equally
unwise to have a load of fixup code which we can't test. I've still got
the fixup code in a separate patch incase we need to put it back in.
I have checked that this printk() doesn't trigger for any of the CPUs
and microcode combinations I have easily to hand, but it's not an
exhaustive test.
>
>> + */
>> +if ( cpu_has_eibrs ? cpu_has_rsba /* Rows 7, 8 */
>> + : cpu_has_rrsba /* Rows 2, 6 */ )
>> +printk(XENLOG_ERR
>> + "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u,
>> EIBRS %u, RRSBA %u\n",
>> + boot_cpu_data.x86, boot_cpu_data.x86_model,
>> + boot_cpu_data.x86_mask, ucode_rev,
>> + cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);
>> +
>> +/*
>> * Processors offering Enhanced IBRS are not guarenteed to be
>> * repoline-safe.
>> */
>> -if ( cpu_has_rsba || cpu_has_eibrs )
>> +if ( cpu_has_eibrs )
>> +{
>> +/*
>> + * Prior to the August 2023 microcode, many eIBRS-capable parts did
>> + * not enumerate RRSBA.
>> + */
>> +if ( !cpu_has_rrsba )
>> +setup_force_cpu_cap(X86_FEATURE_RRSBA);
>> +
>> +return false;
>> +}
> No clearing of RSBA in this case? I fear we may end up with misbehavior if
> our own records aren't kept consistent with our assumptions. (This then
> extends to the "just a log message" above as well.)
Well quite, which is why I've gone to lengths to state what our
assumptions are.
Right now, there is nothing in Xen itself where RSBA vs RRSBA matters.
Until this patch, we don't even have cpu_has_rrsba, and remember that
Xen was not vulnerable to CVE-2022-29901 (Intel Retbleed) because
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
Hi Roger, On 02/06/2023 12:43, Roger Pau Monné wrote: On Fri, Jun 02, 2023 at 09:48:48AM +0100, Ayan Kumar Halder wrote: Hi Xen developers, We are trying to better document xen project development processes and related tools. At present, we are targeting **x86 and Arm** only. These tools range from bug/change request tracking means, compilers, infra, editors, code-review tools, etc which is connected in some way to the Xen development and is being currently used by xen-devel community. What is the end goal of this? We are trying to do an initial assesment of the requirements for Xen functional safety. As a first step, I am trying to make a list tools which are in someways related to Xen development/testing/deployment. I'm kind of unsure why do you care about which editor I use to generate my code, that's up to the developer. I agree that editor, email-clients are something that are an individual developer's choice. However as it is related to Xen development, we want to atleast put down some of the commonly used tools. At a later state when (and if) we go through the list with a safety assessor, we might prune some of these items. I appreciate if you can let me know anything I missed or mistaken and the version currently being used (for some of the tools). 1. Code management portal - xenbits (https://xenbits.xenproject.org), gitlab (https://gitlab.com/xen-project/xen) 2. Project description - wiki.xenproject.org 3. Project management - gitlab 4. Code review - text based email clients (mutt, thunderbird), git-email, b4 5. Text Editors such as vim, emacs 6. Code review history - xen-devel mail archives 7. Code revision management - git 8. Xen coding language - C89, C99, Kconfig assembly (gas), python, perl, shell, Makefile, bison, flex, ocaml, go... Likely more that I've missed. Ack 9. Testing tools for Arm64 in gitlab CI compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) binutils - GNU Binutils for Debian) 2.38.9 emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) dom0/domU kernel - kernel-5.19.0 rootfs - alpine-3.12-arm64-rootfs firmware - U-Boot 2022.10 10. Testing tools for Arm in gitlab CI compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, arm-linux-gnueabihf-gcc (Debian 12.2.0-14) 12.2.0 (most commonly used versions) emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from Debian) rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz firmware - U-Boot 2022.10 11. Testing tools for x86 compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, clang (from Debian) (most commonly used version) binutils - GNU ld (GNU Binutils for Debian) 2.40) emulator/hardware - Qubes HW (**need details regarding machine, firmware, etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) dom0/domU kernel - kernel 6.1.19 rootfs - alpine-3.12-rootfs firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by EDK II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB 2.06~rc1 I do use an LLVM based toolstack, so that's usually latest LLVM import on FreeBSD. We do also test this on the cirrus-ci, see: https://github.com/royger/xen/runs/5334480206 Thanks, this is interesting info. For the moment, I am ignoring the downstream forks of Xen. I am only considering the tools used by the upstream Xen and the associated CI/CD. I_n any case I think the scope to some of the questions is unknown, it's not feasible to expect to list every possible combination of Linux versions vs Xen version vs whatever guests versions a given developer might be running. I agree . That is the reason I am picking up the compiler, linux, binutils, firmware, etc versions from our gitlab CI. It also acts as a proof that we are testing Xen against a known set of compiler, linux versions, etc. - Ayan Regards, Roger.
Re: [PATCH] x86/microcode: Prevent attempting updates known to fail
On Thu, Jun 01, 2023 at 11:54:31AM +0100, Andrew Cooper wrote: > I had something else in mind here. Right now, this will read > MSR_MCU_CONTROL and emit a printk() on every microcode load, which will > be every AP, and every time the user uses the xen-ucode tool. Not every AP. The hypercall would return with an error before the APs are brought in. It is true that the error on dmesg would appear on every microcode load attempt though. > > Instead, I recommend the following: > > 1) One patch moving the early-cpuid/msr read from tsx_init() into > early_microcode_init(), adjusting the comment as it goes. No point > duplicating that logic, and we need it earlier on boot now. > 2) This patch, adjusting early_microcode_init() only. Have a printk() > saying "microcode loading disabled by firmware" and avoid filling in > ucode_ops. Every other part of ucode handling understands "loading not > available". Sure. Going on a tangent though, I do wonder why tsx_init() is preceding identify_cpu(). It's reading cpuid leaf 7d0 simply because it hasn't been read yet, but it's not obvious why this rush in invoking tsx_init(). I can't see any obvious marker that affect the following identify_cpu() call, and swapping them gets rid of the cpuid read. > > In terms of the commit message, you should call out the usecase > explicitly. This feature is intended for baremetal clouds where the > platform owner doesn't trust the tenant to choose the microcode version > in use. > Sure. > Also, I'm really not sure what your 3rd paragraph is trying to say. That the case where CPU_i.DIS_MCU_LOAD != CPU_j.DIS_MCU_LOAD where i != j is not specifically handled on the grounds that sane firmware ensures that condition doesn't happen, and we already notify when the system reached a nonsensical state with different CPUs having different microcode versions. I'll rewrite it better. Alejandro
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 02.06.2023 14:05, Andrew Cooper wrote: > On 01/06/2023 6:43 pm, Alejandro Vallejo wrote: >> @@ -151,6 +152,11 @@ not_multiboot: >> .Lnot_aligned: >> add $sym_offs(.Lbag_alg_msg),%esi # Error message >> jmp .Lget_vtb >> +#if IS_ENABLED(CONFIG_REQUIRE_NX_BIT) > > This doesn't need to be IS_ENABLED(). #if will DTRT for a non-existent > symbol by considering such to be 0. And then it really should be #ifdef, not #if (like for all "real" Kconfig symbols). Jan
[XEN v8 5/5] xen/arm: p2m: Enable support for 32bit IPA for ARM_32
Refer ARM DDI 0406C.d ID040418, B3-1345,
"A stage 2 translation with an input address range of 31-34 bits can
start the translation either:
- With a first-level lookup, accessing a first-level translation
table with 2-16 entries.
- With a second-level lookup, accessing a set of concatenated
second-level translation tables"
Thus, for 32 bit IPA, there will be no concatenated root level tables.
So, the root-order is 0.
Also, Refer ARM DDI 0406C.d ID040418, B3-1348
"Determining the required first lookup level for stage 2 translations
For a stage 2 translation, the output address range from the stage 1
translations determines the required input address range for the stage 2
translation. The permitted values of VTCR.SL0 are:
0b00 Stage 2 translation lookup must start at the second level.
0b01 Stage 2 translation lookup must start at the first level.
VTCR.T0SZ must indicate the required input address range. The size of
the input address region is 2^(32-T0SZ) bytes."
Thus VTCR.SL0 = 1 (maximum value) and VTCR.T0SZ = 0 when the size of
input address region is 2^32 bytes.
Signed-off-by: Ayan Kumar Halder
---
Changes from -
v1 - New patch.
v2 - 1. Added Ack.
v3 - 1. Dropped Ack.
2. Rebased the patch based on the previous change.
v4 - 1. t0sz is 0 for 32-bit IPA on Arm32.
2. Updated the commit message to explain t0sz, sl0 and root_order.
v5 - 1. Rebased on top of the changes in the previous patch.
v6 - 1. Removed the index for ARM_32.
v7 - 1. No changes.
xen/arch/arm/p2m.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 76388ba54b..a969068a68 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -2265,6 +2265,7 @@ void __init setup_virt_paging(void)
[6] = { 52, 12/*12*/, 4, 2 },
[7] = { 0 } /* Invalid */
#else
+{ 32, 0/*0*/,0, 1 },
{ 40, 24/*24*/, 1, 1 }
#endif
};
--
2.17.1
[XEN v8 4/5] xen/arm: Restrict zeroeth_table_offset for ARM_64
When 32 bit physical addresses are used (ie PHYS_ADDR_T_32=y),
"va >> ZEROETH_SHIFT" causes an overflow.
Also, there is no zeroeth level page table on Arm32.
Also took the opportunity to clean up dump_pt_walk(). One could use
DECLARE_OFFSETS() macro instead of declaring an array of page table
offsets.
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from -
v1 - Removed the duplicate declaration for DECLARE_OFFSETS.
v2 - 1. Reworded the commit message.
2. Use CONFIG_ARM_PA_32 to restrict zeroeth_table_offset.
v3 - 1. Added R-b and Ack.
v4 - 1. Removed R-b and Ack as we use CONFIG_PHYS_ADDR_T_32
instead of CONFIG_ARM_PA_BITS_32. This is to be in parity with our earlier
patches where we use CONFIG_PHYS_ADDR_T_32 to denote 32-bit physical addr
support.
v5 - 1. Added R-b and Ack.
v6 - 1. No changes.
v7 - 1. No changes.
xen/arch/arm/include/asm/lpae.h | 4
xen/arch/arm/mm.c | 7 +--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
index 3fdd5d0de2..7d2f6fd1bd 100644
--- a/xen/arch/arm/include/asm/lpae.h
+++ b/xen/arch/arm/include/asm/lpae.h
@@ -259,7 +259,11 @@ lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr);
#define first_table_offset(va) TABLE_OFFSET(first_linear_offset(va))
#define second_table_offset(va) TABLE_OFFSET(second_linear_offset(va))
#define third_table_offset(va) TABLE_OFFSET(third_linear_offset(va))
+#ifdef CONFIG_PHYS_ADDR_T_32
+#define zeroeth_table_offset(va) 0
+#else
#define zeroeth_table_offset(va) TABLE_OFFSET(zeroeth_linear_offset(va))
+#endif
/*
* Macros to define page-tables:
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5ef5fd8c49..e460249736 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -233,12 +233,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
{
static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" };
const mfn_t root_mfn = maddr_to_mfn(ttbr);
-const unsigned int offsets[4] = {
-zeroeth_table_offset(addr),
-first_table_offset(addr),
-second_table_offset(addr),
-third_table_offset(addr)
-};
+DECLARE_OFFSETS(offsets, addr);
lpae_t pte, *mapping;
unsigned int level, root_table;
--
2.17.1
[XEN v8 3/5] xen/arm: guest_walk: LPAE specific bits should be enclosed within "ifndef CONFIG_PHYS_ADDR_T_32"
As the previous patch introduces CONFIG_PHYS_ADDR_T_32 to support 32 bit physical addresses, the code specific to "Large Physical Address Extension" (ie LPAE) should be enclosed within "ifndef CONFIG_PHYS_ADDR_T_32". Refer xen/arch/arm/include/asm/short-desc.h, "short_desc_l1_supersec_t" unsigned int extbase1:4;/* Extended base address, PA[35:32] */ unsigned int extbase2:4;/* Extended base address, PA[39:36] */ Thus, extbase1 and extbase2 are not valid when 32 bit physical addresses are supported. Signed-off-by: Ayan Kumar Halder Acked-by: Stefano Stabellini --- Changes from - v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr". v2 - 1. Reordered this patch so that it appears after CONFIG_ARM_PA_32 is introduced (in 6/9). v3 - 1. Updated the commit message. 2. Added Ack. v4 - 1. No changes. v5 - 1. No changes. v6 - 1. No changes. v7 - 1. No changes. xen/arch/arm/guest_walk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index 43d3215304..c80a0ce55b 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -154,8 +154,10 @@ static bool guest_walk_sd(const struct vcpu *v, mask = (1ULL << L1DESC_SUPERSECTION_SHIFT) - 1; *ipa = gva & mask; *ipa |= (paddr_t)(pte.supersec.base) << L1DESC_SUPERSECTION_SHIFT; +#ifndef CONFIG_PHYS_ADDR_T_32 *ipa |= (paddr_t)(pte.supersec.extbase1) << L1DESC_SUPERSECTION_EXT_BASE1_SHIFT; *ipa |= (paddr_t)(pte.supersec.extbase2) << L1DESC_SUPERSECTION_EXT_BASE2_SHIFT; +#endif /* CONFIG_PHYS_ADDR_T_32 */ } /* Set permissions so that the caller can check the flags by herself. */ -- 2.17.1
[XEN v8 2/5] xen/arm: Introduce choice to enable 64/32 bit physical addressing
Some Arm based hardware platforms which does not support LPAE (eg Cortex-R52), uses 32 bit physical addresses. Also, users may choose to use 32 bits to represent physical addresses for optimization. To support the above use cases, we have introduced arch independent config to choose if the physical address can be represented using 32 bits (PHYS_ADDR_T_32) or 64 bits (!PHYS_ADDR_T_32). For now only ARM_32 provides support to enable 32 bit physical addressing. When PHYS_ADDR_T_32 is defined, PADDR_BITS is set to 32. Note that we use "unsigned long" (not "uint32_t") to denote the datatype of physical address. This is done to avoid using a cast each time PAGE_* macros are used on paddr_t. For eg PAGE_SIZE is defined as unsigned long. Thus, each time PAGE_SIZE is used with paddr_t, the result will be "unsigned long". On 32-bit architecture, "unsigned long" is 32-bit wide. Thus, it can be used to denote physical address. When PHYS_ADDR_T_32 is not defined for ARM_32, PADDR_BITS is set to 40. For ARM_64, PADDR_BITS is set to 48. The last two are same as the current configuration used today on Xen. Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel --- Changes from - v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr". v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 only whereas ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64. 2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'. v3 - 1. Allow user to define PADDR_BITS by selecting different config options ARM_PA_BITS_32, ARM_PA_BITS_40 and ARM_PA_BITS_48. 2. Add the choice under "Architecture Features". v4 - 1. Removed PHYS_ADDR_T_64 as !PHYS_ADDR_T_32 means PHYS_ADDR_T_32. v5 - 1. Removed ARM_PA_BITS_48 as there is no choice for ARM_64. 2. In ARM_PA_BITS_32, "help" is moved to last, and "depends on" before "select". v6 - 1. Explained why we use "unsigned long" to represent physical address for ARM_32. v7 - 1. Updated the reasoning for using "unsigned long" for paddr_t. 2. Added R-b by Michal. xen/arch/Kconfig | 3 +++ xen/arch/arm/Kconfig | 33 xen/arch/arm/include/asm/page-bits.h | 6 + xen/arch/arm/include/asm/types.h | 14 xen/arch/arm/mm.c| 5 + 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig index 7028f7b74f..67ba38f32f 100644 --- a/xen/arch/Kconfig +++ b/xen/arch/Kconfig @@ -1,6 +1,9 @@ config 64BIT bool +config PHYS_ADDR_T_32 + bool + config NR_CPUS int "Maximum number of CPUs" range 1 4095 diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 239d3aed3c..1e87fe0247 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -26,6 +26,39 @@ config ARCH_DEFCONFIG menu "Architecture Features" +choice + prompt "Physical address space size" if ARM_32 + default ARM_PA_BITS_40 if ARM_32 + help + User can choose to represent the width of physical address. This can + sometimes help in optimizing the size of image when user chooses a + smaller size to represent physical address. + +config ARM_PA_BITS_32 + bool "32-bit" + depends on ARM_32 + select PHYS_ADDR_T_32 + help + On platforms where any physical address can be represented within 32 bits, + user should choose this option. This will help in reduced size of the + binary. + Xen uses "unsigned long" and not "uint32_t" to denote the datatype of + physical address. This is done to avoid using a cast each time PAGE_* + macros are used on paddr_t. For eg PAGE_SIZE is defined as unsigned long. + On 32-bit architecture, "unsigned long" is 32-bit wide. Thus, it can be + used to denote physical address. + +config ARM_PA_BITS_40 + bool "40-bit" + depends on ARM_32 +endchoice + +config PADDR_BITS + int + default 32 if ARM_PA_BITS_32 + default 40 if ARM_PA_BITS_40 + default 48 if ARM_64 + source "arch/Kconfig" config ACPI diff --git a/xen/arch/arm/include/asm/page-bits.h b/xen/arch/arm/include/asm/page-bits.h index 5d6477e599..deb381ceeb 100644 --- a/xen/arch/arm/include/asm/page-bits.h +++ b/xen/arch/arm/include/asm/page-bits.h @@ -3,10 +3,6 @@ #define PAGE_SHIFT 12 -#ifdef CONFIG_ARM_64 -#define PADDR_BITS 48 -#else -#define PADDR_BITS 40 -#endif +#define PADDR_BITS CONFIG_PADDR_BITS #endif /* __ARM_PAGE_SHIFT_H__ */ diff --git a/xen/arch/arm/include/asm/types.h b/xen/arch/arm/include/asm/types.h index e218ed77bd..fb6618ef24 100644 --- a/xen/arch/arm/include/asm/types.h +++ b/xen/arch/arm/include/asm/types.h @@ -34,9 +34,23 @@ typedef signed long long s64; typedef unsigned long long u64; typedef u32 vaddr_t; #define PRIvaddr PRIx32 +#if defined(CONFIG_PHYS_ADDR_T_32) + +/* + * We u
[XEN v8 1/5] xen/arm: p2m: Use the pa_range_info table to support ARM_32 and ARM_64
Restructure the code so that one can use pa_range_info[] table for both
ARM_32 as well as ARM_64.
Also, removed the hardcoding for P2M_ROOT_ORDER and P2M_ROOT_LEVEL as
p2m_root_order can be obtained from the pa_range_info[].root_order and
p2m_root_level can be obtained from pa_range_info[].sl0.
Refer ARM DDI 0406C.d ID040418, B3-1345,
"Use of concatenated first-level translation tables
...However, a 40-bit input address range with a translation granularity of 4KB
requires a total of 28 bits of address resolution. Therefore, a stage 2
translation that supports a 40-bit input address range requires two concatenated
first-level translation tables,..."
Thus, root-order is 1 for 40-bit IPA on ARM_32.
Refer ARM DDI 0406C.d ID040418, B3-1348,
"Determining the required first lookup level for stage 2 translations
For a stage 2 translation, the output address range from the stage 1
translations determines the required input address range for the stage 2
translation. The permitted values of VTCR.SL0 are:
0b00 Stage 2 translation lookup must start at the second level.
0b01 Stage 2 translation lookup must start at the first level.
VTCR.T0SZ must indicate the required input address range. The size of the input
address region is 2^(32-T0SZ) bytes."
Thus VTCR.SL0 = 1 (maximum value) and VTCR.T0SZ = -8 when the size of input
address region is 2^40 bytes.
Thus, pa_range_info[].t0sz = 1 (VTCR.S) | 8 (VTCR.T0SZ) ie 11000b which is 24.
VTCR.T0SZ, is bits [5:0] for ARM_64.
VTCR.T0SZ is bits [3:0] and S(sign extension), bit[4] for ARM_32.
For this, we have used struct bitfields to convert pa_range_info[].t0sz to its
ARM_32 variant.
pa_range_info[] is indexed by ID_AA64MMFR0_EL1.PARange which is present in Arm64
only. This is the reason we do not specify the indices for ARM_32. Also, we
duplicated the entry "{ 40, 24/*24*/, 1, 1 }" between ARM_64 and
ARM_32. This is done to avoid introducing extra #if-defs.
Signed-off-by: Ayan Kumar Halder
---
Changes from -
v3 - 1. New patch introduced in v4.
2. Restructure the code such that pa_range_info[] is used both by ARM_32 as
well as ARM_64.
v4 - 1. Removed the hardcoded definitions of P2M_ROOT_ORDER and P2M_ROOT_LEVEL.
The reason being root_order will not be always 1 (See the next patch).
2. Updated the commit message to explain t0sz, sl0 and root_order values for
32-bit IPA on Arm32.
3. Some sanity fixes.
v5 - pa_range_info is indexed by system_cpuinfo.mm64.pa_range. ie
when PARange is 0, the PA size is 32, 1 -> 36 and so on. So pa_range_info[] has
been updated accordingly.
For ARM_32 pa_range_info[0] = 0 and pa_range_info[1] = 0 as we do not support
32-bit, 36-bit physical address range yet.
v6 - 1. Added pa_range_info[] entries for ARM_32 without indices. Some entry
may be duplicated between ARM_64 and ARM_32.
2. Recalculate p2m_ipa_bits for ARM_32 from T0SZ (similar to ARM_64).
3. Introduced an union to reinterpret T0SZ bits between ARM_32 and ARM_64.
v7 - 1. Used struct bifield instead of union to reinterpret T0SZ bits between
ARM_32 and ARM_64.
2. Removed the invalid entry for ARM_32.
xen/arch/arm/include/asm/p2m.h | 6
xen/arch/arm/p2m.c | 50 +-
2 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index f67e9ddc72..940495d42b 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -14,16 +14,10 @@
/* Holds the bit size of IPAs in p2m tables. */
extern unsigned int p2m_ipa_bits;
-#ifdef CONFIG_ARM_64
extern unsigned int p2m_root_order;
extern unsigned int p2m_root_level;
#define P2M_ROOT_ORDERp2m_root_order
#define P2M_ROOT_LEVEL p2m_root_level
-#else
-/* First level P2M is always 2 consecutive pages */
-#define P2M_ROOT_ORDER1
-#define P2M_ROOT_LEVEL 1
-#endif
struct domain;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 418997843d..76388ba54b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -19,9 +19,9 @@
#define INVALID_VMID 0 /* VMID 0 is reserved */
-#ifdef CONFIG_ARM_64
unsigned int __read_mostly p2m_root_order;
unsigned int __read_mostly p2m_root_level;
+#ifdef CONFIG_ARM_64
static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
/* VMID is by default 8 bit width on AArch64 */
#define MAX_VMID max_vmid
@@ -2247,16 +2247,6 @@ void __init setup_virt_paging(void)
/* Setup Stage 2 address translation */
register_t val = VTCR_RES1|VTCR_SH0_IS|VTCR_ORGN0_WBWA|VTCR_IRGN0_WBWA;
-#ifdef CONFIG_ARM_32
-if ( p2m_ipa_bits < 40 )
-panic("P2M: Not able to support %u-bit IPA at the moment\n",
- p2m_ipa_bits);
-
-printk("P2M: 40-bit IPA\n");
-p2m_ipa_bits = 40;
-val |= VTCR_T0SZ(0x18); /* 40 bit IPA */
-val |= VTCR_SL0(0x1); /* P2M starts at first level */
-#else /* CONFIG_ARM_64 */
static const struct {
unsigned int pabits; /* Physical Address Size */
unsigned
[XEN v8 0/5] Add support for 32-bit physical address
Hi All, Please have a look at https://lists.xenproject.org/archives/html/xen-devel/2022-11/msg01465.html for the context. The benefits of using 32 bit physical addresses are as follows :- 1. It helps to use Xen on platforms (for eg R52) which supports 32-bit physical addresses and has no support for large physical address extension. On 32-bit MPU systems which supports flat-mapping (for eg R52), it helps to translate 32 bit VA into 32 bit PA. 2. It also helps in code optimization when the underlying platform does not use large physical address extension. The following points are to be noted :- 1. Device tree always use uint64_t for address and size. The caller needs to translate between uint64_t and unsigned long (when 32 bit physical addressing is used). 2. Currently, we have enabled this option for Arm_32 as the MMU for Arm_64 uses 48-bit physical addressing. Changes from : v1 - 1. Reordered the patches such that the first three patches fixes issues in the existing codebase. These can be applied independent of the remaining patches in this serie. 2. Dropped translate_dt_address_size() for the address/size translation between paddr_t and u64 (as parsed from the device tree). Also, dropped the check for truncation (while converting u64 to paddr_t). Instead now we have modified device_tree_get_reg() and typecasted the return for dt_read_number(), to obtain paddr_t. Also, introduced wrappers for fdt_get_mem_rsv() and dt_device_get_address() for the same purpose. These can be found in patch 4/11 and patch 6/11. 3. Split "Other adaptations required to support 32bit paddr" into the following individual patches for each adaptation : xen/arm: smmu: Use writeq_relaxed_non_atomic() for writing to SMMU_CBn_TTBR0 xen/arm: guest_walk: LPAE specific bits should be enclosed within "ifndef CONFIG_ARM_PA_32" 4. Introduced "xen/arm: p2m: Enable support for 32bit IPA". v2 - 1. Dropped patches 1/11, 2/11 and 3/11 from the v2 as it has already been committed (except 2/11 - "[XEN v5] xen/arm: Use the correct format specifier" which is waiting to be committed). 2. Introduced a new patch "xen/drivers: ns16550: Use paddr_t for io_base/io_size". v3 - 1. Combined the patches from https://lists.xenproject.org/archives/html/xen-devel/2023-02/msg00656.html in this series. v4 - 1. Dropped "xen/drivers: ns16550: Use paddr_t for io_base/io_size" from the patch series. 2. Introduced "xen/arm: domain_build: Check if the address fits the range of physical address". 3. "xen/arm: Use the correct format specifier" has been committed in v4. v5 - 1. Based on the comments on "[XEN v5 08/10] xen/arm: domain_build: Check if the address fits the range of physical address", the patch has been modified and split into the following :- a. xen: dt: Replace u64 with uint64_t as the callback function parameters for dt_for_each_range() b. xen/arm: pci: Use 'uint64_t' as the datatype for the function parameters. c. xen/arm: domain_build: Check if the address fits the range of physical address v6 - 1. Reordered the patches such that only the patches which are dependent on "CONFIG_PHYS_ADDR_T_32" appear after the Kconfig option is introduced. v7 - 1. Changes from "[XEN v7 01/11] xen/arm: domain_build: Track unallocated pages using the frame number " till "[XEN v7 06/11] xen: dt: Replace u64 with uint64_t as the callback function parameters for dt_for_each_range()" have been committed. So the remaining 5 patches are sent out. Ayan Kumar Halder (5): xen/arm: p2m: Use the pa_range_info table to support ARM_32 and ARM_64 xen/arm: Introduce choice to enable 64/32 bit physical addressing xen/arm: guest_walk: LPAE specific bits should be enclosed within "ifndef CONFIG_PHYS_ADDR_T_32" xen/arm: Restrict zeroeth_table_offset for ARM_64 xen/arm: p2m: Enable support for 32bit IPA for ARM_32 xen/arch/Kconfig | 3 ++ xen/arch/arm/Kconfig | 33 ++ xen/arch/arm/guest_walk.c| 2 ++ xen/arch/arm/include/asm/lpae.h | 4 +++ xen/arch/arm/include/asm/p2m.h | 6 xen/arch/arm/include/asm/page-bits.h | 6 +--- xen/arch/arm/include/asm/types.h | 14 xen/arch/arm/mm.c| 12 +++ xen/arch/arm/p2m.c | 51 +--- 9 files changed, 101 insertions(+), 30 deletions(-) -- 2.17.1
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 01/06/2023 6:43 pm, Alejandro Vallejo wrote: > This allows replacing many instances of runtime checks with folded > constants. The patch asserts support for the NX bit in PTEs at boot time > and if so short-circuits cpu_has_nx to 1. This has several knock-on effects > that improve codegen: > * _PAGE_NX matches _PAGE_NX_BIT, optimising the macro to a constant. > * Many PAGE_HYPERVISOR_X are also folded into constants > * A few if ( cpu_has_nx ) statements are optimised out > > We save 2.5KiB off the text section and remove the runtime dependency for > applying NX, which hardens our security posture. The config option defaults > to OFF for compatibility with previous behaviour. While this is all true, I'd say it's not emphasising the correct point. Right now, any attacker with a partial write primitive who can clear the NX feature bit in boot_cpu_info will cause Xen to unintentionally write insecure PTEs. (Or for that matter, a memory corruption bug in Xen.) NX exists on all 64bit CPUs other than early Pentium 4's, and people who don't care about those CPUs can meaningfully improve their security defence-in-depth by enabling this option. The fact we also save 2.5k of code is a nice bonus, but not relevant. People aren't going to turn this option on to save code - they're going to turn it on for the extra security. It's fine to mention, but the code gen improvements should be one sentence max, not the majority of the commit message. > Signed-off-by: Alejandro Vallejo > --- > xen/arch/x86/Kconfig | 10 ++ > xen/arch/x86/boot/head.S | 19 --- > xen/arch/x86/boot/trampoline.S| 3 ++- > xen/arch/x86/efi/efi-boot.h | 9 + > xen/arch/x86/include/asm/cpufeature.h | 3 ++- > 5 files changed, 39 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig > index 406445a358..0983915372 100644 > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -307,6 +307,16 @@ config MEM_SHARING > bool "Xen memory sharing support (UNSUPPORTED)" if UNSUPPORTED > depends on HVM > > +config REQUIRE_NX_BIT > + def_bool n > + prompt "Require NX bit support" > + ---help--- > + Makes Xen require NX bit support on page table entries. This > + allows the resulting code to have folded constants where > + otherwise branches are required, yielding a smaller binary as a > + result. Requiring NX trades compatibility with older CPUs for > + improvements in speed and code size. The intended audience here is different. x86 developers will know what this means from the name alone, and won't read the help. It's distro packagers and end users who need to be able to read this and decide whether to turn it on or not. Therefore, it needs to read something more like this: No-eXecute (also called XD "eXecute Disable" and DEP "Data Execution Prevention") is a security feature designed originally to combat buffer overflow attacks by marking regions of memory which the CPU must not interpret as instructions. The NX feature exists in almost 64bit capable CPUs, except XXX [TBC - we might be extremely lucky here. Questions pending with people]. Enabling this option will improve Xen's security by removing cases where Xen could be tricked into thinking that the feature was unavailable. However, if enabled, Xen will no longer boot on any CPU which is lacking NX support. > + > endmenu > > source "common/Kconfig" > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index 09bebf8635..8414266281 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -123,6 +123,7 @@ multiboot2_header: > .Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!" > .Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!" > .Lbag_alg_msg: .asciz "ERR: Xen must be loaded at a 2Mb boundary!" > +.Lno_nx_bit_msg: .asciz "ERR: Not an NX-bit capable CPU!" > > .section .init.data, "aw", @progbits > .align 4 > @@ -151,6 +152,11 @@ not_multiboot: > .Lnot_aligned: > add $sym_offs(.Lbag_alg_msg),%esi # Error message > jmp .Lget_vtb > +#if IS_ENABLED(CONFIG_REQUIRE_NX_BIT) This doesn't need to be IS_ENABLED(). #if will DTRT for a non-existent symbol by considering such to be 0. IS_ENABLED() is only required for cases where you need an explicit 0 or 1 at the end, which is typically only in real C code, and for initialising of constants. > +no_nx_bit: > +add $sym_offs(.Lno_nx_bit_msg),%esi # Error message The "# Error message" is useless as a comment. Don't propagate it from the other bad examples. (I already had some cleanup planned here from Roger's patch adding not_aligned.) ~Andrew
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
On Fri, Jun 02, 2023 at 09:48:48AM +0100, Ayan Kumar Halder wrote: > Hi Xen developers, > > We are trying to better document xen project development processes and > related tools. At present, we are targeting **x86 and Arm** only. > > These tools range from bug/change request tracking means, compilers, infra, > editors, code-review tools, etc which is connected in some way to the Xen > development and is being currently used by xen-devel community. What is the end goal of this? I'm kind of unsure why do you care about which editor I use to generate my code, that's up to the developer. > I appreciate if you can let me know anything I missed or mistaken and the > version currently being used (for some of the tools). > > > 1. Code management portal - xenbits (https://xenbits.xenproject.org), gitlab > (https://gitlab.com/xen-project/xen) > > 2. Project description - wiki.xenproject.org > > 3. Project management - gitlab > > 4. Code review - text based email clients (mutt, thunderbird), git-email, b4 > > 5. Text Editors such as vim, emacs > > 6. Code review history - xen-devel mail archives > > 7. Code revision management - git > > 8. Xen coding language - C89, C99, Kconfig assembly (gas), python, perl, shell, Makefile, bison, flex, ocaml, go... Likely more that I've missed. > > 9. Testing tools for Arm64 in gitlab CI > > compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) > > binutils - GNU Binutils for Debian) 2.38.9 > > emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky > disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) > > dom0/domU kernel - kernel-5.19.0 > > rootfs - alpine-3.12-arm64-rootfs > > firmware - U-Boot 2022.10 > > 10. Testing tools for Arm in gitlab CI > > compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, arm-linux-gnueabihf-gcc > (Debian 12.2.0-14) 12.2.0 (most commonly used versions) > > emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) > > dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from > Debian) > > rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz > > firmware - U-Boot 2022.10 > > 11. Testing tools for x86 > > compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, > clang (from Debian) (most commonly used version) > > binutils - GNU ld (GNU Binutils for Debian) 2.40) > > emulator/hardware - Qubes HW (**need details regarding machine, firmware, > etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) > > dom0/domU kernel - kernel 6.1.19 > > rootfs - alpine-3.12-rootfs > > firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by EDK > II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB > 2.06~rc1 I do use an LLVM based toolstack, so that's usually latest LLVM import on FreeBSD. We do also test this on the cirrus-ci, see: https://github.com/royger/xen/runs/5334480206 I_n any case I think the scope to some of the questions is unknown, it's not feasible to expect to list every possible combination of Linux versions vs Xen version vs whatever guests versions a given developer might be running. Regards, Roger.
[ovmf test] 181106: all pass - PUSHED
flight 181106 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/181106/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 78262899d225eb30e5fbe6a88e85a4b1d8c04a61 baseline version: ovmf 41abf00bf98e36830974bd669ab7ec3679bd5e67 Last test of basis 181091 2023-06-01 19:43:55 Z0 days Testing same since 181106 2023-06-02 07:40:45 Z0 days1 attempts People who touched revisions under test: Thejaswani Putta 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 41abf00bf9..78262899d2 78262899d225eb30e5fbe6a88e85a4b1d8c04a61 -> xen-tested-master
Re: [PATCH] x86: Add Kconfig option to require NX bit support
Hi, Sure to everything. A couple of notes: On Fri, Jun 02, 2023 at 10:31:08AM +0200, Jan Beulich wrote: > > + def_bool n > > + prompt "Require NX bit support" > > Just > > bool "Require NX bit support" > > please. I didn't realize Kconfig defaulted to 'n'. That's neat, thanks. > > > @@ -151,6 +152,11 @@ not_multiboot: > > .Lnot_aligned: > > add $sym_offs(.Lbag_alg_msg),%esi # Error message > > jmp .Lget_vtb > > +#if IS_ENABLED(CONFIG_REQUIRE_NX_BIT) > > +no_nx_bit: > > .Lno_nx_bit (no need for this to end up in the symbol table, just like > most other labels around here). There's a bunch of others in that general area with global symbols. I'll modify bad_cpu -> .Lbad_cpu as well while dealing with the next suggestion about reordering the NX and Long Mode checks. Alejandro
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
On Fri, Jun 02, 2023 at 09:48:48AM +0100, Ayan Kumar Halder wrote: > Hi Xen developers, > > We are trying to better document xen project development processes and > related tools. At present, we are targeting **x86 and Arm** only. > > These tools range from bug/change request tracking means, compilers, infra, > editors, code-review tools, etc which is connected in some way to the Xen > development and is being currently used by xen-devel community. > > I appreciate if you can let me know anything I missed or mistaken and the > version currently being used (for some of the tools). > > > 1. Code management portal - xenbits (https://xenbits.xenproject.org), gitlab > (https://gitlab.com/xen-project/xen) > > 2. Project description - wiki.xenproject.org > > 3. Project management - gitlab > > 4. Code review - text based email clients (mutt, thunderbird), git-email, b4 > > 5. Text Editors such as vim, emacs > > 6. Code review history - xen-devel mail archives > > 7. Code revision management - git > > 8. Xen coding language - C89, C99, Kconfig > > 9. Testing tools for Arm64 in gitlab CI > > compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) > > binutils - GNU Binutils for Debian) 2.38.9 > > emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky > disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) > > dom0/domU kernel - kernel-5.19.0 > > rootfs - alpine-3.12-arm64-rootfs > > firmware - U-Boot 2022.10 > > 10. Testing tools for Arm in gitlab CI > > compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, arm-linux-gnueabihf-gcc > (Debian 12.2.0-14) 12.2.0 (most commonly used versions) > > emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) > > dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from > Debian) > > rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz > > firmware - U-Boot 2022.10 > > 11. Testing tools for x86 > > compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, > clang (from Debian) (most commonly used version) > > binutils - GNU ld (GNU Binutils for Debian) 2.40) > > emulator/hardware - Qubes HW (**need details regarding machine, firmware, > etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) There are two x86 machines: 1. MSI PRO Z690-A with Intel Core i5-12600K, this one has Dasharo firmware (coreboot + UEFI) 2. MinisForum UM773 Lite with AMD Ryzen 7 7735HS, this one has stock UEFI firmware > dom0/domU kernel - kernel 6.1.19 > > rootfs - alpine-3.12-rootfs > > firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by EDK > II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB > 2.06~rc1 > > 12. Debugger - gdb > > 13. Xen code building infra - make > > 14. Testing OS - CentOS 7, Ubuntu, OpenSuse, Arch Linux, Alpine 3.12.12, > Debian 10 (Buster), Fedora > > ( **I could not get the version info for some of these ^^^**) > > 15. Testing Infra - Gitlab runner, Docker > > 16. Testing tools common to all architectures - All the host OS packages > > dtc, build-essential, zlib1g-dev, libncurses5-dev, libssl-dev, python-dev, > python3-dev, xorg-dev, uuid-dev, libyajl-dev, libaio-dev, libglib2.0-dev, > libpixman-1-dev, pkg-config, flex, bison, gettext, acpica-tools, bin86, bcc, > liblzma-dev, libc6-dev-i386, libnl-3-dev, ocaml-nox, libfindlib-ocaml-dev, > markdown, transfig, pandoc, checkpolicy, wget, nasm, mkimage, uboot-tools > > 17. Documentation related tools - doxygen, rst. > > > Did I miss anything ? > > Kind regards, > > Ayan > > > > > -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
Re: [PATCH v2 3/3] x86/cpu-policy: Derive RSBA/RRSBA for guest policies
On 01.06.2023 16:48, Andrew Cooper wrote:
> The RSBA bit, "RSB Alternative", means that the RSB may use alternative
> predictors when empty. From a practical point of view, this mean "Retpoline
> not safe".
>
> Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
> statement that IBRS is implemented in hardware (as opposed to the form
> retrofitted to existing CPUs in microcode).
>
> The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
> property that predictions are tagged with the mode in which they were learnt.
> Therefore, it means "when eIBRS is active, the RSB may fall back to
> alternative predictors but restricted to the current prediction mode". As
> such, it's stronger statement than RSBA, but still means "Retpoline not safe".
>
> CPUs are not expected to enumerate both RSBA and RRSBA.
>
> Add feature dependencies for EIBRS and RRSBA. While technically they're not
> linked, absolutely nothing good can of letting the guest see RRSBA without
Nit: missing "come"?
> EIBRS. Nor can anything good come of a guest seeing EIBRS without IBRSB.
> Furthermore, we use this dependency to simplify the max derivation logic.
>[...]
> @@ -532,6 +541,21 @@ static void __init calculate_pv_def_policy(void)
> guest_common_default_feature_adjustments(fs);
>
> sanitise_featureset(fs);
> +
> +/*
> + * If the host suffers from RSBA of any form, and the guest can see
> + * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the
> guest
> + * depending on the visibility of eIBRS.
> + */
> +if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
> + (cpu_has_rsba || cpu_has_rrsba) )
> +{
> +bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
> +
> +__set_bit(eibrs ? X86_FEATURE_RRSBA
> +: X86_FEATURE_RSBA, fs);
> +}
> +
> x86_cpu_featureset_to_policy(fs, p);
> recalculate_xstate(p);
> }
> @@ -664,6 +688,21 @@ static void __init calculate_hvm_def_policy(void)
> __set_bit(X86_FEATURE_VIRT_SSBD, fs);
>
> sanitise_featureset(fs);
> +
> +/*
> + * If the host suffers from RSBA of any form, and the guest can see
> + * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the
> guest
> + * depending on the visibility of eIBRS.
> + */
> +if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
> + (cpu_has_rsba || cpu_has_rrsba) )
> +{
> +bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
> +
> +__set_bit(eibrs ? X86_FEATURE_RRSBA
> +: X86_FEATURE_RSBA, fs);
> +}
> +
> x86_cpu_featureset_to_policy(fs, p);
> recalculate_xstate(p);
> }
> @@ -786,6 +825,20 @@ void recalculate_cpuid_policy(struct domain *d)
>
> sanitise_featureset(fs);
>
> +/*
> + * If the host suffers from RSBA of any form, and the guest can see
> + * MSR_ARCH_CAPS, reflect the appropriate RSBA/RRSBA property to the
> guest
> + * depending on the visibility of eIBRS.
> + */
> +if ( test_bit(X86_FEATURE_ARCH_CAPS, fs) &&
> + (cpu_has_rsba || cpu_has_rrsba) )
> +{
> +bool eibrs = test_bit(X86_FEATURE_EIBRS, fs);
> +
> +__set_bit(eibrs ? X86_FEATURE_RRSBA
> +: X86_FEATURE_RSBA, fs);
> +}
Now that we have the same code and comment even a 3rd time, surely this
wants to be put in a helper?
What about a tool stack request leading to us setting the 2nd of the two
bits here, while the other was already set? IOW wouldn't we better clear
the other bit explicitly? (Due to the EIBRS dependency or RRSBA I think
this can really only happen when the tool stack requests RSBA+EIBRS, as
the deep deps clearing doesn't know the concept of "negative"
dependencies.)
Similarly what about a tool stack (and ultimately admin) request setting
both RSBA and RRSBA? Wouldn't we better clear the respectively wrong bit
then? People may do this in their guest configs "just to be on the safe
side" (once expressing this in guest configs is possible, of course),
and due to the max policies having both bits set this also won't occur
"automatically".
Jan
Re: [PATCH v2 2/3] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate
On 01.06.2023 16:48, Andrew Cooper wrote:
> @@ -593,15 +596,85 @@ static bool __init retpoline_calculations(void)
> return false;
>
> /*
> - * RSBA may be set by a hypervisor to indicate that we may move to a
> - * processor which isn't retpoline-safe.
> + * The meaning of the RSBA and RRSBA bits have evolved over time. The
> + * agreed upon meaning at the time of writing (May 2023) is thus:
> + *
> + * - RSBA (RSB Alternative) means that an RSB may fall back to an
> + * alternative predictor on underflow. Skylake uarch and later all
> have
> + * this property. Broadwell too, when running microcode versions prior
> + * to Jan 2018.
> + *
> + * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
> + * tagging of predictions with the mode in which they were learned. So
> + * when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
> + *
> + * - CPUs are not expected to enumerate both RSBA and RRSBA.
> + *
> + * Some parts (Broadwell) are not expected to ever enumerate this
> + * behaviour directly. Other parts have differing enumeration with
> + * microcode version. Fix up Xen's idea, so we can advertise them safely
> + * to guests, and so toolstacks can level a VM safety for migration.
> + *
> + * The following states exist:
> + *
> + * | | RSBA | EIBRS | RRSBA | Notes | Action|
> + * |---+--+---+---++---|
> + * | 1 |0 | 0 | 0 | OK (older parts) | Maybe +RSBA |
> + * | 2 |0 | 0 | 1 | Broken | +RSBA, -RRSBA |
> + * | 3 |0 | 1 | 0 | OK (pre-Aug ucode) | +RRSBA|
> + * | 4 |0 | 1 | 1 | OK | |
> + * | 5 |1 | 0 | 0 | OK | |
> + * | 6 |1 | 0 | 1 | Broken | -RRSBA|
> + * | 7 |1 | 1 | 0 | Broken | -RSBA, +RRSBA |
> + * | 8 |1 | 1 | 1 | Broken | -RSBA |
> *
> + * However, we doesn't need perfect adherence to the spec. Identify the
Nit: "don't" or "it"?
> + * broken cases (so we stand a chance of spotting violated assumptions),
> + * and fix up Rows 1 and 3 so Xen can use RSBA || RRSBA to identify
> + * "alternative predictors potentially in use".
Considering that it's rows 2, 6, 7, and 8 which are broken, I find this
comment a little misleading. To me it doesn't become clear whether them
subsequently being left alone (and merely a log message issued) is
intentional.
> + */
> +if ( cpu_has_eibrs ? cpu_has_rsba /* Rows 7, 8 */
> + : cpu_has_rrsba /* Rows 2, 6 */ )
> +printk(XENLOG_ERR
> + "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u,
> EIBRS %u, RRSBA %u\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model,
> + boot_cpu_data.x86_mask, ucode_rev,
> + cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);
> +
> +/*
> * Processors offering Enhanced IBRS are not guarenteed to be
> * repoline-safe.
> */
> -if ( cpu_has_rsba || cpu_has_eibrs )
> +if ( cpu_has_eibrs )
> +{
> +/*
> + * Prior to the August 2023 microcode, many eIBRS-capable parts did
> + * not enumerate RRSBA.
> + */
> +if ( !cpu_has_rrsba )
> +setup_force_cpu_cap(X86_FEATURE_RRSBA);
> +
> +return false;
> +}
No clearing of RSBA in this case? I fear we may end up with misbehavior if
our own records aren't kept consistent with our assumptions. (This then
extends to the "just a log message" above as well.)
Also the inner conditional continues to strike me as odd; could you add
half a sentence to the comment (or description) as to meaning to leave
is_forced_cpu_cap() function correctly (which in turn raises the question
whether - down the road - this is actually going to matter)?
> +/*
> + * RSBA is explicitly enumerated in some cases, but may also be set by a
> + * hypervisor to indicate that we may move to a processor which isn't
> + * retpoline-safe.
> + */
> +if ( cpu_has_rsba )
> return false;
>
> +/*
> + * At this point, we've filtered all the legal RSBA || RRSBA cases (or
> the
> + * known non-ideal cases). If ARCH_CAPS is visible, trust the absence of
> + * RSBA || RRSBA. There's no known microcode which advertises ARCH_CAPS
> + * without RSBA or EIBRS, and if we're virtualised we can't rely the
> model
> + * check anyway.
> + */
I think "no known" wants further qualification: When IBRSB was first
introduced, EIBRS and RSBA weren't even known about. I also didn't
think all hardware (i.e. sufficiently old one) did get newer ucode
when these started to be known. Possibly you mean "... which wrong
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 02/06/2023 9:31 am, Jan Beulich wrote: > On 01.06.2023 19:43, Alejandro Vallejo wrote: >> This allows replacing many instances of runtime checks with folded >> constants. The patch asserts support for the NX bit in PTEs at boot time >> and if so short-circuits cpu_has_nx to 1. This has several knock-on effects >> that improve codegen: >> * _PAGE_NX matches _PAGE_NX_BIT, optimising the macro to a constant. >> * Many PAGE_HYPERVISOR_X are also folded into constants >> * A few if ( cpu_has_nx ) statements are optimised out >> >> We save 2.5KiB off the text section and remove the runtime dependency for >> applying NX, which hardens our security posture. The config option defaults >> to OFF for compatibility with previous behaviour. >> >> Signed-off-by: Alejandro Vallejo > At a guess this may want a Suggested-by: Andrew? Well - it was a work item off the backlog, and a one-liner at that. I wouldn't have said an explicit tag was warranted simply because I put the backlog together. ~Andrew
[xen-unstable test] 181095: tolerable FAIL - PUSHED
flight 181095 xen-unstable real [real] flight 181107 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/181095/ http://logs.test-lab.xenproject.org/osstest/logs/181107/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-livepatch 7 xen-install fail pass in 181107-retest test-amd64-i386-freebsd10-i386 7 xen-install fail pass in 181107-retest test-amd64-amd64-dom0pvh-xl-amd 22 guest-start/debian.repeat fail pass in 181107-retest test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail pass in 181107-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181079 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181079 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181079 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181079 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181079 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181079 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181079 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181079 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181079 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181079 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181079 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181079 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail 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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 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-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support
Re: [PATCH v2 1/3] x86/spec-ctrl: Rename retpoline_safe() to retpoline_calculations()
On 01.06.2023 16:48, Andrew Cooper wrote: > This is prep work, split out to simply the diff on the following change. > > * Rename to retpoline_calculations(), and call unconditionally. It is >shortly going to synthesise missing enumerations required for guest safety. > * For the model check switch statement, store the result in a variable and >break rather than returning directly. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich
Re: Listing the tools required for Xen development/testing on x86 and Arm by the community
On Fri, Jun 2, 2023 at 1:49 AM Ayan Kumar Halder wrote: > Hi Xen developers, > > We are trying to better document xen project development processes and > related tools. At present, we are targeting **x86 and Arm** only. > > These tools range from bug/change request tracking means, compilers, > infra, editors, code-review tools, etc which is connected in some way to > the Xen development and is being currently used by xen-devel community. > > I appreciate if you can let me know anything I missed or mistaken and > the version currently being used (for some of the tools). > The Xen git tree contains a README file with some of the information that you are looking for. This link is to that file for the 4.17 release: https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=README;h=a947e8f3e35b78e2d5c935caa4c7ea71897f0f3c;hb=11560248ffda3f00f20bbdf3ae088af474f7f2a3 The README specifies specific minimum compiler versions for that Xen release. > > > 1. Code management portal - xenbits (https://xenbits.xenproject.org), > gitlab (https://gitlab.com/xen-project/xen) > Also: patchew: https://patchew.org/Xen/ > > 2. Project description - wiki.xenproject.org > > 3. Project management - gitlab > > 4. Code review - text based email clients (mutt, thunderbird), git-email, > b4 > > 5. Text Editors such as vim, emacs > > 6. Code review history - xen-devel mail archives > > 7. Code revision management - git > > 8. Xen coding language - C89, C99, Kconfig > The Xen community also has tools written in Python, OCaml, Go, and Rust. > > 9. Testing tools for Arm64 in gitlab CI > > compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) > > binutils - GNU Binutils for Debian) 2.38.9 > > emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, > poky disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) > > dom0/domU kernel - kernel-5.19.0 > > rootfs - alpine-3.12-arm64-rootfs > > firmware - U-Boot 2022.10 > > 10. Testing tools for Arm in gitlab CI > > compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, > arm-linux-gnueabihf-gcc (Debian 12.2.0-14) 12.2.0 (most commonly used > versions) > > emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) > > dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from > Debian) > > rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz > > firmware - U-Boot 2022.10 > > 11. Testing tools for x86 > > compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) > 12.2.0, clang (from Debian) (most commonly used version) > > binutils - GNU ld (GNU Binutils for Debian) 2.40) > > emulator/hardware - Qubes HW (**need details regarding machine, > firmware, etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) > > dom0/domU kernel - kernel 6.1.19 > > rootfs - alpine-3.12-rootfs > > firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by > EDK II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), > GRUB 2.06~rc1 > > 12. Debugger - gdb > There is a tools/debugger directory in the Xen git tree that contains: gdbsx, a gdb server for Xen, and kdd, a tool for debugging Xen with the windows kernel debugger. > 13. Xen code building infra - make > > 14. Testing OS - CentOS 7, Ubuntu, OpenSuse, Arch Linux, Alpine 3.12.12, > Debian 10 (Buster), Fedora > > ( **I could not get the version info for some of these ^^^**) > Yocto and OpenEmbedded releases contain support for building Xen guest OS images suitable for testing. Devuan, FreeBSD and OpenBSD. > > 15. Testing Infra - Gitlab runner, Docker > > 16. Testing tools common to all architectures - All the host OS packages > > dtc, build-essential, zlib1g-dev, libncurses5-dev, libssl-dev, > python-dev, python3-dev, xorg-dev, uuid-dev, libyajl-dev, libaio-dev, > libglib2.0-dev, libpixman-1-dev, pkg-config, flex, bison, gettext, > acpica-tools, bin86, bcc, liblzma-dev, libc6-dev-i386, libnl-3-dev, > ocaml-nox, libfindlib-ocaml-dev, markdown, transfig, pandoc, > checkpolicy, wget, nasm, mkimage, uboot-tools > > 17. Documentation related tools - doxygen, rst. > Possibly: Sphinx > > > Did I miss anything ? > I think you have a good start! best, Christopher > > Kind regards, > > Ayan > > > > >
Re: [XEN][PATCH v7 15/19] xen/arm: Implement device tree node removal functionalities
On 02.06.2023 02:48, Vikram Garhwal wrote:
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1057,6 +1057,25 @@ typedef struct xen_sysctl_cpu_policy
> xen_sysctl_cpu_policy_t;
> DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
> #endif
>
> +#if defined(__arm__) || defined (__aarch64__)
> +/*
> + * XEN_SYSCTL_dt_overlay
> + * Performs addition/removal of device tree nodes under parent node using
> dtbo.
> + * This does in three steps:
> + * - Adds/Removes the nodes from dt_host.
> + * - Adds/Removes IRQ permission for the nodes.
> + * - Adds/Removes MMIO accesses.
> + */
> +struct xen_sysctl_dt_overlay {
> +XEN_GUEST_HANDLE_64(void) overlay_fdt; /* IN: overlay fdt. */
If the comment is correct, then const_void please.
Jan
Re: [XEN][PATCH v7 09/19] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller
On 02.06.2023 11:19, Jan Beulich wrote:
> On 02.06.2023 02:48, Vikram Garhwal wrote:
>> --- a/xen/drivers/passthrough/device_tree.c
>> +++ b/xen/drivers/passthrough/device_tree.c
>> @@ -18,6 +18,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -83,16 +84,14 @@ fail:
>> return rc;
>> }
>>
>> -static bool_t iommu_dt_device_is_assigned(const struct dt_device_node *dev)
>> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev)
>> {
>> bool_t assigned = 0;
>>
>> if ( !dt_device_is_protected(dev) )
>> return 0;
>>
>> -spin_lock(&dtdevs_lock);
>> assigned = !list_empty(&dev->domain_list);
>> -spin_unlock(&dtdevs_lock);
>>
>> return assigned;
>> }
>> @@ -213,27 +212,40 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl,
>> struct domain *d,
>> if ( (d && d->is_dying) || domctl->u.assign_device.flags )
>> break;
>>
>> +spin_lock(&dtdevs_lock);
>> +
>> ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
>> domctl->u.assign_device.u.dt.size,
>> &dev);
>> if ( ret )
>> +{
>> +spin_unlock(&dtdevs_lock);
>> break;
>> +}
>>
>> ret = xsm_assign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
>> if ( ret )
>> +{
>> +spin_unlock(&dtdevs_lock);
>> break;
>> +}
>>
>> if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
>> {
>> -if ( iommu_dt_device_is_assigned(dev) )
>> +
>> +if ( iommu_dt_device_is_assigned_locked(dev) )
>> {
>> printk(XENLOG_G_ERR "%s already assigned.\n",
>> dt_node_full_name(dev));
>> ret = -EINVAL;
>> }
>> +
>> +spin_unlock(&dtdevs_lock);
>> break;
>> }
>>
>> +spin_unlock(&dtdevs_lock);
>> +
>> if ( d == dom_io )
>> return -EINVAL;
>>
>
> I'm having trouble seeing how this patch can be correct: How do present
> callers of iommu_dt_device_is_assigned() continue to build? I can only
> guess that a new wrapper of this name is introduced in an earlier or
> later patch, but thus breaking bisectability (either here or, if the
> wrapper is added in an earlier patch, there).
Oh, I'm sorry - looks like I overlooked that the 2nd hunk alters the
(sole) caller. Somehow I was under the (wrong) impression that both
hunks fiddle with the same function.
Jan
RE: [XEN][PATCH v7 05/19] xen/arm: Add CONFIG_OVERLAY_DTB
Hi Jan and Vikram, > -Original Message- > Subject: Re: [XEN][PATCH v7 05/19] xen/arm: Add CONFIG_OVERLAY_DTB > > > diff --git a/CHANGELOG.md b/CHANGELOG.md > > index 5bfd3aa5c0..a137fce576 100644 > > --- a/CHANGELOG.md > > +++ b/CHANGELOG.md > > @@ -20,6 +20,8 @@ The format is based on [Keep a > Changelog](https://keepachangelog.com/en/1.0.0/) > > - Bus-lock detection, used by Xen to mitigate (by rate-limiting) the > > system > > wide impact of a guest misusing atomic instructions. > > - xl/libxl can customize SMBIOS strings for HVM guests. > > + - On Arm, support for dynamic addition/removal of Xen device tree nodes > using > > + a device tree overlay binary(.dtbo). > > May I suggest "..., experimental support ..." here? Great point! I agree using "experimental support" is better here. @Vikram: Just to be clear, if you agree and change the wording following Jan's suggestion, you can keep my acked-by tag for the changelog :)) Kind regards, Henry > > Jan
Re: [XEN][PATCH v7 11/19] xen/iommu: Introduce iommu_remove_dt_device()
On 02.06.2023 02:48, Vikram Garhwal wrote:
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -126,6 +126,47 @@ int iommu_release_dt_devices(struct domain *d)
> return 0;
> }
>
> +int iommu_remove_dt_device(struct dt_device_node *np)
> +{
> +const struct iommu_ops *ops = iommu_get_ops();
> +struct device *dev = dt_to_dev(np);
> +int rc;
> +
> +if ( !ops )
> +return -EOPNOTSUPP;
> +
> +spin_lock(&dtdevs_lock);
> +
> +if ( iommu_dt_device_is_assigned_locked(np) )
> +{
> +rc = -EBUSY;
> +goto fail;
> +}
> +
> +/*
> + * The driver which supports generic IOMMU DT bindings must have this
> + * callback implemented.
> + */
> +if ( !ops->remove_device )
> +{
> +rc = -EOPNOTSUPP;
> +goto fail;
> +}
> +
> +/*
> + * Remove master device from the IOMMU if latter is present and
> available.
> + * The driver is responsible for removing is_protected flag.
> + */
> +rc = ops->remove_device(0, dev);
> +
> +if ( !rc )
> +iommu_fwspec_free(dev);
> +
> +fail:
> +spin_unlock(&dtdevs_lock);
> +return rc;
Same remark regarding label indentation - please address throughout the
series.
Jan
Re: [XEN][PATCH v7 10/19] xen/iommu: protect iommu_add_dt_device() with dtdevs_lock
On 02.06.2023 02:48, Vikram Garhwal wrote: > @@ -189,6 +194,8 @@ int iommu_add_dt_device(struct dt_device_node *np) > if ( rc < 0 ) > iommu_fwspec_free(dev); > > +fail: > +spin_unlock(&dtdevs_lock); Nit: Labels indented by at least on blank please (see ./CODING_STYLE). Jan
Re: [XEN][PATCH v7 09/19] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller
On 02.06.2023 02:48, Vikram Garhwal wrote:
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -83,16 +84,14 @@ fail:
> return rc;
> }
>
> -static bool_t iommu_dt_device_is_assigned(const struct dt_device_node *dev)
> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev)
> {
> bool_t assigned = 0;
>
> if ( !dt_device_is_protected(dev) )
> return 0;
>
> -spin_lock(&dtdevs_lock);
> assigned = !list_empty(&dev->domain_list);
> -spin_unlock(&dtdevs_lock);
>
> return assigned;
> }
> @@ -213,27 +212,40 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl,
> struct domain *d,
> if ( (d && d->is_dying) || domctl->u.assign_device.flags )
> break;
>
> +spin_lock(&dtdevs_lock);
> +
> ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
> domctl->u.assign_device.u.dt.size,
> &dev);
> if ( ret )
> +{
> +spin_unlock(&dtdevs_lock);
> break;
> +}
>
> ret = xsm_assign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
> if ( ret )
> +{
> +spin_unlock(&dtdevs_lock);
> break;
> +}
>
> if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
> {
> -if ( iommu_dt_device_is_assigned(dev) )
> +
> +if ( iommu_dt_device_is_assigned_locked(dev) )
> {
> printk(XENLOG_G_ERR "%s already assigned.\n",
> dt_node_full_name(dev));
> ret = -EINVAL;
> }
> +
> +spin_unlock(&dtdevs_lock);
> break;
> }
>
> +spin_unlock(&dtdevs_lock);
> +
> if ( d == dom_io )
> return -EINVAL;
>
I'm having trouble seeing how this patch can be correct: How do present
callers of iommu_dt_device_is_assigned() continue to build? I can only
guess that a new wrapper of this name is introduced in an earlier or
later patch, but thus breaking bisectability (either here or, if the
wrapper is added in an earlier patch, there).
> --- /dev/null
> +++ b/xen/include/xen/iommu-private.h
I don't think private headers belong in this directory. From this patch
alone (including its description) it also doesn't become clear how (and
hence from where) this is intended to be used, which makes it hard to
suggest an alternative.
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> + /*
Nit: Stray blank.
> + * xen/iommu-private.h
> + *
> + *
Nit: No double empty comment lines please.
> + * Copyright (C) 2023, Advanced Micro Devices, Inc. All Rights Reserved.
> + * Written by Vikram Garhwal
> + *
> + */
> +#ifndef __XEN_IOMMU_PRIVATE_H__
> +#define __XEN_IOMMU_PRIVATE_H__
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +#include
Why (for both of the lines)? All you need is a forward declaration of
struct dt_device_node.
> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev);
Just bool please.
Jan
Re: [XEN][PATCH v7 06/19] libfdt: Keep fdt functions after init for CONFIG_OVERLAY_DTB.
On 02.06.2023 02:48, Vikram Garhwal wrote: > --- a/xen/common/libfdt/Makefile > +++ b/xen/common/libfdt/Makefile > @@ -1,7 +1,11 @@ > include $(src)/Makefile.libfdt > > SECTIONS := text data $(SPECIAL_DATA_SECTIONS) > + > +# For CONFIG_OVERLAY_DTB, libfdt functionalities will be needed during > runtime. > +ifneq ($(CONFIG_OVERLAY_DTB),y) > OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) > +endif I have to admit that I find this odd: While it may be the least intrusive change, why do the objcopy step at all in this case? Jan
Re: [XEN][PATCH v7 05/19] xen/arm: Add CONFIG_OVERLAY_DTB
On 02.06.2023 02:48, Vikram Garhwal wrote: > Introduce a config option where the user can enable support for > adding/removing > device tree nodes using a device tree binary overlay. > > Update SUPPORT.md and CHANGELOG.md to state the Device Tree Overlays support > for > Arm. > > Signed-off-by: Vikram Garhwal > > --- > Changes from v6: > Add CHANGELOG and SUPPORT.md entries. > --- > CHANGELOG.md | 2 ++ > SUPPORT.md | 6 ++ > xen/arch/arm/Kconfig | 5 + > 3 files changed, 13 insertions(+) > > diff --git a/CHANGELOG.md b/CHANGELOG.md > index 5bfd3aa5c0..a137fce576 100644 > --- a/CHANGELOG.md > +++ b/CHANGELOG.md > @@ -20,6 +20,8 @@ The format is based on [Keep a > Changelog](https://keepachangelog.com/en/1.0.0/) > - Bus-lock detection, used by Xen to mitigate (by rate-limiting) the > system > wide impact of a guest misusing atomic instructions. > - xl/libxl can customize SMBIOS strings for HVM guests. > + - On Arm, support for dynamic addition/removal of Xen device tree nodes > using > + a device tree overlay binary(.dtbo). May I suggest "..., experimental support ..." here? Jan
Listing the tools required for Xen development/testing on x86 and Arm by the community
Hi Xen developers, We are trying to better document xen project development processes and related tools. At present, we are targeting **x86 and Arm** only. These tools range from bug/change request tracking means, compilers, infra, editors, code-review tools, etc which is connected in some way to the Xen development and is being currently used by xen-devel community. I appreciate if you can let me know anything I missed or mistaken and the version currently being used (for some of the tools). 1. Code management portal - xenbits (https://xenbits.xenproject.org), gitlab (https://gitlab.com/xen-project/xen) 2. Project description - wiki.xenproject.org 3. Project management - gitlab 4. Code review - text based email clients (mutt, thunderbird), git-email, b4 5. Text Editors such as vim, emacs 6. Code review history - xen-devel mail archives 7. Code revision management - git 8. Xen coding language - C89, C99, Kconfig 9. Testing tools for Arm64 in gitlab CI compiler - gcc-9.3.0 (Alpine 3.12)) (most commonly used version) binutils - GNU Binutils for Debian) 2.38.9 emulator/hw - qemu-system-aarch64-6.0.0, qemuarm64 6.2.0 (From yocto, poky disto - 4.0.5), zcu102 (**need the uboot, TF-A versions **) dom0/domU kernel - kernel-5.19.0 rootfs - alpine-3.12-arm64-rootfs firmware - U-Boot 2022.10 10. Testing tools for Arm in gitlab CI compiler - arm-poky-linux-gnueabi-gcc (GCC) 11.3.0, arm-linux-gnueabihf-gcc (Debian 12.2.0-14) 12.2.0 (most commonly used versions) emulator/hw - qemu-system-arm 6.2.0 (From yocto, poky disto - 4.0.5) dom0/domU kernel - kernel-5.15.72 (from Yocto), Kernel-5.10.0-22 (from Debian) rootfs - alpine-minirootfs-3.15.1-armhf.tar.gz firmware - U-Boot 2022.10 11. Testing tools for x86 compiler - gcc-9.3.0 (Alpine Linux 9.3.0), gcc (Debian 12.2.0-14) 12.2.0, clang (from Debian) (most commonly used version) binutils - GNU ld (GNU Binutils for Debian) 2.40) emulator/hardware - Qubes HW (**need details regarding machine, firmware, etc**) , qemu 6.2.0 (From yocto, poky distro - 4.0.5) dom0/domU kernel - kernel 6.1.19 rootfs - alpine-3.12-rootfs firmware - BIOS Dasharo (coreboot+UEFI) v1.1.1 02/22/2023 , EFI v2.70 by EDK II , SMBIOS 3.3.0 , SeaBIOS (version rel-1.16.2-0-gea1b7a0-Xen), GRUB 2.06~rc1 12. Debugger - gdb 13. Xen code building infra - make 14. Testing OS - CentOS 7, Ubuntu, OpenSuse, Arch Linux, Alpine 3.12.12, Debian 10 (Buster), Fedora ( **I could not get the version info for some of these ^^^**) 15. Testing Infra - Gitlab runner, Docker 16. Testing tools common to all architectures - All the host OS packages dtc, build-essential, zlib1g-dev, libncurses5-dev, libssl-dev, python-dev, python3-dev, xorg-dev, uuid-dev, libyajl-dev, libaio-dev, libglib2.0-dev, libpixman-1-dev, pkg-config, flex, bison, gettext, acpica-tools, bin86, bcc, liblzma-dev, libc6-dev-i386, libnl-3-dev, ocaml-nox, libfindlib-ocaml-dev, markdown, transfig, pandoc, checkpolicy, wget, nasm, mkimage, uboot-tools 17. Documentation related tools - doxygen, rst. Did I miss anything ? Kind regards, Ayan
Re: [linux-linus test] 181082: regressions - FAIL
On 02.06.2023 05:21, osstest service owner wrote: > flight 181082 linux-linus real [real] > flight 181098 linux-linus real-retest [real] > http://logs.test-lab.xenproject.org/osstest/logs/181082/ > http://logs.test-lab.xenproject.org/osstest/logs/181098/ > > Regressions :-( > > Tests which did not succeed and are blocking, > including tests which could not be run: > test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. > 180278 Following up from yesterday's discussion, I've noticed only now that we had an apparently random success once in mid April. Without that, we'd see ... > Tests which did not succeed, but are not blocking: > test-armhf-armhf-examine 8 reboot fail like > 180278 > test-armhf-armhf-xl-arndale 8 xen-boot fail like > 180278 > test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like > 180278 > test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like > 180278 > test-armhf-armhf-xl-credit2 8 xen-boot fail like > 180278 > test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like > 180278 > test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like > 180278 > test-armhf-armhf-xl-multivcpu 8 xen-boot fail like > 180278 > test-armhf-armhf-libvirt-raw 8 xen-boot fail like > 180278 > test-armhf-armhf-libvirt 8 xen-boot fail like > 180278 > test-armhf-armhf-libvirt-qcow2 8 xen-bootfail like > 180278 > test-armhf-armhf-xl 8 xen-boot fail like > 180278 > test-armhf-armhf-xl-vhd 8 xen-boot fail like > 180278 > test-armhf-armhf-xl-rtds 8 xen-boot fail like > 180278 that singular test in the same group as all the other armhf ones. I wonder whether we shouldn't try to get those in sync. Which direction depends - aiui a force push would allow subsequent automatic pushes if only the armhf tests fail. Whereas clearing the "fail like" state for all of them would give a better picture of what's actually broken right now. Jan
Re: [PATCH] x86: Add Kconfig option to require NX bit support
On 01.06.2023 19:43, Alejandro Vallejo wrote:
> This allows replacing many instances of runtime checks with folded
> constants. The patch asserts support for the NX bit in PTEs at boot time
> and if so short-circuits cpu_has_nx to 1. This has several knock-on effects
> that improve codegen:
> * _PAGE_NX matches _PAGE_NX_BIT, optimising the macro to a constant.
> * Many PAGE_HYPERVISOR_X are also folded into constants
> * A few if ( cpu_has_nx ) statements are optimised out
>
> We save 2.5KiB off the text section and remove the runtime dependency for
> applying NX, which hardens our security posture. The config option defaults
> to OFF for compatibility with previous behaviour.
>
> Signed-off-by: Alejandro Vallejo
At a guess this may want a Suggested-by: Andrew?
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -307,6 +307,16 @@ config MEM_SHARING
> bool "Xen memory sharing support (UNSUPPORTED)" if UNSUPPORTED
> depends on HVM
>
> +config REQUIRE_NX_BIT
I don't think "_BIT" in the name is necessary or useful.
> + def_bool n
> + prompt "Require NX bit support"
Just
bool "Require NX bit support"
please.
> + ---help---
In new code just "help" please.
> @@ -151,6 +152,11 @@ not_multiboot:
> .Lnot_aligned:
> add $sym_offs(.Lbag_alg_msg),%esi # Error message
> jmp .Lget_vtb
> +#if IS_ENABLED(CONFIG_REQUIRE_NX_BIT)
> +no_nx_bit:
.Lno_nx_bit (no need for this to end up in the symbol table, just like
most other labels around here).
> @@ -647,11 +653,18 @@ trampoline_setup:
> cpuid
> 1: mov %edx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM) +
> sym_esi(boot_cpu_data)
>
> -/* Check for NX. Adjust EFER setting if available. */
> +/*
> + * Check for NX:
> + * - If Xen was compiled requiring it simply assert it's
> + * supported. The trampoline already has the right constant.
> + * - Otherwise, update the trampoline EFER mask accordingly.
> + */
> bt $cpufeat_bit(X86_FEATURE_NX), %edx
> -jnc 1f
> +jnc no_nx_bit
> +#if !IS_ENABLED(CONFIG_REQUIRE_NX_BIT)
> orb $EFER_NXE >> 8, 1 + sym_esi(trampoline_efer)
> -1:
> +no_nx_bit:
> +#endif
>
> /* Check for availability of long mode. */
> bt $cpufeat_bit(X86_FEATURE_LM),%edx
I think it would be nice if this check came ahead of the NX one, as
LM availability is quite a bit more important (and hence imo lack
thereof wants diagnosing first). Especially as the re-ordering looks
to be entirely trivial.
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -751,6 +751,15 @@ static void __init efi_arch_cpu(void)
> {
> caps[FEATURESET_e1d] = cpuid_edx(0x8001);
>
> +/*
> + * This check purposefully doesn't use cpu_has_nx because
> + * cpu_has_nx bypasses the boot_cpu_data read if Xen was compiled
> + * with CONFIG_REQUIRE_NX_BIT
> + */
> +if ( IS_ENABLED(CONFIG_REQUIRE_NX_BIT) &&
> + !boot_cpu_has(X86_FEATURE_NX) )
> +blexit(L"This Xen build requires NX bit support");
Nit: Nearby uses of blexit() suggest there wants to be a full stop.
Jan
Re: [XEN][PATCH v7 12/19] xen/smmu: Add remove_device callback for smmu_iommu ops
On 02/06/2023 02:48, Vikram Garhwal wrote:
> Add remove_device callback for removing the device entry from smmu-master
> using
> following steps:
> 1. Find if SMMU master exists for the device node.
> 2. Check if device is currently in use.
> 3. Remove the SMMU master.
>
> Signed-off-by: Vikram Garhwal
> Reviewed-by: Luca Fancellu
> Reviewed-by: Michal Orzel
> ---
> xen/drivers/passthrough/arm/smmu.c | 59 ++
> 1 file changed, 59 insertions(+)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index c37fa9af13..fdef6e7a7d 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -44,6 +44,7 @@
> #include
> #include
> #include
> +#include
Please list the headers in alphabetical order ('i' before 'l').
~Michal
Re: [XEN][PATCH v7 09/19] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller
On 02/06/2023 02:48, Vikram Garhwal wrote:
> Rename iommu_dt_device_is_assigned() to iommu_dt_device_is_assigned_locked().
> Remove static type so this can also be used by SMMU drivers to check if the
> device is being used before removing.
>
> Moving spin_lock to caller was done to prevent the concurrent access to
> iommu_dt_device_is_assigned while doing add/remove/assign/deassign.
Would be nice to explain the need of putting a prototype into a private header.
>
> Signed-off-by: Vikram Garhwal
>
> ---
> Changes from v6:
> Created a private header and moved iommu_dt_device_is_assigned() to
> header.
> ---
> xen/drivers/passthrough/device_tree.c | 20
> xen/include/xen/iommu-private.h | 27 +++
> 2 files changed, 43 insertions(+), 4 deletions(-)
> create mode 100644 xen/include/xen/iommu-private.h
>
> diff --git a/xen/drivers/passthrough/device_tree.c
> b/xen/drivers/passthrough/device_tree.c
> index 1c32d7b50c..52e370db01 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -83,16 +84,14 @@ fail:
> return rc;
> }
>
> -static bool_t iommu_dt_device_is_assigned(const struct dt_device_node *dev)
I would write a comment that this function expects to be called with
dtdevs_lock locked.
> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev)
> {
> bool_t assigned = 0;
>
> if ( !dt_device_is_protected(dev) )
> return 0;
>
> -spin_lock(&dtdevs_lock);
> assigned = !list_empty(&dev->domain_list);
> -spin_unlock(&dtdevs_lock);
>
> return assigned;
> }
> @@ -213,27 +212,40 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl,
> struct domain *d,
> if ( (d && d->is_dying) || domctl->u.assign_device.flags )
> break;
>
> +spin_lock(&dtdevs_lock);
> +
> ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
> domctl->u.assign_device.u.dt.size,
> &dev);
> if ( ret )
> +{
> +spin_unlock(&dtdevs_lock);
> break;
> +}
>
> ret = xsm_assign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
> if ( ret )
> +{
> +spin_unlock(&dtdevs_lock);
> break;
> +}
>
> if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
> {
> -if ( iommu_dt_device_is_assigned(dev) )
> +
> +if ( iommu_dt_device_is_assigned_locked(dev) )
> {
> printk(XENLOG_G_ERR "%s already assigned.\n",
> dt_node_full_name(dev));
> ret = -EINVAL;
> }
> +
> +spin_unlock(&dtdevs_lock);
> break;
> }
>
> +spin_unlock(&dtdevs_lock);
> +
> if ( d == dom_io )
> return -EINVAL;
>
> diff --git a/xen/include/xen/iommu-private.h b/xen/include/xen/iommu-private.h
> new file mode 100644
> index 00..5615decaff
> --- /dev/null
> +++ b/xen/include/xen/iommu-private.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> + /*
incorrect indentation (<< 1)
> + * xen/iommu-private.h
> + *
> + *
> + * Copyright (C) 2023, Advanced Micro Devices, Inc. All Rights Reserved.
> + * Written by Vikram Garhwal
I'm not sure if placing the copyright is appropriate, given a single prototype
for a function
not really implemented by you with just spinlocks removed. But I might be wrong.
> + *
> + */
> +#ifndef __XEN_IOMMU_PRIVATE_H__
> +#define __XEN_IOMMU_PRIVATE_H__
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +#include
> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node *dev);
> +#endif
> +
> +#endif /* __XEN_IOMMU_PRIVATE_H__ */
empty line here please
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
~Michal
Re: [XEN][PATCH v7 08/19] xen/device-tree: Add device_tree_find_node_by_path() to find nodes in device tree
On 02/06/2023 03:52, Henry Wang wrote: > > > Hi Vikram, > >> -Original Message- >> Subject: [XEN][PATCH v7 08/19] xen/device-tree: Add >> device_tree_find_node_by_path() to find nodes in device tree >> >> Add device_tree_find_node_by_path() to find a matching node with path for >> a >> dt_device_node. >> >> Reason behind this function: >> Each time overlay nodes are added using .dtbo, a new fdt(memcpy of >> device_tree_flattened) is created and updated with overlay nodes. This >> updated fdt is further unflattened to a dt_host_new. Next, we need to >> find >> the overlay nodes in dt_host_new, find the overlay node's parent in >> dt_host >> and add the nodes as child under their parent in the dt_host. Thus we >> need >> this function to search for node in different unflattened device trees. >> >> Also, make dt_find_node_by_path() static inline. >> >> Signed-off-by: Vikram Garhwal >> >> --- >> Changes from v6: >> Rename "dt_node" to "from" >> --- >> xen/common/device_tree.c | 6 -- >> xen/include/xen/device_tree.h | 18 -- >> 2 files changed, 20 insertions(+), 4 deletions(-) >> >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index 16b4b4e946..c5250a1644 100644 >> --- a/xen/common/device_tree.c >> +++ b/xen/common/device_tree.c >> @@ -358,11 +358,13 @@ struct dt_device_node >> *dt_find_node_by_type(struct dt_device_node *from, >> return np; >> } >> >> -struct dt_device_node *dt_find_node_by_path(const char *path) >> +struct dt_device_node * >> +device_tree_find_node_by_path(struct dt_device_node >> *from, >> + const char *path) > > NIT: I found that the indentation here is a bit strange to me. I personally > would > write like: > struct dt_device_node * > device_tree_find_node_by_path(struct dt_device_node *from, char *path) +1 With the indentation fixed: Reviewed-by: Michal Orzel ~Michal
Re: [XEN][PATCH v7 05/19] xen/arm: Add CONFIG_OVERLAY_DTB
On 02/06/2023 02:48, Vikram Garhwal wrote: > Introduce a config option where the user can enable support for > adding/removing > device tree nodes using a device tree binary overlay. > > Update SUPPORT.md and CHANGELOG.md to state the Device Tree Overlays support > for > Arm. > > Signed-off-by: Vikram Garhwal Reviewed-by: Michal Orzel ~Michal
Re: [XEN][PATCH v7 04/19] common/device_tree: change __unflatten_device_tree() type
On 02/06/2023 02:48, Vikram Garhwal wrote: > Following changes are done to __unflatten_device_tree(): > 1. __unflatten_device_tree() is renamed to unflatten_device_tree(). > 2. Remove __init and static function type. > > Signed-off-by: Vikram Garhwal > Reviewed-by: Henry Wang Reviewed-by: Michal Orzel ~Michal
Re: [XEN][PATCH v7 02/19] common/device_tree.c: unflatten_device_tree() propagate errors
Title: s/unflatten_device_tree/__unflatten_device_tree/ or you mean to propagate
errors from unflatten_dt_node?
On 02/06/2023 02:48, Vikram Garhwal wrote:
> This will be useful in dynamic node programming when new dt nodes are
> unflattend
> during runtime. Invalid device tree node related errors should be propagated
> back to the caller.
>
> Signed-off-by: Vikram Garhwal
> Reviewed-by: Henry Wang
> ---
> xen/common/device_tree.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index dfdb10e488..117b09 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -2108,6 +2108,9 @@ static int __init __unflatten_device_tree(const void
> *fdt,
> /* First pass, scan for size */
> start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
> size = unflatten_dt_node(fdt, 0, &start, NULL, NULL, 0);
> +if ( !size )
> +return -EINVAL;
> +
> size = (size | 3) + 1;
>
> dt_dprintk(" size is %#lx allocating...\n", size);
> @@ -2125,11 +2128,19 @@ static int __init __unflatten_device_tree(const void
> *fdt,
> start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
> unflatten_dt_node(fdt, mem, &start, NULL, &allnextp, 0);
> if ( be32_to_cpup((__be32 *)start) != FDT_END )
> -printk(XENLOG_WARNING "Weird tag at end of tree: %08x\n",
> +{
> +printk(XENLOG_ERR "Weird tag at end of tree: %08x\n",
>*((u32 *)start));
> +return -EINVAL;
What about memory that we allocated? Shouldn't it be freed in case of these two
errors?
For now it is called from boot so we do panic but later in this series this
could result
in a memory leak. Am I correct?
> +}
> +
> if ( be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef )
> -printk(XENLOG_WARNING "End of tree marker overwritten: %08x\n",
> +{
> +printk(XENLOG_ERR "End of tree marker overwritten: %08x\n",
>be32_to_cpu(((__be32 *)mem)[size / 4]));
> +return -EINVAL;
> +}
> +
> *allnextp = NULL;
>
> dt_dprintk(" <- unflatten_device_tree()\n");
~Michal
Re: [XEN][PATCH v7 01/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
On 02/06/2023 02:48, Vikram Garhwal wrote:
> Change __unflatten_device_tree() return type to integer so it can propagate
> memory allocation failure. Add panic() in dt_unflatten_host_device_tree() for
> memory allocation failure during boot.
>
> Fixes: fb97eb614acf ("xen/arm: Create a hierarchical device tree")
>
NIT: no blank line here
> Signed-off-by: Vikram Garhwal
> Reviewed-by: Henry Wang
Reviewed-by: Michal Orzel
~Michal
