[ovmf test] 149665: all pass - PUSHED
flight 149665 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/149665/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 8c654bb3ec0b5232dec2b2b07234c5479eb14d62 baseline version: ovmf bd6aa93296de36c5afabd34e4fa4083bccb8488d Last test of basis 149638 2020-04-13 10:09:17 Z2 days Testing same since 149665 2020-04-15 01:40:25 Z1 days1 attempts People who touched revisions under test: Christopher J Zurcher Zurcher, Christopher J 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 bd6aa93296..8c654bb3ec 8c654bb3ec0b5232dec2b2b07234c5479eb14d62 -> xen-tested-master
[xen-4.13-testing test] 149664: tolerable FAIL - PUSHED
flight 149664 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149664/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-libvirt 18 guest-start/debian.repeat fail in 149647 pass in 149664 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail pass in 149647 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail blocked in 149604 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen b66ce5058ec9ce84418cedd39b2bf07b7c5a1f65 baseline version: xen 736da59cbe2752502ad863b6e71eb83352e22276 Last test of basis 149604 2020-04-10 16:36:52 Z5 days Testing same since 149647 2020-04-14 13:06:03 Z1 days2 attempts People who touched revisions
[libvirt test] 149666: regressions - FAIL
flight 149666 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/149666/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-i386-libvirt6 libvirt-buildfail REGR. vs. 146182 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 146182 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a version targeted for testing: libvirt a7db0b757d210071d39e6d116e6a4bc761e2ed66 baseline version: libvirt a1cd25b919509be2645dbe6f952d5263e0d4e4e5 Last test of basis 146182 2020-01-17 06:00:23 Z 89 days Failing since146211 2020-01-18 04:18:52 Z 89 days 85 attempts Testing same since 149666 2020-04-15 04:18:44 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani Arnaud Patard Bjoern Walk Boris Fiuczynski Christian Borntraeger Christian Ehrhardt Christian Schoenebeck Collin Walling Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Dario Faggioli Erik Skultety Gaurav Agrawal Han Han Jim Fehlig Jiri Denemark Jonathon Jongsma Julio Faracco Ján Tomko Laine Stump Lin Ma Marc-André Lureau Marek Marczykowski-Górecki Mauro S. M. Rodrigues Michal Privoznik Nikolay Shirokovskiy Pavel Hrdina Pavel Mores Peter Krempa Pino Toscano Prathamesh Chavan Rafael Fonseca Richard W.M. Jones Rikard Falkeborn Ryan Moeller Sahid Orentino Ferdjaoui Sebastian Mitterle Seeteena Thoufeek Stefan Berger Stefan Berger Stefan Hajnoczi Thomas Huth Wu Qingliang Yi Li Your Name Zhang Bo zhenwei pi Zhimin Feng jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairblocked test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw
[qemu-mainline test] 149660: tolerable FAIL - PUSHED
flight 149660 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/149660/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 15 guest-saverestorefail REGR. vs. 149641 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 149641 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149641 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149641 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149641 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149641 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149641 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: qemuua457215ed2aaa9598bd4ebbc6745d2a494ba9990 baseline version: qemuu14e5526b51910efd62cd31cd95b49baca975c83f Last test of basis 149641 2020-04-14 00:36:57 Z2 days Testing same since 149660 2020-04-14 19:36:24 Z1 days1 attempts People who touched revisions under test: Peter Maydell jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf
[linux-linus test] 149659: regressions - FAIL
flight 149659 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/149659/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-examine 4 memdisk-try-append fail REGR. vs. 149632 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 16 guest-localmigrate fail REGR. vs. 149632 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 149632 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 149632 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149632 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 149632 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149632 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149632 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149632 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 149632 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149632 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: linux8632e9b5645bbc2331d21d892b0d6961c1a08429 baseline version: linux8f3d9f354286745c751374f5f1fcafee6b3f3136 Last test of basis 149632 2020-04-13 01:08:56 Z2 days Testing same since 149659 2020-04-14 19:09:17 Z1 days1 attempts
[qemu-upstream-4.13-testing test] 149658: tolerable FAIL - PUSHED
flight 149658 qemu-upstream-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149658/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass version targeted for testing: qemuu730e2b1927e7d911bbd5350714054ddd5912f4ed baseline version: qemuu933ebad2470a169504799a1d95b8e410bd9847ef Last test of basis 144391 2019-11-29 15:07:52 Z 138 days Testing same since 149658 2020-04-14 18:08:38 Z1 days1 attempts People who touched revisions under test: Adrian Moreno Alberto Garcia Anthony PERARD Aurelien Jarno Boris Fiuczynski Christian Borntraeger Christophe Lyon Cornelia Huck Daniel P. Berrangé David Hildenbrand Dr. David Alan Gilbert Eduardo Habkost Fan Yang Gerd Hoffmann Guoyi Tu Hikaru Nishida Igor Mammedov Jason Wang Johannes Berg John Snow Kevin Wolf Markus Armbruster Matthew Rosato Max Filippov Max Reitz Maxim Levitsky Michael Roth Michael S. Tsirkin Michael Weiser
[PATCH v4 4/4] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
From: Wei Liu
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
---
Changed in v4:
- switch to normal unmap_domain_page() for variable right before
end-of-scope.
Changed in v3:
- rename l2_ro_mpt into pl2e to avoid confusion.
Changed in v2:
- no point in re-mapping l2t because it is exactly the same as
l2_ro_mpt.
- point l2_ro_mpt to the entry instead of doing l2_table_offset() all
the time.
---
xen/arch/x86/x86_64/mm.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cfaeae84e9..e85ef449f3 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -263,7 +263,8 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
unsigned long i, va, rwva;
unsigned long smap = info->spfn, emap = info->epfn;
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
+l3_ro_mpt = map_l3t_from_l4e(
+idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
/*
* No need to clean m2p structure existing before the hotplug
@@ -271,7 +272,7 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
for (i = smap; i < emap;)
{
unsigned long pt_pfn;
-l2_pgentry_t *l2_ro_mpt;
+l2_pgentry_t *pl2e;
va = RO_MPT_VIRT_START + i * sizeof(*machine_to_phys_mapping);
rwva = RDWR_MPT_VIRT_START + i * sizeof(*machine_to_phys_mapping);
@@ -285,26 +286,30 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
continue;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
-if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT))
+pl2e = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]) +
+l2_table_offset(va);
+if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
{
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3)) ;
+UNMAP_DOMAIN_PAGE(pl2e);
continue;
}
-pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
+pt_pfn = l2e_get_pfn(*pl2e);
if ( hotadd_mem_valid(pt_pfn, info) )
{
destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
-l2e_write(_ro_mpt[l2_table_offset(va)], l2e_empty());
+l2e_write(pl2e, l2e_empty());
}
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3));
+unmap_domain_page(pl2e);
}
+UNMAP_DOMAIN_PAGE(l3_ro_mpt);
+
destroy_compat_m2p_mapping(info);
/* Brute-Force flush all TLB */
--
2.24.1.AMZN
[PATCH v4 0/4] use new API for Xen page tables
From: Hongyan Xia This small series is basically just rewriting functions using the new API to map and unmap PTEs. Each patch is independent. Apart from mapping and unmapping page tables, no other functional change intended. --- Changed in v4: - use _ suffix instead of prefix in macros. - use normal unmap_domain_page() for variable right before end-of-scope. Changed in v3: - address all comments in v2. - drop patch 4, since other clean-ups will make it unnecessary. Changed in v2: - I kept UNMAP_DOMAIN_PAGE() for now in v2, but I if people say in some cases it is an overkill and unmap_domain_page() should be used, I am okay with that and can make the change. - code cleanup and style fixes. - unmap as early as possible. Wei Liu (4): x86/shim: map and unmap page tables in replace_va_mapping x86_64/mm: map and unmap page tables in m2p_mapped x86_64/mm: map and unmap page tables in share_hotadd_m2p_table x86_64/mm: map and unmap page tables in destroy_m2p_mapping xen/arch/x86/pv/shim.c | 9 xen/arch/x86/x86_64/mm.c | 44 +- xen/include/asm-x86/page.h | 19 3 files changed, 48 insertions(+), 24 deletions(-) -- 2.24.1.AMZN
[PATCH v4 3/4] x86_64/mm: map and unmap page tables in share_hotadd_m2p_table
From: Wei Liu
Fetch lYe by mapping and unmapping lXe instead of using the direct map,
which is now done via the lYe_from_lXe() helpers.
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
Reviewed-by: Jan Beulich
---
Changed in v2:
- the introduction of the macros is now lifted to a previous patch.
---
xen/arch/x86/x86_64/mm.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 41755ded26..cfaeae84e9 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -166,14 +166,14 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v += n << PAGE_SHIFT )
{
n = L2_PAGETABLE_ENTRIES * L1_PAGETABLE_ENTRIES;
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
if ( !(l3e_get_flags(l3e) & _PAGE_PSE) )
{
n = L1_PAGETABLE_ENTRIES;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
@@ -194,11 +194,11 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v != RDWR_COMPAT_MPT_VIRT_END;
v += 1 << L2_PAGETABLE_SHIFT )
{
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
--
2.24.1.AMZN
[PATCH v4 2/4] x86_64/mm: map and unmap page tables in m2p_mapped
From: Wei Liu
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
Reviewed-by: Jan Beulich
---
Changed in v3:
- rename l3e_ro_mpt and l2e_ro_mpt, just call them l3e and l2e.
Changed in v2:
- avoid adding goto labels, simply get the PTE and unmap quickly.
- code style fixes.
---
xen/arch/x86/x86_64/mm.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cee836ec37..41755ded26 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -129,14 +129,13 @@ static mfn_t alloc_hotadd_mfn(struct mem_hotadd_info
*info)
static int m2p_mapped(unsigned long spfn)
{
unsigned long va;
-l3_pgentry_t *l3_ro_mpt;
-l2_pgentry_t *l2_ro_mpt;
+l3_pgentry_t l3e;
+l2_pgentry_t l2e;
va = RO_MPT_VIRT_START + spfn * sizeof(*machine_to_phys_mapping);
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(va)],
l3_table_offset(va));
-switch ( l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
- (_PAGE_PRESENT |_PAGE_PSE))
+switch ( l3e_get_flags(l3e) & (_PAGE_PRESENT | _PAGE_PSE) )
{
case _PAGE_PSE|_PAGE_PRESENT:
return M2P_1G_MAPPED;
@@ -146,9 +145,9 @@ static int m2p_mapped(unsigned long spfn)
default:
return M2P_NO_MAPPED;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
+l2e = l2e_from_l3e(l3e, l2_table_offset(va));
-if (l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT)
+if ( l2e_get_flags(l2e) & _PAGE_PRESENT )
return M2P_2M_MAPPED;
return M2P_NO_MAPPED;
--
2.24.1.AMZN
[PATCH v4 1/4] x86/shim: map and unmap page tables in replace_va_mapping
From: Wei Liu
Also, introduce lYe_from_lXe() macros which do not rely on the direct
map when walking page tables. Unfortunately, they cannot be inline
functions due to the header dependency on domain_page.h, so keep them as
macros just like map_lYt_from_lXe().
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
---
Changed in v4:
- use _ suffixes instead of prefixes.
Changed in v3:
- use unmap_domain_page() instead of the macro in several places.
- also introduce l1e_from_l2e().
- add _ prefix in macros to avoid aliasing.
Changed in v2:
- instead of map, map, map, read/write, unmap, unmap, unmap, do map,
read PTE, unmap for each level instead.
- use lYe_from_lXe() macros and lift them from a later patch to this
patch.
- const qualify pointers in new macros.
---
xen/arch/x86/pv/shim.c | 9 +
xen/include/asm-x86/page.h | 19 +++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index ed2ece8a8a..31264582cc 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -168,16 +168,17 @@ const struct platform_bad_page *__init
pv_shim_reserved_pages(unsigned int *size
static void __init replace_va_mapping(struct domain *d, l4_pgentry_t *l4start,
unsigned long va, mfn_t mfn)
{
-l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
-l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
-l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
-l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
+l4_pgentry_t l4e = l4start[l4_table_offset(va)];
+l3_pgentry_t l3e = l3e_from_l4e(l4e, l3_table_offset(va));
+l2_pgentry_t l2e = l2e_from_l3e(l3e, l2_table_offset(va));
+l1_pgentry_t *pl1e = map_l1t_from_l2e(l2e) + l1_table_offset(va);
struct page_info *page = mfn_to_page(l1e_get_mfn(*pl1e));
put_page_and_type(page);
*pl1e = l1e_from_mfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
: COMPAT_L1_PROT));
+unmap_domain_page(pl1e);
}
static void evtchn_reserve(struct domain *d, unsigned int port)
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index eb73a0fc23..5acf3d3d5a 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -197,6 +197,25 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa,
unsigned int flags)
#define map_l2t_from_l3e(x)(l2_pgentry_t
*)map_domain_page(l3e_get_mfn(x))
#define map_l3t_from_l4e(x)(l3_pgentry_t
*)map_domain_page(l4e_get_mfn(x))
+/* Unlike lYe_to_lXe(), lXe_from_lYe() do not rely on the direct map. */
+#define l1e_from_l2e(l2e_, offset_) ({ \
+const l1_pgentry_t *l1t_ = map_l1t_from_l2e(l2e_); \
+l1_pgentry_t l1e_ = l1t_[offset_]; \
+unmap_domain_page(l1t_);\
+l1e_; })
+
+#define l2e_from_l3e(l3e_, offset_) ({ \
+const l2_pgentry_t *l2t_ = map_l2t_from_l3e(l3e_); \
+l2_pgentry_t l2e_ = l2t_[offset_]; \
+unmap_domain_page(l2t_);\
+l2e_; })
+
+#define l3e_from_l4e(l4e_, offset_) ({ \
+const l3_pgentry_t *l3t_ = map_l3t_from_l4e(l4e_); \
+l3_pgentry_t l3e_ = l3t_[offset_]; \
+unmap_domain_page(l3t_);\
+l3e_; })
+
/* Given a virtual address, get an entry offset into a page table. */
#define l1_table_offset(a) \
(((a) >> L1_PAGETABLE_SHIFT) & (L1_PAGETABLE_ENTRIES - 1))
--
2.24.1.AMZN
[PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
The PERFC_INCR() macro uses current->processor, but current is not valid during early boot. This causes the following crash to occur if e.g. rdmsr_safe() has to recover from a #GP fault. (XEN) Early fatal page fault at e008:82d0803b1a39 (cr2=0004, ec=) (XEN) [ Xen-4.14-unstable x86_64 debug=y Not tainted ] (XEN) CPU:0 (XEN) RIP:e008:[] x86_64/entry.S#handle_exception_saved+0x64/0xb8 ... (XEN) Xen call trace: (XEN)[] R x86_64/entry.S#handle_exception_saved+0x64/0xb8 (XEN)[] F __start_xen+0x2cd/0x2980 (XEN)[] F __high_start+0x4c/0x4e Furthermore, the PERFC_INCR() macro is wildly inefficient. There has been a single caller for many releases now, so inline it and delete the macro completely. For the assembly, move entry_vector from %eax into %ecx. There is no encoding benefit for movzbl, and it frees up %eax to be used inside the CONFIG_PERF_COUNTERS block where there is an encoding benefit. There is no need to reference current at all. What is actually needed is the per_cpu_offset which can be obtained directly from the top-of-stack block. This simplifies the counter handling to 3 instructions and no spilling to the stack at all. The same breakage from above is now handled properly: (XEN) traps.c:1591: GPF (): 82d0806394fe [__start_xen+0x2cd/0x2980] -> 82d0803b3bfb Reported-by: Julien Grall Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Julien Grall --- xen/arch/x86/x86_64/asm-offsets.c | 1 + xen/arch/x86/x86_64/entry.S | 10 +++--- xen/include/asm-x86/asm_defns.h | 16 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index b8e8510439..500df7a3e7 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -112,6 +112,7 @@ void __dummy__(void) OFFSET(CPUINFO_guest_cpu_user_regs, struct cpu_info, guest_cpu_user_regs); OFFSET(CPUINFO_verw_sel, struct cpu_info, verw_sel); OFFSET(CPUINFO_current_vcpu, struct cpu_info, current_vcpu); +OFFSET(CPUINFO_per_cpu_offset, struct cpu_info, per_cpu_offset); OFFSET(CPUINFO_cr4, struct cpu_info, cr4); OFFSET(CPUINFO_xen_cr3, struct cpu_info, xen_cr3); OFFSET(CPUINFO_pv_cr3, struct cpu_info, pv_cr3); diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 997c481ecb..52bd41d9f6 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -677,10 +677,14 @@ handle_exception_saved: #endif /* CONFIG_PV */ sti 1: movq %rsp,%rdi -movzbl UREGS_entry_vector(%rsp),%eax +movzbl UREGS_entry_vector(%rsp), %ecx leaq exception_table(%rip),%rdx -PERFC_INCR(exceptions, %rax, %rbx) -mov (%rdx, %rax, 8), %rdx +#ifdef CONFIG_PERF_COUNTERS +lea per_cpu__perfcounters(%rip), %rax +add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rax +incl ASM_PERFC_exceptions * 4(%rax, %rcx, 4) +#endif +mov (%rdx, %rcx, 8), %rdx INDIRECT_CALL %rdx mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) mov %r13b, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14) diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h index bc9d9fcdb2..b42a19b654 100644 --- a/xen/include/asm-x86/asm_defns.h +++ b/xen/include/asm-x86/asm_defns.h @@ -346,22 +346,6 @@ static always_inline void stac(void) #endif -#ifdef CONFIG_PERF_COUNTERS -#define PERFC_INCR(_name,_idx,_cur) \ -pushq _cur; \ -movslq VCPU_processor(_cur),_cur; \ -pushq %rdx; \ -leaq __per_cpu_offset(%rip),%rdx; \ -movq (%rdx,_cur,8),_cur;\ -leaq per_cpu__perfcounters(%rip),%rdx; \ -addq %rdx,_cur; \ -popq %rdx; \ -incl ASM_PERFC_##_name*4(_cur,_idx,4); \ -popq _cur -#else -#define PERFC_INCR(_name,_idx,_cur) -#endif - /* Work around AMD erratum #88 */ #define safe_swapgs \ "mfence; swapgs;" -- 2.11.0
Re: [XEN PATCH v4 12/18] xen/build: factorise generation of the linker scripts
On Wed, Apr 08, 2020 at 02:46:42PM +0200, Jan Beulich wrote:
> On 31.03.2020 12:30, Anthony PERARD wrote:
> > - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line is
> > still mandatory for if_changed (or cmd) macro to work.
>
> I still don't believe in there being a need for "; \" there. This
> actually breaks things, after all:
>
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -236,6 +236,12 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags))
> > $< -o $@
> > %.s: %.S FORCE
> > $(call if_changed,cpp_s_S)
> >
> > +# Linker scripts, .lds.S -> .lds
> > +quiet_cmd_cc_lds_S = LDS $@
> > +cmd_cc_lds_S = $(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \
> > +sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new;
> > \
> > +mv -f $(dot-target).d.new $(dot-target).d
>
> if $(CPP) or sed fail, previously the whole rule would have failed,
> which no longer is the case with your use of semicolons. There
> ought to be a solution to this, ideally one better than adding
> "set -e" as the first command ("define" would at least deal with
> the multi-line make issue, but without it being clear to me why the
> semicolons would be needed I don't think I can suggest anything
> there at the moment).
The only macro that will consumes cmd_cc_lds_S (and other cmd_*) is
"cmd", it is defined as:
cmd = @set -e; $(echo-cmd) $(cmd_$(1))
So, "set -e" is already there, and using semicolons in commands is
equivalent to using "&&".
With "cmd" alone, multi-line command would work as expected (unless
$(echo-cmd) is is trying to print the command line).
It's "if_changed" macro that doesn't work with multi-line commands.
It does:
$(cmd); printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
With a multiple line command, $(make-cmd) get's expanded to multiple
line, so the second argument of "printf" is going to be spread over
multiple line in make, and thus multiple shell. We run into this error:
/bin/sh: -c: line 0: unexpected EOF while looking for matching `''
/bin/sh: -c: line 1: syntax error: unexpected end of file
This is why we need to have commands on a single line.
I hope the explanation is clear enough.
Thanks,
--
Anthony PERARD
Re: [XEN PATCH v4 10/18] xen/build: use if_changed on built_in.o
On Wed, Apr 08, 2020 at 03:35:17PM +0200, Jan Beulich wrote: > On 08.04.2020 15:13, Andrew Cooper wrote: > > On 08/04/2020 13:40, Jan Beulich wrote: > >> On 31.03.2020 12:30, Anthony PERARD wrote: > >>> --- a/xen/Rules.mk > >>> +++ b/xen/Rules.mk > >>> @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk > >>> c_flags += $(CFLAGS-y) > >>> a_flags += $(CFLAGS-y) $(AFLAGS-y) > >>> > >>> -built_in.o: $(obj-y) $(extra-y) > >>> -ifeq ($(obj-y),) > >>> - $(CC) $(c_flags) -c -x c /dev/null -o $@ > >>> -else > >>> +quiet_cmd_ld_builtin = LD $@ > >>> ifeq ($(CONFIG_LTO),y) > >>> - $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^) > >>> +cmd_ld_builtin = \ > >>> +$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs)) > >>> else > >>> - $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^) > >>> +cmd_ld_builtin = \ > >>> +$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out > >>> $(extra-y),$(real-prereqs)) > >>> endif > >> How about going yet one step further and doing away with the > >> ifeq here altogether: > >> > >> cmd_ld_builtin-y = \ > >> $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs)) > >> cmd_ld_builtin-$(CONFIG_LTO) = \ > >> $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs)) > > > > Please don't. > > > > Logic like this is substantially harder to follow than a plain if/else > > construct, and clarity is of far higher importance than optimising the > > line count in the build system. > > I could maybe see the argument if the two definitions were far apart. > This suggestion isn't about line count at all, but about clarity. In > particular because of the need to use ifeq(,) rather than simple "if" > constructs, I view this list model as the better alternative in all > cases where it can be made use of. We could use "ifdef CONFIG_LTO" to avoid ifeq ;-). But I think you disliked that because CONFIG_LTO could be present in the environment with a value different than "y" and mess up the build system, just to annoy us. > > This trick only works for trivial cases, and interferes with diff's when > > the logic inevitably becomes less trivial. > > It may, but whether it actually will can't be known until such time > as it would get touched. The suggested model may very well also be > suitable then. > > Anyway, Anthony, I'm not going to insist. This is just another aspect > where we would better generally settle on the preferred style to use. I think if/else is better for alternatives. And we can keep "var-y" for lists with optional items. Thanks, -- Anthony PERARD
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On 15.04.2020 17:54, Roger Pau Monné wrote:
> On Wed, Apr 15, 2020 at 05:42:20PM +0200, Jan Beulich wrote:
>> On 15.04.2020 16:49, Roger Pau Monné wrote:
>>> On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote:
On 14.04.2020 16:53, Roger Pau Monné wrote:
> On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote:
>> On 14.04.2020 13:19, Roger Pau Monné wrote:
>>> On Tue, Apr 14, 2020 at 12:13:04PM +0200, Jan Beulich wrote:
On 14.04.2020 12:02, Roger Pau Monné wrote:
> That seems nice, we would have to be careful however as reducing the
> number of ASID/VPID flushes could uncover issues in the existing code.
> I guess you mean something like:
>
> static inline void guest_flush_tlb_mask(const struct domain *d,
> const cpumask_t *mask)
> {
> flush_mask(mask, (is_pv_domain(d) || shadow_mode_enabled(d) ?
> FLUSH_TLB
> : 0) |
>(is_hvm_domain(d) && cpu_has_svm ?
> FLUSH_HVM_ASID_CORE
> : 0));
> }
Almost - is_hvm_domain(d) && cpu_has_svm seems to wide for me. I'd
rather use hap_enabled() && cpu_has_svm, which effectively means NPT.
Or am I overlooking a need to do ASID flushes also in shadow mode?
>>>
>>> I think so, I've used is_hvm_domain in order to cover for HVM domains
>>> running in shadow mode on AMD hardware, I think those also need the
>>> ASID flushes.
>>
>> I'm unconvinced: The entire section "TLB Management" in the PM gives
>> the impression that "ordinary" TLB flushing covers all ASIDs anyway.
>> It's not stated anywhere (I could find) explicitly though.
>
> Hm, I don't think so. XenRT found a myriad of issues with NPT when p2m
> code wasn't modified to do ASID flushes instead of plain TLB flushes.
Well, that's clear from e.g. p2m_pt_set_entry() not doing any
flushing itself.
> Even if it's just to stay on the safe side I would perform ASID
> flushes for HVM guests with shadow running on AMD.
Tim, any chance you could voice your thoughts here? To me it seems
odd to do an all-ASIDs flush followed by an ASID one.
>>>
>>> I've been reading a bit more into this, and section 15.16.1 states:
>>>
>>> "TLB flush operations must not be assumed to affect all ASIDs."
>>
>> That's the section talking about the tlb_control VMCB field. It is
>> in this context that the sentence needs to be interpreted, imo.
>
> It explicitly mentions move-to-cr3 and move-to-cr4 before that phrase:
>
> "TLB flush operations function identically whether or not SVM is
> enabled (e.g., MOV-TO-CR3 flushes non-global mappings, whereas
> MOV-TO-CR4 flushes global and non-global mappings). TLB flush
> operations must not be assumed to affect all ASIDs."
Hmm, indeed. How did I not spot this already when reading this the
other day?
> So my reading is that normal flush operations not involving
> tlb_control VMCB field should not assume to flush all ASIDs. Again
> this is of course my interpretation of the text in the PM. I will go
> with my suggested approach, which is safer and should cause no
> functional issues AFAICT. The only downside is the maybe redundant
> flush, but that's safe.
Okay. And I'm sorry for having attempted to mislead you.
Jan
Re: [XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
On 15.04.2020 16:10, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 01:50:33PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:30, Anthony PERARD wrote: >>> # Always build obj-bin files as binary even if they come from C source. >>> -$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) >>> +# FIXME LTO broken, but we would need a different way to filter -flto out >>> +# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) >> >> While you mention this in the description, I'm still not overly >> happy with it getting commented out. What's wrong with making the >> construct simply act on c_flags? > > It doesn't work. > > I tried > $(obj-bin-y): c_flags := $(filter-out -flto,$(c_flags)) > but the $@ expansion was empty. Hmm, yes, presumably because of having to use :=. But the old code gives the appearance of having worked despite this fact. > I guess we could do: > $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) > that's probably enough for now. Even if we can't test it properly. If -flto gets added to XEN_CFLAGS (not c_flags) - why not? Jan
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On Wed, Apr 15, 2020 at 05:42:20PM +0200, Jan Beulich wrote:
> On 15.04.2020 16:49, Roger Pau Monné wrote:
> > On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote:
> >> On 14.04.2020 16:53, Roger Pau Monné wrote:
> >>> On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote:
> On 14.04.2020 13:19, Roger Pau Monné wrote:
> > On Tue, Apr 14, 2020 at 12:13:04PM +0200, Jan Beulich wrote:
> >> On 14.04.2020 12:02, Roger Pau Monné wrote:
> >>> That seems nice, we would have to be careful however as reducing the
> >>> number of ASID/VPID flushes could uncover issues in the existing code.
> >>> I guess you mean something like:
> >>>
> >>> static inline void guest_flush_tlb_mask(const struct domain *d,
> >>> const cpumask_t *mask)
> >>> {
> >>> flush_mask(mask, (is_pv_domain(d) || shadow_mode_enabled(d) ?
> >>> FLUSH_TLB
> >>> : 0) |
> >>>(is_hvm_domain(d) && cpu_has_svm ?
> >>> FLUSH_HVM_ASID_CORE
> >>> : 0));
> >>> }
> >>
> >> Almost - is_hvm_domain(d) && cpu_has_svm seems to wide for me. I'd
> >> rather use hap_enabled() && cpu_has_svm, which effectively means NPT.
> >> Or am I overlooking a need to do ASID flushes also in shadow mode?
> >
> > I think so, I've used is_hvm_domain in order to cover for HVM domains
> > running in shadow mode on AMD hardware, I think those also need the
> > ASID flushes.
>
> I'm unconvinced: The entire section "TLB Management" in the PM gives
> the impression that "ordinary" TLB flushing covers all ASIDs anyway.
> It's not stated anywhere (I could find) explicitly though.
> >>>
> >>> Hm, I don't think so. XenRT found a myriad of issues with NPT when p2m
> >>> code wasn't modified to do ASID flushes instead of plain TLB flushes.
> >>
> >> Well, that's clear from e.g. p2m_pt_set_entry() not doing any
> >> flushing itself.
> >>
> >>> Even if it's just to stay on the safe side I would perform ASID
> >>> flushes for HVM guests with shadow running on AMD.
> >>
> >> Tim, any chance you could voice your thoughts here? To me it seems
> >> odd to do an all-ASIDs flush followed by an ASID one.
> >
> > I've been reading a bit more into this, and section 15.16.1 states:
> >
> > "TLB flush operations must not be assumed to affect all ASIDs."
>
> That's the section talking about the tlb_control VMCB field. It is
> in this context that the sentence needs to be interpreted, imo.
It explicitly mentions move-to-cr3 and move-to-cr4 before that phrase:
"TLB flush operations function identically whether or not SVM is
enabled (e.g., MOV-TO-CR3 flushes non-global mappings, whereas
MOV-TO-CR4 flushes global and non-global mappings). TLB flush
operations must not be assumed to affect all ASIDs."
So my reading is that normal flush operations not involving
tlb_control VMCB field should not assume to flush all ASIDs. Again
this is of course my interpretation of the text in the PM. I will go
with my suggested approach, which is safer and should cause no
functional issues AFAICT. The only downside is the maybe redundant
flush, but that's safe.
Thanks, Roger.
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On 15.04.2020 16:49, Roger Pau Monné wrote:
> On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote:
>> On 14.04.2020 16:53, Roger Pau Monné wrote:
>>> On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote:
On 14.04.2020 13:19, Roger Pau Monné wrote:
> On Tue, Apr 14, 2020 at 12:13:04PM +0200, Jan Beulich wrote:
>> On 14.04.2020 12:02, Roger Pau Monné wrote:
>>> That seems nice, we would have to be careful however as reducing the
>>> number of ASID/VPID flushes could uncover issues in the existing code.
>>> I guess you mean something like:
>>>
>>> static inline void guest_flush_tlb_mask(const struct domain *d,
>>> const cpumask_t *mask)
>>> {
>>> flush_mask(mask, (is_pv_domain(d) || shadow_mode_enabled(d) ?
>>> FLUSH_TLB
>>> : 0) |
>>> (is_hvm_domain(d) && cpu_has_svm ?
>>> FLUSH_HVM_ASID_CORE
>>> : 0));
>>> }
>>
>> Almost - is_hvm_domain(d) && cpu_has_svm seems to wide for me. I'd
>> rather use hap_enabled() && cpu_has_svm, which effectively means NPT.
>> Or am I overlooking a need to do ASID flushes also in shadow mode?
>
> I think so, I've used is_hvm_domain in order to cover for HVM domains
> running in shadow mode on AMD hardware, I think those also need the
> ASID flushes.
I'm unconvinced: The entire section "TLB Management" in the PM gives
the impression that "ordinary" TLB flushing covers all ASIDs anyway.
It's not stated anywhere (I could find) explicitly though.
>>>
>>> Hm, I don't think so. XenRT found a myriad of issues with NPT when p2m
>>> code wasn't modified to do ASID flushes instead of plain TLB flushes.
>>
>> Well, that's clear from e.g. p2m_pt_set_entry() not doing any
>> flushing itself.
>>
>>> Even if it's just to stay on the safe side I would perform ASID
>>> flushes for HVM guests with shadow running on AMD.
>>
>> Tim, any chance you could voice your thoughts here? To me it seems
>> odd to do an all-ASIDs flush followed by an ASID one.
>
> I've been reading a bit more into this, and section 15.16.1 states:
>
> "TLB flush operations must not be assumed to affect all ASIDs."
That's the section talking about the tlb_control VMCB field. It is
in this context that the sentence needs to be interpreted, imo.
Jan
[xen-4.11-testing test] 149651: tolerable FAIL - PUSHED
flight 149651 xen-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149651/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen d353f82b2edae3019b0b9405976a05f18d120ce7 baseline version: xen affb032b9b2624b67ffc7fb246a915dd08074b3f Last test of basis 149556 2020-04-09 08:42:59 Z6 days Testing same since 149651 2020-04-14 13:35:44 Z1 days1 attempts People who touched revisions under test: Jan Beulich Julien Grall Pawel Wieczorkiewicz Ross Lagerwall jobs: build-amd64-xsm
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote:
> On 14.04.2020 16:53, Roger Pau Monné wrote:
> > On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote:
> >> On 14.04.2020 13:19, Roger Pau Monné wrote:
> >>> On Tue, Apr 14, 2020 at 12:13:04PM +0200, Jan Beulich wrote:
> On 14.04.2020 12:02, Roger Pau Monné wrote:
> > That seems nice, we would have to be careful however as reducing the
> > number of ASID/VPID flushes could uncover issues in the existing code.
> > I guess you mean something like:
> >
> > static inline void guest_flush_tlb_mask(const struct domain *d,
> > const cpumask_t *mask)
> > {
> > flush_mask(mask, (is_pv_domain(d) || shadow_mode_enabled(d) ?
> > FLUSH_TLB
> > : 0) |
> > (is_hvm_domain(d) && cpu_has_svm ?
> > FLUSH_HVM_ASID_CORE
> > : 0));
> > }
>
> Almost - is_hvm_domain(d) && cpu_has_svm seems to wide for me. I'd
> rather use hap_enabled() && cpu_has_svm, which effectively means NPT.
> Or am I overlooking a need to do ASID flushes also in shadow mode?
> >>>
> >>> I think so, I've used is_hvm_domain in order to cover for HVM domains
> >>> running in shadow mode on AMD hardware, I think those also need the
> >>> ASID flushes.
> >>
> >> I'm unconvinced: The entire section "TLB Management" in the PM gives
> >> the impression that "ordinary" TLB flushing covers all ASIDs anyway.
> >> It's not stated anywhere (I could find) explicitly though.
> >
> > Hm, I don't think so. XenRT found a myriad of issues with NPT when p2m
> > code wasn't modified to do ASID flushes instead of plain TLB flushes.
>
> Well, that's clear from e.g. p2m_pt_set_entry() not doing any
> flushing itself.
>
> > Even if it's just to stay on the safe side I would perform ASID
> > flushes for HVM guests with shadow running on AMD.
>
> Tim, any chance you could voice your thoughts here? To me it seems
> odd to do an all-ASIDs flush followed by an ASID one.
I've been reading a bit more into this, and section 15.16.1 states:
"TLB flush operations must not be assumed to affect all ASIDs."
Since the VMM runs on it's own ASID it's my understanding that doing a
TLB flush from the VMM does not flush any of the guests TLBs, and
hence an ASID flush is still required.
Thanks, Roger.
Re: [XEN PATCH v4 07/18] build: Introduce documentation for xen Makefiles
On Wed, Apr 08, 2020 at 02:00:43PM +0200, Jan Beulich wrote: > On 31.03.2020 12:30, Anthony PERARD wrote: > > This start explainning the variables that can be used in the many > > Makefiles in xen/. > > > > Most of the document copies and modifies text from Linux v5.4 document > > linux.git/Documentation/kbuild/makefiles.rst. Modification are mostly > > to avoid mentioning kbuild. Thus I've added the SPDX tag which was > > only in index.rst in linux.git. > > > > Signed-off-by: Anthony PERARD > > Acked-by: Jan Beulich > with one question: > > > +Compilation flags > > +- > > + > > +CFLAGS-y and AFLAGS-y > > + These two flags apply only to the makefile in which they > > + are assigned. They are used for all the normal cc, as and ld > > + invocations happening during a recursive build. > > + > > + $(CFLAGS-y) is necessary because the top Makefile owns the > > + variable $(XEN_CFLAGS) and uses it for compilation flags for the > > + entire tree. And the variable $(CFLAGS) is modified by Config.mk > > + which evaluated in every subdirs. > > + > > + CFLAGS-y specifies options for compiling with $(CC). > > + AFLAGS-y specifies assembler options. > > Is it perhaps worth mentioning that c_flags and a_flags should > not be fiddled with by individual Makefile-s? No, I don't think that's needed. Outside of Rules.mk-s nothing modifies c_flags, so there isn't a good reason to modify CFLAGS via c_flags after looking for other examples. If they do change c_flags anyway, I don't think they would have read that new documentation. Thanks, -- Anthony PERARD
Re: [XEN PATCH v5] hvmloader: Enable MMIO and I/O decode, after all resource allocation
On Wed, 2020-04-15 at 16:14 +0200, Jan Beulich wrote: > CAUTION: This email originated from outside of the organization. Do > not click links or open attachments unless you can confirm the sender > and know the content is safe. > > > > On 15.04.2020 14:27, Harsha Shamsundara Havanur wrote: > > It was observed that PCI MMIO and/or IO BARs were programmed with > > memory and I/O decodes (bits 0 and 1 of PCI COMMAND register) > > enabled, > > during PCI setup phase. This resulted in incorrect memory mapping > > as > > soon as the lower half of the 64 bit bar is programmed. > > This displaced any RAM mappings under 4G. After the > > upper half is programmed PCI memory mapping is restored to its > > intended high mem location, but the RAM displaced is not restored. > > The OS then continues to boot and function until it tries to access > > the displaced RAM at which point it suffers a page fault and > > crashes. > > > > This patch address the issue by deferring enablement of memory and > > I/O decode in command register until all the resources, like > > interrupts > > I/O and/or MMIO BARs for all the PCI device functions are > > programmed, > > in the descending order of memory requested. > > > > Signed-off-by: Harsha Shamsundara Havanur > > Reviewed-by: Jan Beulich > with one further adjustment: > > > @@ -120,6 +121,13 @@ void pci_setup(void) > > */ > > bool allow_memory_relocate = 1; > > > > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_IO != > > +PCI_COMMAND_IO); > > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MEMOR > > Y != > > +PCI_COMMAND_MEMORY); > > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MASTE > > R != > > +PCI_COMMAND_MASTER); > > Indentation here still looks wrong, despite me having given you > the precise form to use in reply to v4. This can be taken care > of while committing (if no other need for a v6 arises), but it > would have been nice if you had done this as indicated. > This is due to my vimrc configuration. I will fix this while comitting to align second line to begin with typeof > Jan
Xen Project {lists,mail,downloads} outage, 22 April
Our listserver and mailserver are going to be updated to current Debian stable on Wednesday the 22nd of April. The outage window is 0600-0900 UTC, but we hope not to use all of it. During this time we expect that mail will be delayed, but hopefully not lost or rejected. The mailing list management UI may be unavailable. We recommend that even if it appears to be available, you should defer your list (un)subscriptions until after the outage window. It is possible that if things don't go well, changes made during the outage window might be rolled back. The Xen release downloads (served from downloads.xenproject.org) will also be unavailable during the upgrade. Mail sent via xenbits will be affected, since xenbits sends outgoing mail via mail.xenproject.org. But access to git repositories (xenbits.xenproject.org), and the osstest CI (test-lab.xenproject.org), should be unaffected. Ian.
RE: [XEN PATCH v5] hvmloader: Enable MMIO and I/O decode, after all resource allocation
> -Original Message-
> From: Xen-devel On Behalf Of Harsha
> Shamsundara Havanur
> Sent: 15 April 2020 13:28
> To: xen-devel@lists.xenproject.org
> Cc: Wei Liu ; Andrew Cooper ; Ian
> Jackson
> ; Jan Beulich ; Harsha
> Shamsundara Havanur
> ; Roger Pau Monné
> Subject: [XEN PATCH v5] hvmloader: Enable MMIO and I/O decode, after all
> resource allocation
>
> It was observed that PCI MMIO and/or IO BARs were programmed with
> memory and I/O decodes (bits 0 and 1 of PCI COMMAND register) enabled,
> during PCI setup phase. This resulted in incorrect memory mapping as
> soon as the lower half of the 64 bit bar is programmed.
> This displaced any RAM mappings under 4G. After the
> upper half is programmed PCI memory mapping is restored to its
> intended high mem location, but the RAM displaced is not restored.
> The OS then continues to boot and function until it tries to access
> the displaced RAM at which point it suffers a page fault and crashes.
>
> This patch address the issue by deferring enablement of memory and
> I/O decode in command register until all the resources, like interrupts
> I/O and/or MMIO BARs for all the PCI device functions are programmed,
> in the descending order of memory requested.
>
> Signed-off-by: Harsha Shamsundara Havanur
>
> ---
> Changes in v5:
> - Fix style and comment
> - Enable Bus master for all valid device functions
>
> Changes in v4:
> Addressed review comments from Jan Beulich
> - Disable decodes before writing ~0 to BARs
> - Enable BUS MASTER at the end along with decode bits
>
> Changes in v3:
> - Retained enabling of BUS master for all device functions as
> was in original commit
>
> Changes in v2:
> - BUS MASTER state was captured and restored to the same state
> while enabling decode bits.
> ---
> ---
> tools/firmware/hvmloader/pci.c | 49
> +++---
> 1 file changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
> index 0b708bf578..47cba69ce4 100644
> --- a/tools/firmware/hvmloader/pci.c
> +++ b/tools/firmware/hvmloader/pci.c
> @@ -84,6 +84,7 @@ void pci_setup(void)
> uint32_t vga_devfn = 256;
> uint16_t class, vendor_id, device_id;
> unsigned int bar, pin, link, isa_irq;
> +uint8_t pci_devfn_decode_type[256] = {};
>
> /* Resources assignable to PCI devices via BARs. */
> struct resource {
> @@ -120,6 +121,13 @@ void pci_setup(void)
> */
> bool allow_memory_relocate = 1;
>
> +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_IO !=
> +PCI_COMMAND_IO);
> +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MEMORY !=
> +PCI_COMMAND_MEMORY);
> +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MASTER !=
> +PCI_COMMAND_MASTER);
> +
These don't align. Looks like you used an indent of 8 which seems entirely
arbitrary.
The rest LGTM.
Paul
Re: preparations for 4.13.1 and 4.12.3
On 09/04/2020 09:41, Jan Beulich wrote: > All, > > the releases are due in a week or two. Please point out backports > you find missing from the respective staging branches, but which > you consider relevant. (Ian, I notice there haven't been any > tools side backports at all so far. Julien, Stefano - same for > Arm.) > > Jan I would like to suggest for 4.13.1: 4b5b431edd984b26f43b3efc7de465f3560a949e "tools/xentop: Fix calculation of used memory" Thanks, -- Sander
Re: [PATCH 12/12] xen/arm: call iomem_permit_access for passthrough devices
On 15/04/2020 02:02, Stefano Stabellini wrote:
iomem_permit_access should be called for MMIO regions of devices
assigned to a domain. Currently it is not called for MMIO regions of
passthrough devices of Dom0less guests. This patch fixes it.
You can already have cases today where the MMIO regions are mapped to
the guest but the guest doesn't have permission on them.
Can you explain why this is a problem here?
Cheers,
Signed-off-by: Stefano Stabellini
---
xen/arch/arm/domain_build.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d0fc497d07..d3d42eef5d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1846,6 +1846,17 @@ static int __init handle_passthrough_prop(struct
kernel_info *kinfo,
return -EINVAL;
}
+res = iomem_permit_access(kinfo->d, paddr_to_pfn(mstart),
+ paddr_to_pfn(PAGE_ALIGN(mstart + size - 1)));
+if ( res )
+{
+printk(XENLOG_ERR "Unable to permit to dom%d access to"
+ " 0x%"PRIx64" - 0x%"PRIx64"\n",
+ kinfo->d->domain_id,
+ mstart & PAGE_MASK, PAGE_ALIGN(mstart + size) - 1);
+return res;
+}
+
res = map_regions_p2mt(kinfo->d,
gaddr_to_gfn(gstart),
PFN_DOWN(size),
--
Julien Grall
Re: [XEN PATCH v5] hvmloader: Enable MMIO and I/O decode, after all resource allocation
On 15.04.2020 14:27, Harsha Shamsundara Havanur wrote: > It was observed that PCI MMIO and/or IO BARs were programmed with > memory and I/O decodes (bits 0 and 1 of PCI COMMAND register) enabled, > during PCI setup phase. This resulted in incorrect memory mapping as > soon as the lower half of the 64 bit bar is programmed. > This displaced any RAM mappings under 4G. After the > upper half is programmed PCI memory mapping is restored to its > intended high mem location, but the RAM displaced is not restored. > The OS then continues to boot and function until it tries to access > the displaced RAM at which point it suffers a page fault and crashes. > > This patch address the issue by deferring enablement of memory and > I/O decode in command register until all the resources, like interrupts > I/O and/or MMIO BARs for all the PCI device functions are programmed, > in the descending order of memory requested. > > Signed-off-by: Harsha Shamsundara Havanur Reviewed-by: Jan Beulich with one further adjustment: > @@ -120,6 +121,13 @@ void pci_setup(void) > */ > bool allow_memory_relocate = 1; > > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_IO != > +PCI_COMMAND_IO); > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MEMORY != > +PCI_COMMAND_MEMORY); > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MASTER != > +PCI_COMMAND_MASTER); Indentation here still looks wrong, despite me having given you the precise form to use in reply to v4. This can be taken care of while committing (if no other need for a v6 arises), but it would have been nice if you had done this as indicated. Jan
Re: [PATCH 11/12] xen/arm: if xen_force don't try to setup the IOMMU
Hi Stefano, On 15/04/2020 02:02, Stefano Stabellini wrote: If xen_force (which means xen,force-assign-without-iommu was requested) don't try to add the device to the IOMMU. Return early instead. Could you explain why this is an issue to call xen_force after iommu_add_dt_device()? Cheers, -- Julien Grall
Re: [PATCH 10/12] xen/arm: if is_domain_direct_mapped use native UART address for vPL011
Hi Stefano, On 15/04/2020 02:02, Stefano Stabellini wrote: We always use a fix address to map the vPL011 to domains. The address could be a problem for domains that are directly mapped. Instead, for domains that are directly mapped, reuse the address of the physical UART on the platform to avoid potential clashes. How do you know the physical UART MMIO region is big enough to fit the PL011? What if the user want to assign the physical UART to the domain + the vpl011? Cheers, -- Julien Grall
Re: [PATCH V1] Fix for Coverity ID: 1461759
From: Xen-devel on behalf of Wei Liu Sent: Wednesday, April 15, 2020 4:59 PM To: Alexandru Stefan ISAILA Cc: xen-devel@lists.xenproject.org ; Roger Pau Monné ; Wei Liu ; Jan Beulich ; Andrew Cooper Subject: Re: [PATCH V1] Fix for Coverity ID: 1461759 On Wed, Apr 15, 2020 at 04:53:13PM +0300, Alexandru Isaila wrote: > Signed-off-by: Alexandru Isaila Thanks, but Roger already posted a fix for this. Sorry about the late reaction to this, I did no check my email. Alex
[PATCH OSSTEST v3 2/2] make-flight: add a core scheduling job
Run a simple core scheduling tests on a host that has SMT support.
This is only enabled for Xen >= 4.13.
The runvar difference is:
+test-amd64-coresched-amd64-xl all_host_di_version 2020-02-10
+test-amd64-coresched-i386-xl all_host_di_version 2020-02-10
+test-amd64-coresched-amd64-xl all_host_suite stretch
+test-amd64-coresched-i386-xl all_host_suite stretch
+test-amd64-coresched-amd64-xl all_hostflags
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,smt
+test-amd64-coresched-i386-xl all_hostflags
arch-i386,arch-xen-amd64,suite-stretch,purpose-test,smt
+test-amd64-coresched-amd64-xl archamd64
+test-amd64-coresched-i386-xl archi386
+test-amd64-coresched-amd64-xl buildjobbuild-amd64
+test-amd64-coresched-i386-xl buildjobbuild-i386
+test-amd64-coresched-amd64-xl debian_arch amd64
+test-amd64-coresched-i386-xl debian_arch i386
+test-amd64-coresched-amd64-xl debian_kernkind pvops
+test-amd64-coresched-i386-xl debian_kernkind pvops
+test-amd64-coresched-amd64-xl debian_suitestretch
+test-amd64-coresched-i386-xl debian_suitestretch
+test-amd64-coresched-amd64-xl kernbuildjobbuild-amd64-pvops
+test-amd64-coresched-i386-xl kernbuildjobbuild-i386-pvops
+test-amd64-coresched-amd64-xl kernkindpvops
+test-amd64-coresched-i386-xl kernkindpvops
+test-amd64-coresched-amd64-xl toolstack xl
+test-amd64-coresched-i386-xl toolstack xl
+test-amd64-coresched-amd64-xl xen_boot_append sched-gran=core
+test-amd64-coresched-i386-xl xen_boot_append sched-gran=core
+test-amd64-coresched-amd64-xl xenbuildjob build-amd64
+test-amd64-coresched-i386-xl xenbuildjob build-amd64
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- Use hw-smt instead of smt as hostflag.
---
make-flight | 20
1 file changed, 20 insertions(+)
diff --git a/make-flight b/make-flight
index 2f445d95..a361bcb1 100755
--- a/make-flight
+++ b/make-flight
@@ -763,6 +763,17 @@ test_matrix_do_one () {
*)test_dom0pvh=n ;;
esac
+ # core scheduling tests for versions >= 4.13 only
+ case "$xenbranch" in
+ xen-3.*-testing) test_coresched=n ;;
+ xen-4.?-testing) test_coresched=n ;;
+ xen-4.10-testing) test_coresched=n ;;
+ xen-4.11-testing) test_coresched=n ;;
+ xen-4.12-testing) test_coresched=n ;;
+ *)test_coresched=y ;;
+ esac
+
+
# xend PV guest test on x86 only
if [ x$test_xend = xy -a \( $dom0arch = "i386" -o $dom0arch = "amd64" \) ];
then
job_create_test test-$xenarch$kern-$dom0arch-pv test-debian xend \
@@ -894,6 +905,15 @@ test_matrix_do_one () {
fi
+ # Core-scheduling tests are x86 only
+ if [ x$test_coresched = xy -a $xenarch = amd64 ]; then
+job_create_test test-$xenarch$kern-coresched-$dom0arch-xl \
+test-debian xl $xenarch $dom0arch $debian_runvars \
+all_hostflags=$most_hostflags,hw-smt \
+xen_boot_append='sched-gran=core'
+
+ fi
+
#do_passthrough_tests
do_pygrub_tests
--
2.26.0
Re: [XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
On Wed, Apr 08, 2020 at 01:50:33PM +0200, Jan Beulich wrote: > On 31.03.2020 12:30, Anthony PERARD wrote: > > # Always build obj-bin files as binary even if they come from C source. > > -$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) > > +# FIXME LTO broken, but we would need a different way to filter -flto out > > +# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) > > While you mention this in the description, I'm still not overly > happy with it getting commented out. What's wrong with making the > construct simply act on c_flags? It doesn't work. I tried $(obj-bin-y): c_flags := $(filter-out -flto,$(c_flags)) but the $@ expansion was empty. I guess we could do: $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) that's probably enough for now. Even if we can't test it properly. -- Anthony PERARD
[PATCH OSSTEST v3 1/2] exanime: test for SMT and add a host flag
Check if hosts have SMT based on the number of threads per core. A
value of threads per core greater than 1 implies SMT support.
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- Use hw-smt.
Changes since v1:
- Fix regex and set SMT if number of threads per core is > 1.
---
sg-run-job | 1 +
ts-examine-cpu | 32
2 files changed, 33 insertions(+)
create mode 100755 ts-examine-cpu
diff --git a/sg-run-job b/sg-run-job
index 97011843..aa7953ac 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
if {$ok} {
run-ts -. = ts-examine-serial-post + host
run-ts . = ts-examine-iommu + host
+ run-ts . = ts-examine-cpu + host
run-ts . = ts-examine-logs-save + host
run-ts . = ts-examine-hostprops-save
}
diff --git a/ts-examine-cpu b/ts-examine-cpu
new file mode 100755
index ..81cf7544
--- /dev/null
+++ b/ts-examine-cpu
@@ -0,0 +1,32 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2020 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see
Re: [PATCH 09/12] xen/arm: if is_domain_direct_mapped use native addresses for GICv3
Hi,
On 15/04/2020 02:02, Stefano Stabellini wrote:
Today we use native addresses to map the GICv3 for Dom0 and fixed
addresses for DomUs.
This patch changes the behavior so that native addresses are used for
any domain that is_domain_direct_mapped. The patch has to introduce one
#ifndef CONFIG_NEW_VGIC because the new vgic doesn't support GICv3.
Please no #ifdef. Instead move the creation of the DT node in vgic.c and
introduce a stub for non-GICv3 platform.
Signed-off-by: Stefano Stabellini
---
xen/arch/arm/domain_build.c | 12 +---
xen/arch/arm/vgic-v3.c | 18 ++
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 303bee60f6..beec0a144c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1697,8 +1697,12 @@ static int __init make_gicv3_domU_node(struct
kernel_info *kinfo)
int res = 0;
__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
__be32 *cells;
+struct domain *d = kinfo->d;
+char buf[38];
-res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE));
+snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
+ d->arch.vgic.dbase);
+res = fdt_begin_node(fdt, buf);
if ( res )
return res;
@@ -1720,9 +1724,11 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
cells = [0];
dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS,
GUEST_ROOT_SIZE_CELLS,
- GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE);
+ d->arch.vgic.dbase, GUEST_GICV3_GICD_SIZE);
+#if defined(CONFIG_GICV3) && !defined(CONFIG_NEW_VGIC)
CONFIG_GICV3 does not exist. Did you mean CONFIG_HAS_GICV3?
dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS,
GUEST_ROOT_SIZE_CELLS,
- GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE);
+ d->arch.vgic.rdist_regions[0].base,
GUEST_GICV3_GICR0_SIZE);
+#endif
res = fdt_property(fdt, "reg", reg, sizeof(reg));
if (res)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 4e60ba15cc..4cf430f865 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1677,13 +1677,25 @@ static int vgic_v3_domain_init(struct domain *d)
I think you also want to modify vgic_v3_max_rdist_count().
* Domain 0 gets the hardware address.
* Guests get the virtual platform layout.
This comment needs to be updated.
*/
-if ( is_hardware_domain(d) )
+if ( is_domain_direct_mapped(d) )
{
unsigned int first_cpu = 0;
+unsigned int nr_rdist_regions;
d->arch.vgic.dbase = vgic_v3_hw.dbase;
-for ( i = 0; i < vgic_v3_hw.nr_rdist_regions; i++ )
+if ( is_hardware_domain(d) )
+{
+nr_rdist_regions = vgic_v3_hw.nr_rdist_regions;
+d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
+}
+else
+{
+nr_rdist_regions = 1;
What does promise your the rdist region will be big enough to cater all
the re-distributors for your domain?
Cheers,
--
Julien Grall
Re: [PATCH v3 4/4] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
On 15.04.2020 13:59, Hongyan Xia wrote:
> @@ -285,26 +286,30 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
> *info)
> continue;
> }
>
> -l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> -if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT))
> +pl2e = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]) +
> +l2_table_offset(va);
> +if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
> {
> i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
> (1UL << (L2_PAGETABLE_SHIFT - 3)) ;
> +UNMAP_DOMAIN_PAGE(pl2e);
> continue;
> }
>
> -pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
> +pt_pfn = l2e_get_pfn(*pl2e);
> if ( hotadd_mem_valid(pt_pfn, info) )
> {
> destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
>
> -l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> -l2e_write(_ro_mpt[l2_table_offset(va)], l2e_empty());
> +l2e_write(pl2e, l2e_empty());
> }
> i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
>(1UL << (L2_PAGETABLE_SHIFT - 3));
> +UNMAP_DOMAIN_PAGE(pl2e);
Along the lines of comments given elsewhere I would have expected
this one to change to the lower case version, as it again sits
right at the and of the scope of the variable.
> }
>
> +UNMAP_DOMAIN_PAGE(l3_ro_mpt);
This, otoh, is still a few lines away from its end-of-scope, and
hence I can see why the variable clearing variant is being used.
Jan
Re: [PATCH v3 1/4] x86/shim: map and unmap page tables in replace_va_mapping
On 15.04.2020 13:59, Hongyan Xia wrote: > From: Wei Liu > > Also, introduce lYe_from_lXe() macros which do not rely on the direct > map when walking page tables. Unfortunately, they cannot be inline > functions due to the header dependency on domain_page.h, so keep them as > macros just like map_lYt_from_lXe(). > > Signed-off-by: Wei Liu > Signed-off-by: Hongyan Xia > > --- > Changed in v3: > - use unmap_domain_page() instead of the macro in several places. > - also introduce l1e_from_l2e(). > - add _ prefix in macros to avoid aliasing. Why prefixes, violating name space rules? In my reply to v2 requesting the adjustment I specifically said "suffixes". Jan
Re: [PATCH 08/12] xen/arm: if is_domain_direct_mapped use native addresses for GICv2
On 15/04/2020 02:02, Stefano Stabellini wrote: Today we use native addresses to map the GICv2 for Dom0 and fixed addresses for DomUs. This patch changes the behavior so that native addresses are used for any domain that is_domain_direct_mapped. Signed-off-by: Stefano Stabellini --- xen/arch/arm/domain_build.c | 10 +++--- xen/arch/arm/vgic-v2.c | 12 ++-- xen/arch/arm/vgic/vgic-v2.c | 2 +- xen/include/asm-arm/vgic.h | 1 + 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 627e0c5e8e..303bee60f6 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1643,8 +1643,12 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo) int res = 0; __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2]; __be32 *cells; +struct domain *d = kinfo->d; +char buf[38]; -res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICD_BASE)); +snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64, + d->arch.vgic.dbase); +res = fdt_begin_node(fdt, buf); if ( res ) return res; @@ -1666,9 +1670,9 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo) cells = [0]; dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, - GUEST_GICD_BASE, GUEST_GICD_SIZE); + d->arch.vgic.dbase, GUEST_GICD_SIZE); dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, - GUEST_GICC_BASE, GUEST_GICC_SIZE); + d->arch.vgic.cbase, GUEST_GICC_SIZE); You can't use the native base address and not the native size. Particularly, this is going to screw any GIC using 8KB alias. It would be preferable if only expose part of the CPU interface as we do for the guest. So d->arch.vgic.cbase would be equal to vgic_v2_hw.dbase + vgic_v2.hw.aliased_offset. Cheers, -- Julien Grall
Re: [PATCH V1] Fix for Coverity ID: 1461759
On Wed, Apr 15, 2020 at 04:53:13PM +0300, Alexandru Isaila wrote: > Signed-off-by: Alexandru Isaila Thanks, but Roger already posted a fix for this. Wei.
[PATCH V1] Fix for Coverity ID: 1461759
Signed-off-by: Alexandru Isaila --- CC: Jan Beulich CC: Andrew Cooper CC: Wei Liu CC: "Roger Pau Monné" --- xen/arch/x86/hvm/hvm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 6f6f3f73a8..45959d3412 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4798,6 +4798,7 @@ static int do_altp2m_op( else rc = p2m_set_altp2m_view_visibility(d, idx, a.u.set_visibility.visible); +break; } default: -- 2.17.1
Re: [PATCH OSSTEST 1/2] exanime: test for SMT and add a host flag
Roger Pau Monne writes ("Re: [PATCH OSSTEST 1/2] exanime: test for SMT and add
a host flag"):
> If OTOH we don't want to be that fine grained I think
> hw-{smt,iommu,vmx,...} would also be fine.
>
> Not sure whether this has helped much. I guess my vote would be for
> cpu-smt namespace.
Let's go with hw-*. That will avoid chopping logic over the precise
nature of hardware features, especially ones which have elements in
multiple components.
Thanks,
Ian.
Re: [PATCH OSSTEST 1/2] exanime: test for SMT and add a host flag
On Wed, Apr 15, 2020 at 02:04:35PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH OSSTEST 1/2] exanime: test for SMT and add a
> host flag"):
> > Check if hosts have SMT based on the number of threads per core. A
> > value of threads per core different than 0 implies SMT support.
> ...
> > +logm("$ho->{Ident} threads per core: $threads");
> > +hostflag_putative_record($ho, "smt", !!$threads);
>
> This code LGTM but I wonder if it would be a good idea to start
> namespacing these kind of hardware feature flags. cpu-*, hardware-*,
> feature-* maybe ? Would you care to make a suggestion ?
cpu-smt seems fine if we plan to do similar namespacing with other
hardware features, I could see cpu-{smt,vmx,svm} and
devices-{iommu,sriov,ats} or some such for example.
If OTOH we don't want to be that fine grained I think
hw-{smt,iommu,vmx,...} would also be fine.
Not sure whether this has helped much. I guess my vote would be for
cpu-smt namespace.
Thanks, Roger.
[xen-4.10-testing test] 149650: regressions - trouble: fail/pass/starved
flight 149650 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149650/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-xtf-amd64-amd64-5 51 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 146101 Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail like 146076 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 146101 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-thunderx 2 hosts-allocate starved n/a version targeted for testing: xen 24d62e126296b3f67dabdd49887d41d8ed69487f baseline version: xen 49a5d6e92317a7d9acbf0bdbd25b2809dfd84260 Last test of basis 146101 2020-01-15 04:00:32 Z 91 days Testing same since 149650 2020-04-14 13:35:36 Z0 days1 attempts People who touched revisions under test: Jan Beulich Julien Grall Pawel Wieczorkiewicz Ross Lagerwall jobs: build-amd64-xsm
Re: [PATCH 07/12] xen/arm: new vgic: rename vgic_cpu/dist_base to c/dbase
Hi,
On 15/04/2020 02:02, Stefano Stabellini wrote:
To be uniform with the old vgic. Name uniformity will become immediately
useful in the following patch.
Please no. Those fields are not meant to be exposed outside of the vGIC.
'cbase' is also much less obvious to read over vgic_cpu_base.
Instead please introduce an helper to retrieve the information you need.
In vgic_v2_map_resources, use the fields in struct vgic_dist rather than
local variables.
Signed-off-by: Stefano Stabellini
---
xen/arch/arm/vgic/vgic-init.c | 4 ++--
xen/arch/arm/vgic/vgic-v2.c| 16
xen/include/asm-arm/new_vgic.h | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
index 62ae553699..ea739d081e 100644
--- a/xen/arch/arm/vgic/vgic-init.c
+++ b/xen/arch/arm/vgic/vgic-init.c
@@ -112,8 +112,8 @@ int domain_vgic_register(struct domain *d, int *mmio_count)
BUG();
}
-d->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
-d->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
+d->arch.vgic.dbase = VGIC_ADDR_UNDEF;
+d->arch.vgic.cbase = VGIC_ADDR_UNDEF;
d->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
return 0;
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
index b5ba4ace87..09cb92039a 100644
--- a/xen/arch/arm/vgic/vgic-v2.c
+++ b/xen/arch/arm/vgic/vgic-v2.c
@@ -258,7 +258,7 @@ void vgic_v2_enable(struct vcpu *vcpu)
int vgic_v2_map_resources(struct domain *d)
{
struct vgic_dist *dist = >arch.vgic;
-paddr_t cbase, csize;
+paddr_t csize;
paddr_t vbase;
int ret;
@@ -268,7 +268,7 @@ int vgic_v2_map_resources(struct domain *d)
*/
if ( is_hardware_domain(d) )
{
-d->arch.vgic.vgic_dist_base = gic_v2_hw_data.dbase;
+d->arch.vgic.dbase = gic_v2_hw_data.dbase;
/*
* For the hardware domain, we always map the whole HW CPU
* interface region in order to match the device tree (the "reg"
@@ -276,13 +276,13 @@ int vgic_v2_map_resources(struct domain *d)
* Note that we assume the size of the CPU interface is always
* aligned to PAGE_SIZE.
*/
-cbase = gic_v2_hw_data.cbase;
+d->arch.vgic.cbase = gic_v2_hw_data.cbase;
csize = gic_v2_hw_data.csize;
vbase = gic_v2_hw_data.vbase;
}
else
{
-d->arch.vgic.vgic_dist_base = GUEST_GICD_BASE;
+d->arch.vgic.dbase = GUEST_GICD_BASE;
/*
* The CPU interface exposed to the guest is always 8kB. We may
* need to add an offset to the virtual CPU interface base
@@ -290,13 +290,13 @@ int vgic_v2_map_resources(struct domain *d)
* region.
*/
BUILD_BUG_ON(GUEST_GICC_SIZE != SZ_8K);
-cbase = GUEST_GICC_BASE;
+d->arch.vgic.cbase = GUEST_GICC_BASE;
csize = GUEST_GICC_SIZE;
vbase = gic_v2_hw_data.vbase + gic_v2_hw_data.aliased_offset;
}
-ret = vgic_register_dist_iodev(d, gaddr_to_gfn(dist->vgic_dist_base),
+ret = vgic_register_dist_iodev(d, gaddr_to_gfn(dist->dbase),
VGIC_V2);
if ( ret )
{
@@ -308,8 +308,8 @@ int vgic_v2_map_resources(struct domain *d)
* Map the gic virtual cpu interface in the gic cpu interface
* region of the guest.
*/
-ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
- maddr_to_mfn(vbase));
+ret = map_mmio_regions(d, gaddr_to_gfn(d->arch.vgic.cbase),
+ csize / PAGE_SIZE, maddr_to_mfn(vbase));
if ( ret )
{
gdprintk(XENLOG_ERR, "Unable to remap VGIC CPU to VCPU\n");
diff --git a/xen/include/asm-arm/new_vgic.h b/xen/include/asm-arm/new_vgic.h
index 97d622bff6..e3319900ab 100644
--- a/xen/include/asm-arm/new_vgic.h
+++ b/xen/include/asm-arm/new_vgic.h
@@ -115,11 +115,11 @@ struct vgic_dist {
unsigned intnr_spis;
/* base addresses in guest physical address space: */
-paddr_t vgic_dist_base; /* distributor */
+paddr_t dbase; /* distributor */
union
{
/* either a GICv2 CPU interface */
-paddr_t vgic_cpu_base;
+paddr_t cbase;
/* or a number of GICv3 redistributor regions */
struct
{
--
Julien Grall
Re: [PATCH 06/12] xen/arm: reserve 1:1 memory for direct_map domUs
On 15/04/2020 02:02, Stefano Stabellini wrote:
Use reserve_domheap_pages to implement the direct-map ranges allocation
for DomUs.
Signed-off-by: Stefano Stabellini
---
xen/arch/arm/domain_build.c | 37 +++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a2bb411568..627e0c5e8e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -198,6 +198,40 @@ fail:
return false;
}
+static void __init reserve_memory_11(struct domain *d,
+ struct kernel_info *kinfo,
+ struct membank *banks,
+ unsigned int nr_banks)
Can we stop introduce more void function and properly return an error?
+{
+unsigned int i, order;
+struct page_info *pg;
+
+kinfo->mem.nr_banks = 0;
+
+for ( i = 0; i < nr_banks; i++ )
+{
+order = get_order_from_bytes(banks[i].size);
+pg = reserve_domheap_pages(d, banks[i].start, order, 0);
+if ( pg == NULL || !insert_11_bank(d, kinfo, pg, order) )
+{
+printk(XENLOG_ERR
+ "%pd: cannot reserve memory start=%#"PRIpaddr"
size=%#"PRIpaddr"\n",
+d, banks[i].start, banks[i].size);
+BUG();
+}
+}
+
+for( i = 0; i < kinfo->mem.nr_banks; i++ )
+{
+printk("BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" (%ldMB)\n",
+i,
+kinfo->mem.bank[i].start,
+kinfo->mem.bank[i].start + kinfo->mem.bank[i].size,
+/* Don't want format this as PRIpaddr (16 digit hex) */
+(unsigned long)(kinfo->mem.bank[i].size >> 20));
+}
+}
+
/*
* This is all pretty horrible.
*
@@ -2477,8 +2511,7 @@ static int __init construct_domU(struct domain *d,
banks[i].start, banks[i].size);
}
-/* reserve_memory_11(d, , [0], i); */
-BUG();
+reserve_memory_11(d, , [0], i);
If you fold this in #3 and re-order the patches then you don't need the
the commented code + BUG().
}
rc = prepare_dtb_domU(d, );
Cheers,
--
Julien Grall
Re: [PATCH 03/12] xen/arm: introduce 1:1 mapping for domUs
On 15/04/2020 02:02, Stefano Stabellini wrote:
In some cases it is desirable to map domU memory 1:1 (guest physical ==
physical.) For instance, because we want to assign a device to the domU
but the IOMMU is not present or cannot be used. In these cases, other
mechanisms should be used for DMA protection, e.g. a MPU.
I am not against this, however the documentation should clearly explain
that you are making your platform insecure unless you have other mean
for DMA protection.
This patch introduces a new device tree option for dom0less guests to
request a domain to be directly mapped. It also specifies the memory
ranges. This patch documents the new attribute and parses it at boot
time. (However, the implementation of 1:1 mapping is missing and just
BUG() out at the moment.) Finally the patch sets the new direct_map
flag for DomU domains.
Signed-off-by: Stefano Stabellini
---
docs/misc/arm/device-tree/booting.txt | 13 +++
docs/misc/arm/passthrough-noiommu.txt | 35 ++
xen/arch/arm/domain_build.c | 52 +--
3 files changed, 98 insertions(+), 2 deletions(-)
create mode 100644 docs/misc/arm/passthrough-noiommu.txt
diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index 5243bc7fd3..fce5f7ed5a 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -159,6 +159,19 @@ with the following properties:
used, or GUEST_VPL011_SPI+1 if vpl011 is enabled, whichever is
greater.
+- direct-map
+
+Optional. An array of integer pairs specifying addresses and sizes.
+direct_map requests the memory of the domain to be 1:1 mapped with
+the memory ranges specified as argument. Only sizes that are a
+power of two number of pages are allowed.
+
+- #direct-map-addr-cells and #direct-map-size-cells
+
+The number of cells to use for the addresses and for the sizes in
+direct-map. Default and maximum are 2 cells for both addresses and
+sizes.
+
As this is going to be mostly used for passthrough, can't we take
advantage of the partial device-tree and describe the memory region
using memory node?
- #address-cells and #size-cells
Both #address-cells and #size-cells need to be specified because
diff --git a/docs/misc/arm/passthrough-noiommu.txt
b/docs/misc/arm/passthrough-noiommu.txt
new file mode 100644
index 00..d2bfaf26c3
--- /dev/null
+++ b/docs/misc/arm/passthrough-noiommu.txt
@@ -0,0 +1,35 @@
+Request Device Assignment without IOMMU support
+===
+
+Add xen,force-assign-without-iommu; to the device tree snippet
+
+ethernet: ethernet@ff0e {
+compatible = "cdns,zynqmp-gem";
+xen,path = "/amba/ethernet@ff0e";
+xen,reg = <0x0 0xff0e 0x1000 0x0 0xff0e>;
+xen,force-assign-without-iommu;
+
+Optionally, if none of the domains require an IOMMU, then it could be
+disabled (not recommended). For instance by adding status = "disabled";
+under the smmu node:
+
+smmu@fd80 {
+compatible = "arm,mmu-500";
+status = "disabled";
I am not sure why this section is added in this patch. Furthermore, the
file is named "noiommu" but here you mention the IOMMU.
+
+
+Request 1:1 memory mapping for the dom0-less domain
+===
+
+Add a direct-map property under the appropriate /chosen/domU node with
+the memory ranges you want to assign to your domain. If you are using
+imagebuilder, you can add to boot.source something like the following:
+
+fdt set /chosen/domU0 direct-map <0x0 0x1000 0x0 0x1000 0x0
0x6000 0x0 0x1000>
+
+Which will assign the ranges:
+
+0x1000 - 0x2000
+0x6000 - 0x7000
+
+to the first dom0less domU.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2ec7453aa3..a2bb411568 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2435,7 +2435,51 @@ static int __init construct_domU(struct domain *d,
/* type must be set before allocate memory */
d->arch.type = kinfo.type;
#endif
-allocate_memory(d, );
+
+if ( !is_domain_direct_mapped(d) )
+allocate_memory(d, );
+else
+{
+struct membank banks[NR_MEM_BANKS];
+const struct dt_property *prop;
+u32 direct_map_len, direct_map_addr_len = 2, direct_map_size_len = 2;
+unsigned int i;
+__be32 *p;
+
+prop = dt_find_property(node, "direct-map", _map_len);
+BUG_ON(!prop);
+
+dt_property_read_u32(node,
+ "#direct-map-addr-cells",
+ _map_addr_len);
+dt_property_read_u32(node,
+ "#direct-map-size-cells",
+ _map_size_len);
+BUG_ON(direct_map_size_len > 2 || direct_map_addr_len > 2);
+
+for (
Re: [PATCH 01/12] xen: introduce xen_dom_flags
On 15/04/2020 10:12, Jan Beulich wrote:
On 15.04.2020 03:02, Stefano Stabellini wrote:
We are passing an extra special boolean flag at domain creation to
specify whether we want to the domain to be privileged (i.e. dom0) or
not. Another flag will be introduced later in this series.
Introduce a new struct xen_dom_flags and move the privileged flag to it.
Other flags will be added to struct xen_dom_flags.
I'm unsure whether introducing a 2nd structure is worth it here.
We could as well define some internal-use-only flags for
struct xen_domctl_createdomain's respective field.
+1.
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -529,7 +529,8 @@ static bool emulation_flags_ok(const struct domain *d,
uint32_t emflags)
}
int arch_domain_create(struct domain *d,
- struct xen_domctl_createdomain *config)
+ struct xen_domctl_createdomain *config,
+ struct xen_dom_flags *flags)
const (also elsewhere)?
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -706,6 +706,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
.max_maptrack_frames = -1,
};
const char *hypervisor_name;
+struct xen_dom_flags flags = { !pv_shim };
Here and elsewhere please use field designators right away, even if
there's only a single field now.
@@ -363,7 +363,7 @@ struct domain *domain_create(domid_t domid,
ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
/* Sort out our idea of is_control_domain(). */
-d->is_privileged = is_priv;
+d->is_privileged = flags ? flags->is_priv : false;
Stray double blanks.
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -364,6 +364,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
bool_t copyback = 0;
struct xen_domctl curop, *op =
struct domain *d;
+struct xen_dom_flags flags ={ false };
Missing blank.
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -63,8 +63,13 @@ void arch_vcpu_destroy(struct vcpu *v);
int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
void unmap_vcpu_info(struct vcpu *v);
+struct xen_dom_flags {
+bool is_priv;
Use a single bit bitfield instead? May even want to consider passing
this struct by value then.
This is an alternative if extending xen_domctl_createdomain is not a
solution. The bitfield is easier to extend because we don't need to
create a new field for each flag in struct domain.
Cheers,
--
Julien Grall
Re: [PATCH 05/12] xen: introduce reserve_heap_pages
On 15/04/2020 02:02, Stefano Stabellini wrote: Introduce a function named reserve_heap_pages (similar to alloc_heap_pages) that allocates a requested memory range. Call __alloc_heap_pages for the implementation. Change __alloc_heap_pages so that the original page doesn't get modified, giving back unneeded memory top to bottom rather than bottom to top. Also introduce a function named reserve_domheap_pages, similar to alloc_domheap_pages, that checks memflags before calling reserve_heap_pages. It also assign_pages to the domain on success. Xen may have already allocated the part of region for its own purpose or for another domain. So this will not work reliably. We have the same issues with LiveUpdate as memory have to be preserved. We need to mark the page reserved before any allocation (including early boot allocation) so nobody can use them. See [1]. Cheers, [1] Live update: boot memory management, data stream handling, record format -- Julien Grall
Re: [PATCH OSSTEST v2 1/2] exanime: test for SMT and add a host flag
Roger Pau Monne writes ("[PATCH OSSTEST v2 1/2] exanime: test for SMT and add a
host flag"):
> Check if hosts have SMT based on the number of threads per core. A
> value of threads per core greater than 1 implies SMT support.
I spotted the "0" in v1 but since you had clearly stated the same
thing in the commit message too I thought you knew what you were doing
:-).
Thanks,
Ian.
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
Roger Pau Monne writes ("[PATCH OSSTEST 2/2] make-flight: add a core scheduling
job"):
> Run a simple core scheduling tests on a host that has SMT support.
> This is only enabled for Xen >= 4.13.
...
> + # Core-scheduling tests are x86 only
> + if [ x$test_coresched = xy -a $xenarch = amd64 ]; then
> +job_create_test test-$xenarch$kern-coresched-$dom0arch-xl \
> +test-debian xl $xenarch $dom0arch $debian_runvars \
> +all_hostflags=$most_hostflags,smt \
> +xen_boot_append='sched-gran=core'
> +
> + fi
This seems fine as far as it goes, but all it does is check that
things still work if sched-gran=core is passed. I'm not sure whether
anything more sophisticated is needed, and in any case this is a step
in the right direction, so:
Acked-by: Ian Jackson
(subject to my comment on 1/ about the flag name.)
Ian.
Re: [PATCH OSSTEST 1/2] exanime: test for SMT and add a host flag
Roger Pau Monne writes ("[PATCH OSSTEST 1/2] exanime: test for SMT and add a
host flag"):
> Check if hosts have SMT based on the number of threads per core. A
> value of threads per core different than 0 implies SMT support.
...
> +logm("$ho->{Ident} threads per core: $threads");
> +hostflag_putative_record($ho, "smt", !!$threads);
This code LGTM but I wonder if it would be a good idea to start
namespacing these kind of hardware feature flags. cpu-*, hardware-*,
feature-* maybe ? Would you care to make a suggestion ?
Ian.
[PATCH v3 3/4] x86_64/mm: map and unmap page tables in share_hotadd_m2p_table
From: Wei Liu
Fetch lYe by mapping and unmapping lXe instead of using the direct map,
which is now done via the lYe_from_lXe() helpers.
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
Reviewed-by: Jan Beulich
---
Changed in v2:
- the introduction of the macros is now lifted to a previous patch.
---
xen/arch/x86/x86_64/mm.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 41755ded26..cfaeae84e9 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -166,14 +166,14 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v += n << PAGE_SHIFT )
{
n = L2_PAGETABLE_ENTRIES * L1_PAGETABLE_ENTRIES;
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
if ( !(l3e_get_flags(l3e) & _PAGE_PSE) )
{
n = L1_PAGETABLE_ENTRIES;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
@@ -194,11 +194,11 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v != RDWR_COMPAT_MPT_VIRT_END;
v += 1 << L2_PAGETABLE_SHIFT )
{
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
--
2.24.1.AMZN
[PATCH v3 2/4] x86_64/mm: map and unmap page tables in m2p_mapped
From: Wei Liu
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
Reviewed-by: Jan Beulich
---
Changed in v3:
- rename l3e_ro_mpt and l2e_ro_mpt, just call them l3e and l2e.
Changed in v2:
- avoid adding goto labels, simply get the PTE and unmap quickly.
- code style fixes.
---
xen/arch/x86/x86_64/mm.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cee836ec37..41755ded26 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -129,14 +129,13 @@ static mfn_t alloc_hotadd_mfn(struct mem_hotadd_info
*info)
static int m2p_mapped(unsigned long spfn)
{
unsigned long va;
-l3_pgentry_t *l3_ro_mpt;
-l2_pgentry_t *l2_ro_mpt;
+l3_pgentry_t l3e;
+l2_pgentry_t l2e;
va = RO_MPT_VIRT_START + spfn * sizeof(*machine_to_phys_mapping);
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(va)],
l3_table_offset(va));
-switch ( l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
- (_PAGE_PRESENT |_PAGE_PSE))
+switch ( l3e_get_flags(l3e) & (_PAGE_PRESENT | _PAGE_PSE) )
{
case _PAGE_PSE|_PAGE_PRESENT:
return M2P_1G_MAPPED;
@@ -146,9 +145,9 @@ static int m2p_mapped(unsigned long spfn)
default:
return M2P_NO_MAPPED;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
+l2e = l2e_from_l3e(l3e, l2_table_offset(va));
-if (l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT)
+if ( l2e_get_flags(l2e) & _PAGE_PRESENT )
return M2P_2M_MAPPED;
return M2P_NO_MAPPED;
--
2.24.1.AMZN
[PATCH v3 0/4] use new API for Xen page tables
From: Hongyan Xia This small series is basically just rewriting functions using the new API to map and unmap PTEs. Each patch is independent. Apart from mapping and unmapping page tables, no other functional change intended. --- Changed in v3: - address all comments in v2. - drop patch 4, since other clean-ups will make it unnecessary. Changed in v2: - I kept UNMAP_DOMAIN_PAGE() for now in v2, but I if people say in some cases it is an overkill and unmap_domain_page() should be used, I am okay with that and can make the change. - code cleanup and style fixes. - unmap as early as possible. Wei Liu (4): x86/shim: map and unmap page tables in replace_va_mapping x86_64/mm: map and unmap page tables in m2p_mapped x86_64/mm: map and unmap page tables in share_hotadd_m2p_table x86_64/mm: map and unmap page tables in destroy_m2p_mapping xen/arch/x86/pv/shim.c | 9 xen/arch/x86/x86_64/mm.c | 44 +- xen/include/asm-x86/page.h | 19 3 files changed, 48 insertions(+), 24 deletions(-) -- 2.24.1.AMZN
[PATCH v3 4/4] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
From: Wei Liu
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
---
Changed in v3:
- rename l2_ro_mpt into pl2e to avoid confusion.
Changed in v2:
- no point in re-mapping l2t because it is exactly the same as
l2_ro_mpt.
- point l2_ro_mpt to the entry instead of doing l2_table_offset() all
the time.
---
xen/arch/x86/x86_64/mm.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cfaeae84e9..30ed4e98dd 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -263,7 +263,8 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
unsigned long i, va, rwva;
unsigned long smap = info->spfn, emap = info->epfn;
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
+l3_ro_mpt = map_l3t_from_l4e(
+idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
/*
* No need to clean m2p structure existing before the hotplug
@@ -271,7 +272,7 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
for (i = smap; i < emap;)
{
unsigned long pt_pfn;
-l2_pgentry_t *l2_ro_mpt;
+l2_pgentry_t *pl2e;
va = RO_MPT_VIRT_START + i * sizeof(*machine_to_phys_mapping);
rwva = RDWR_MPT_VIRT_START + i * sizeof(*machine_to_phys_mapping);
@@ -285,26 +286,30 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
continue;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
-if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT))
+pl2e = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]) +
+l2_table_offset(va);
+if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
{
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3)) ;
+UNMAP_DOMAIN_PAGE(pl2e);
continue;
}
-pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
+pt_pfn = l2e_get_pfn(*pl2e);
if ( hotadd_mem_valid(pt_pfn, info) )
{
destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
-l2e_write(_ro_mpt[l2_table_offset(va)], l2e_empty());
+l2e_write(pl2e, l2e_empty());
}
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3));
+UNMAP_DOMAIN_PAGE(pl2e);
}
+UNMAP_DOMAIN_PAGE(l3_ro_mpt);
+
destroy_compat_m2p_mapping(info);
/* Brute-Force flush all TLB */
--
2.24.1.AMZN
[PATCH v3 1/4] x86/shim: map and unmap page tables in replace_va_mapping
From: Wei Liu
Also, introduce lYe_from_lXe() macros which do not rely on the direct
map when walking page tables. Unfortunately, they cannot be inline
functions due to the header dependency on domain_page.h, so keep them as
macros just like map_lYt_from_lXe().
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
---
Changed in v3:
- use unmap_domain_page() instead of the macro in several places.
- also introduce l1e_from_l2e().
- add _ prefix in macros to avoid aliasing.
Changed in v2:
- instead of map, map, map, read/write, unmap, unmap, unmap, do map,
read PTE, unmap for each level instead.
- use lYe_from_lXe() macros and lift them from a later patch to this
patch.
- const qualify pointers in new macros.
---
xen/arch/x86/pv/shim.c | 9 +
xen/include/asm-x86/page.h | 19 +++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index ed2ece8a8a..31264582cc 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -168,16 +168,17 @@ const struct platform_bad_page *__init
pv_shim_reserved_pages(unsigned int *size
static void __init replace_va_mapping(struct domain *d, l4_pgentry_t *l4start,
unsigned long va, mfn_t mfn)
{
-l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
-l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
-l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
-l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
+l4_pgentry_t l4e = l4start[l4_table_offset(va)];
+l3_pgentry_t l3e = l3e_from_l4e(l4e, l3_table_offset(va));
+l2_pgentry_t l2e = l2e_from_l3e(l3e, l2_table_offset(va));
+l1_pgentry_t *pl1e = map_l1t_from_l2e(l2e) + l1_table_offset(va);
struct page_info *page = mfn_to_page(l1e_get_mfn(*pl1e));
put_page_and_type(page);
*pl1e = l1e_from_mfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
: COMPAT_L1_PROT));
+unmap_domain_page(pl1e);
}
static void evtchn_reserve(struct domain *d, unsigned int port)
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index eb73a0fc23..d50989a357 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -197,6 +197,25 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa,
unsigned int flags)
#define map_l2t_from_l3e(x)(l2_pgentry_t
*)map_domain_page(l3e_get_mfn(x))
#define map_l3t_from_l4e(x)(l3_pgentry_t
*)map_domain_page(l4e_get_mfn(x))
+/* Unlike lYe_to_lXe(), lXe_from_lYe() do not rely on the direct map. */
+#define l1e_from_l2e(_l2e, _offset) ({ \
+const l1_pgentry_t *_l1t = map_l1t_from_l2e(_l2e); \
+l1_pgentry_t _l1e = _l1t[_offset]; \
+unmap_domain_page(_l1t);\
+_l1e; })
+
+#define l2e_from_l3e(_l3e, _offset) ({ \
+const l2_pgentry_t *_l2t = map_l2t_from_l3e(_l3e); \
+l2_pgentry_t _l2e = _l2t[_offset]; \
+unmap_domain_page(_l2t);\
+_l2e; })
+
+#define l3e_from_l4e(_l4e, _offset) ({ \
+const l3_pgentry_t *_l3t = map_l3t_from_l4e(_l4e); \
+l3_pgentry_t _l3e = _l3t[_offset]; \
+unmap_domain_page(_l3t);\
+_l3e; })
+
/* Given a virtual address, get an entry offset into a page table. */
#define l1_table_offset(a) \
(((a) >> L1_PAGETABLE_SHIFT) & (L1_PAGETABLE_ENTRIES - 1))
--
2.24.1.AMZN
[PATCH] x86: retrieve and log CPU frequency information
While from just a single Skylake system it is already clear that we
can't base any of our logic on CPUID leaf 15 [1] (leaf 16 is
documented to be used for display purposes only anyway), logging this
information may still give us some reference in case of problems as well
as for future work. Additionally on the AMD side it is unclear whether
the deviation between reported and measured frequencies is because of us
not doing well, or because of nominal and actual frequencies being quite
far apart.
The chosen variable naming in amd_log_freq() has pointed out a naming
problem in rdmsr_safe(), which is being taken care of at the same time.
Symmetrically wrmsr_safe(), being an inline function, also gets an
unnecessary underscore dropped from one of its local variables.
[1] With a core crystal clock of 24MHz and a ratio of 216/2, the
reported frequency nevertheless is 2600MHz, rather than the to be
expected (and calibrated by both us and Linux) 2592MHz.
Suggested-by: Andrew Cooper
Signed-off-by: Jan Beulich
---
TBD: The node ID retrieval using extended leaf 1E implies it won't work
on older hardware (pre-Fam15 I think). Besides the Node ID MSR,
which doesn't get advertised on my Fam10 box (and it's zero on all
processors despite there being two nodes as per the PCI device
map), and which isn't even documented for Fam11, Fam12, and Fam14,
I didn't find any other means to retrieve the node ID a CPU is
associated with - the NodeId register in PCI config space depends
on one already knowing the node ID for doing the access, as the
device to be used is a function of the node ID.
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -532,6 +532,102 @@ static void amd_get_topology(struct cpui
: c->cpu_core_id);
}
+void amd_log_freq(const struct cpuinfo_x86 *c)
+{
+ unsigned int idx = 0, h;
+ uint64_t hi, lo, val;
+
+ if (c->x86 < 0x10 || c->x86 > 0x19 ||
+ (c != _cpu_data &&
+(!opt_cpu_info || (c->apicid & (c->x86_num_siblings - 1)
+ return;
+
+ if (c->x86 < 0x17) {
+ unsigned int node = 0;
+ uint64_t nbcfg;
+
+ /*
+* Make an attempt at determining the node ID, but assume
+* symmetric setup (using node 0) if this fails.
+*/
+ if (c->extended_cpuid_level >= 0x801e &&
+ cpu_has(c, X86_FEATURE_TOPOEXT)) {
+ node = cpuid_ecx(0x801e) & 0xff;
+ if (node > 7)
+ node = 0;
+ } else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
+ rdmsrl(0xC001100C, val);
+ node = val & 7;
+ }
+
+ /*
+* Enable (and use) Extended Config Space accesses, as we
+* can't be certain that MCFG is available here during boot.
+*/
+ rdmsrl(MSR_AMD64_NB_CFG, nbcfg);
+ wrmsrl(MSR_AMD64_NB_CFG,
+ nbcfg | (1ULL << AMD64_NB_CFG_CF8_EXT_ENABLE_BIT));
+#define PCI_ECS_ADDRESS(sbdf, reg) \
+(0x8000 | ((sbdf).bdf << 8) | ((reg) & 0xfc) | (((reg) & 0xf00) << 16))
+
+ for ( ; ; ) {
+ pci_sbdf_t sbdf = PCI_SBDF(0, 0, 0x18 | node, 4);
+
+ switch (pci_conf_read32(sbdf, PCI_VENDOR_ID)) {
+ case 0x:
+ case 0x:
+ /* No device at this SBDF. */
+ if (!node)
+ break;
+ node = 0;
+ continue;
+
+ default:
+ /*
+* Core Performance Boost Control, family
+* dependent up to 3 bits starting at bit 2.
+*/
+ switch (c->x86) {
+ case 0x10: idx = 1; break;
+ case 0x12: idx = 7; break;
+ case 0x14: idx = 7; break;
+ case 0x15: idx = 7; break;
+ case 0x16: idx = 7; break;
+ }
+ idx &= pci_conf_read(PCI_ECS_ADDRESS(sbdf,
+0x15c),
+0, 4) >> 2;
+ break;
+ }
+ break;
+ }
+
+#undef PCI_ECS_ADDRESS
+ wrmsrl(MSR_AMD64_NB_CFG, nbcfg);
+ }
+
+ lo = 0; /* gcc may not recognize the loop having at least 5 iterations
*/
+ for (h = c->x86 == 0x10 ?
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On 15.04.2020 13:49, Roger Pau Monné wrote: > On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote: >> On 14.04.2020 16:53, Roger Pau Monné wrote: >>> On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote: On 14.04.2020 13:19, Roger Pau Monné wrote: >>> I think this should work, but I would rather do it in a separate >>> patch. >> >> Yes, just like the originally (wrongly, as you validly say) suggested >> full removal of them, putting this in a separate patch would indeed >> seem better. > > Would you like me to resend with the requested fix to > paging_log_dirty_range (ie: drop the FLUSH_TLB and only call > flush_mask for HAP guests running on AMD) then? Well, ideally I'd see that function also make use of the intended new helper function, if at all possible (and suitable). >>> >>> Oh, OK. Just to make sure I understand what you are asking for, you >>> would like me to resend introducing the new guest_flush_tlb_mask >>> helper and use it in the flush_tlb_mask callers modified by this >>> patch? >> >> Yes (and I now realize it may not make sense to split it off into a >> separate change). > > I could do a pre-patch that introduces guest_flush_tlb_mask as a > simple wrapper around flush_tlb_mask and replace the callers that I > modify in this patch. Then this patch would only introduce > FLUSH_HVM_ASID_CORE and modify guest_flush_tlb_mask to use it when > required. > > It might make it more complicated to see which callers require the > ASID flush, but if you think it's better I can arrange the patches in > that way. No, I think it's beteer to leave as a single patch. The idea with splitting was that you'd (fully) take care of some of the sites needing modification ahead of what is now patch 1. I.e. this would have been a suitable approach only if some use sites could really have the call dropped altogether. Jan
Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag
On Tue, Apr 14, 2020 at 05:06:23PM +0200, Jan Beulich wrote: > On 14.04.2020 16:53, Roger Pau Monné wrote: > > On Tue, Apr 14, 2020 at 03:50:15PM +0200, Jan Beulich wrote: > >> On 14.04.2020 13:19, Roger Pau Monné wrote: > > I think this should work, but I would rather do it in a separate > > patch. > > Yes, just like the originally (wrongly, as you validly say) suggested > full removal of them, putting this in a separate patch would indeed > seem better. > >>> > >>> Would you like me to resend with the requested fix to > >>> paging_log_dirty_range (ie: drop the FLUSH_TLB and only call > >>> flush_mask for HAP guests running on AMD) then? > >> > >> Well, ideally I'd see that function also make use of the intended > >> new helper function, if at all possible (and suitable). > > > > Oh, OK. Just to make sure I understand what you are asking for, you > > would like me to resend introducing the new guest_flush_tlb_mask > > helper and use it in the flush_tlb_mask callers modified by this > > patch? > > Yes (and I now realize it may not make sense to split it off into a > separate change). I could do a pre-patch that introduces guest_flush_tlb_mask as a simple wrapper around flush_tlb_mask and replace the callers that I modify in this patch. Then this patch would only introduce FLUSH_HVM_ASID_CORE and modify guest_flush_tlb_mask to use it when required. It might make it more complicated to see which callers require the ASID flush, but if you think it's better I can arrange the patches in that way. Thanks, Roger.
[PATCH AUTOSEL 5.5 053/106] x86/xen: Make the boot CPU idle task reliable
From: Miroslav Benes [ Upstream commit 2f62f36e62daec43aa7b9633ef7f18e042a80bed ] The unwinder reports the boot CPU idle task's stack on XEN PV as unreliable, which affects at least live patching. There are two reasons for this. First, the task does not follow the x86 convention that its stack starts at the offset right below saved pt_regs. It allows the unwinder to easily detect the end of the stack and verify it. Second, startup_xen() function does not store the return address before jumping to xen_start_kernel() which confuses the unwinder. Amend both issues by moving the starting point of initial stack in startup_xen() and storing the return address before the jump, which is exactly what call instruction does. Signed-off-by: Miroslav Benes Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross Signed-off-by: Sasha Levin --- arch/x86/xen/xen-head.S | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S index 1d0cee3163e41..d63806e1ff7ae 100644 --- a/arch/x86/xen/xen-head.S +++ b/arch/x86/xen/xen-head.S @@ -35,7 +35,11 @@ SYM_CODE_START(startup_xen) rep __ASM_SIZE(stos) mov %_ASM_SI, xen_start_info - mov $init_thread_union+THREAD_SIZE, %_ASM_SP +#ifdef CONFIG_X86_64 + mov initial_stack(%rip), %rsp +#else + mov pa(initial_stack), %esp +#endif #ifdef CONFIG_X86_64 /* Set up %gs. @@ -51,7 +55,7 @@ SYM_CODE_START(startup_xen) wrmsr #endif - jmp xen_start_kernel + call xen_start_kernel SYM_CODE_END(startup_xen) __FINIT #endif -- 2.20.1
[PATCH AUTOSEL 5.6 062/129] x86/xen: Make the boot CPU idle task reliable
From: Miroslav Benes [ Upstream commit 2f62f36e62daec43aa7b9633ef7f18e042a80bed ] The unwinder reports the boot CPU idle task's stack on XEN PV as unreliable, which affects at least live patching. There are two reasons for this. First, the task does not follow the x86 convention that its stack starts at the offset right below saved pt_regs. It allows the unwinder to easily detect the end of the stack and verify it. Second, startup_xen() function does not store the return address before jumping to xen_start_kernel() which confuses the unwinder. Amend both issues by moving the starting point of initial stack in startup_xen() and storing the return address before the jump, which is exactly what call instruction does. Signed-off-by: Miroslav Benes Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross Signed-off-by: Sasha Levin --- arch/x86/xen/xen-head.S | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S index 1d0cee3163e41..d63806e1ff7ae 100644 --- a/arch/x86/xen/xen-head.S +++ b/arch/x86/xen/xen-head.S @@ -35,7 +35,11 @@ SYM_CODE_START(startup_xen) rep __ASM_SIZE(stos) mov %_ASM_SI, xen_start_info - mov $init_thread_union+THREAD_SIZE, %_ASM_SP +#ifdef CONFIG_X86_64 + mov initial_stack(%rip), %rsp +#else + mov pa(initial_stack), %esp +#endif #ifdef CONFIG_X86_64 /* Set up %gs. @@ -51,7 +55,7 @@ SYM_CODE_START(startup_xen) wrmsr #endif - jmp xen_start_kernel + call xen_start_kernel SYM_CODE_END(startup_xen) __FINIT #endif -- 2.20.1
Re: [PATCH 02/12] xen/arm: introduce arch_xen_dom_flags and direct_map
On 15/04/2020 11:27, Jan Beulich wrote: > On 15.04.2020 03:02, Stefano Stabellini wrote: >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -682,6 +682,7 @@ int arch_domain_create(struct domain *d, >> return 0; >> >> ASSERT(config != NULL); >> +d->arch.direct_map = flags != NULL ? flags->arch.is_direct_map : false; > Shouldn't "true" here imply ->disable_migrate also getting > set to true? Or is this already the case anyway for domains > created right at boot? Please don't introduce any more uses for disable_migrate. It is fundamentally broken and won't survive to 4.14 (assuming that the 30 odd patches of prereq fixes on xen-devel start unblocking themselves in time) ~Andrew
Re: [PATCH 04/12] xen: split alloc_heap_pages in two halves for reusability
On Tue, Apr 14, 2020 at 06:02:47PM -0700, Stefano Stabellini wrote: > This patch splits the implementation of alloc_heap_pages into two halves > so that the second half can be reused by the next patch. It would be good if you can put a summary on what each half does here so that we can check you intent against the implementation. Wei.
Re: [PATCH 3/3] xenoprof: limit scope of types and #define-s
On Wed, Apr 15, 2020 at 10:53:38AM +0200, Jan Beulich wrote: > Quite a few of the items are used by xenoprof.c only, so move them there > to limit their visibility as well as the amount of re-building needed in > case of changes. Also drop the inclusion of the public header there. > > Signed-off-by: Jan Beulich I haven't read this patch carefully, but what it does is a good thing in general and I trust our build test can catch any fallout from this sort of change, so: Acked-by: Wei Liu
Re: [PATCH 2/3] xenoprof: drop unused struct xenoprof fields
On Wed, Apr 15, 2020 at 10:53:20AM +0200, Jan Beulich wrote: > Both is_primary and domain_ready are only ever written to. Drop both > fields and restrict structure visibility to just the one involved CU. > While doing so (and just for starters) make "is_compat" properly bool. > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu
Re: [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling
On Wed, Apr 15, 2020 at 10:23:31AM +0200, Jan Beulich wrote: > We have a global variable where the necessary L2 table is recorded; no > need to inspect L4 and L3 tables (and this way a few less places will > eventually need adjustment when we want to support 5-level page tables). > Also avoid setting up the L3 entry, as the address range never gets used > anyway (it'll be dropped altogether in a subsequent patch). > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu
Re: [PATCH] x86/altp2m: add missing break
On Wed, Apr 15, 2020 at 01:09:39PM +0200, Roger Pau Monne wrote:
> Add a missing break in the HVMOP_altp2m_set_visibility case, or else
> code flow will continue into the default case and trigger the assert.
>
> Fixes: 3fd3e9303ec4b1 ('x86/altp2m: hypercall to set altp2m view visibility')
> Coverity-ID: 1461759
> Signed-off-by: Roger Pau Monné
Reviewed-by: Wei Liu
[PATCH] x86/altp2m: add missing break
Add a missing break in the HVMOP_altp2m_set_visibility case, or else
code flow will continue into the default case and trigger the assert.
Fixes: 3fd3e9303ec4b1 ('x86/altp2m: hypercall to set altp2m view visibility')
Coverity-ID: 1461759
Signed-off-by: Roger Pau Monné
---
xen/arch/x86/hvm/hvm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 6f6f3f73a8..45959d3412 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4798,6 +4798,7 @@ static int do_altp2m_op(
else
rc = p2m_set_altp2m_view_visibility(d, idx,
a.u.set_visibility.visible);
+break;
}
default:
--
2.26.0
Re: [PATCH V8] x86/altp2m: Hypercall to set altp2m view visibility
On 13.04.2020 08:51, Alexandru Isaila wrote:
> @@ -4786,6 +4787,19 @@ static int do_altp2m_op(
> break;
> }
>
> +case HVMOP_altp2m_set_visibility:
> +{
> +unsigned int idx = a.u.set_visibility.altp2m_idx;
> +
> +if ( a.u.set_visibility.pad )
> +rc = -EINVAL;
> +else if ( !altp2m_active(d) )
> +rc = -EOPNOTSUPP;
> +else
> +rc = p2m_set_altp2m_view_visibility(d, idx,
> +a.u.set_visibility.visible);
> +}
> +
> default:
> ASSERT_UNREACHABLE();
> }
Coverity points out that there's a break statement missing here.
Which makes me wonder how you would have successfully tested this
on a debug build. Please submit a fix (Coverity ID: 1461759).
Jan
Re: [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling
On Wed, 2020-04-15 at 12:34 +0200, Jan Beulich wrote: > On 15.04.2020 11:59, Hongyan Xia wrote: > > ... > > I would like to drop relevant map/unmap patches and replace them > > with > > the new clean-up ones (and place them at the beginning of the > > series), > > if there is no objection with that. > > Depending on turnaround, I'd much rather see this go in before > you re-post. But if you feel like making it part of your series, > go ahead. I actually also very much prefer to see those clean-up patches go in before I post the next revision, so please go ahead. I will post the next revision minus patch 4 then to avoid conflict. Hongyan
Re: [PATCH] docs: update xenstore migration design document
On 15.04.20 12:16, Edwin Torok wrote: On Tue, 2020-04-14 at 17:59 +0200, Juergen Gross wrote: In the past there have been several attempts to make Xenstore restartable. This requires to transfer the internal Xenstore state to the new instance. With the Xenstore migration protocol added recently to Xen's documentation a first base has been defined to represent the state of Xenstore. This can be expanded a little bit in order to have a full state representation which is needed as a first step for live updating Xenstore. Add some definitions to designs/xenstore-migration.md which are needed for live update of xenstored. Signed-off-by: Juergen Gross --- docs/designs/xenstore-migration.md | 90 -- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md index 6ab351e8fe..09bb4700b4 100644 --- a/docs/designs/xenstore-migration.md +++ b/docs/designs/xenstore-migration.md @@ -9,6 +9,10 @@ records must include details of registered xenstore watches as well as content; information that cannot currently be recovered from `xenstored`, and hence some extension to the xenstore protocol[2] will also be required. +As a similar set of data is needed for transferring xenstore data from +one instance to another when live updating xenstored the same definitions +are being used. + The *libxenlight Domain Image Format* specification[3] already defines a record type `EMULATOR_XENSTORE_DATA` but this is not suitable for transferring xenstore data pertaining to the domain directly as it is @@ -48,7 +52,10 @@ where type is one of the following values || 0x0001: NODE_DATA| || 0x0002: WATCH_DATA | || 0x0003: TRANSACTION_DATA | -|| 0x0004 - 0x: reserved for future use | +|| 0x0004: TRANSACTION_NODE_DATA| +|| 0x0005: GUEST_RING_DATA | +|| 0x0006: DOMAIN_START (live update only) | +|| 0x0007 - 0x: reserved for future use | and data is one of the record data formats described in the following @@ -79,7 +86,7 @@ as follows: +---+ | perm count (N)| +---+ -| perm0 | +| perm1 | +---+ ... +---+ @@ -93,7 +100,7 @@ as follows: +---+ ``` -where perm0..N are formatted as follows: +where perm1..N are formatted as follows: ``` @@ -164,6 +171,83 @@ as follows: where tx_id is the non-zero identifier values of an open transaction. +**TRANSACTION_NODE_DATA** + + +Each TRANSACTION_NODE_DATA record specifies a transaction local xenstore +node. Its is similar to the NODE_DATA record with the addition of a +transaction id: + +``` +0 1 2 3 octet ++---+---+---+---+ +| TRANSACTION_NODE_DATA | ++---+ +| tx_id | ++---+ +| path length | ++---+ +| path data | +... +| pad (0 to 3 octets) | ++---+ +| perm count (N)| ++---+ +| perm1 | ++---+ +... ++---+ +| permN | ++---+ +| value length | ++---+ +| value data| +... +| pad (0 to 3 octets) | ++---+ +``` + +where perm1..N are formatted as specified in the NODE_DATA record. A perm +count of 0 denotes a node having been deleted in the transaction. oxenstored also tracks the number of operations that a transaction has performed, which includes readonly operations AFAICT, which cannot be inferred from counting TRANSACTION_NODE_DATA entries. I think the operation count would have to be serialized as part of TRANSACTION_DATA. No, I don't think this is necessary. The read nodes can be included in the TRANSACTION_NODE_DATA entries, too, as long as the transaction is terminated with failure (EAGAIN). In case oxenstored is needing more, e.g. access types, we can include that. The TRANSACTION_NODE_DATA entries are primarily needed to ensure returning consistent data in case of reads of nodes after having them accessed before in the same transaction. + + +**GUEST_RING_DATA** + + +The GUEST_RING_DATA record is used to transfer data which is pending to be +written to the guest's xenstore ring buffer. It si formatted as typo: s/si/is/ Thanks. follows: + + +``` ++---+---+---+---+ +|
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
On 15.04.2020 11:56, George Dunlap wrote: > > >> On Apr 15, 2020, at 10:49 AM, Julien Grall wrote: >> >> >> >> On 15/04/2020 10:43, George Dunlap wrote: On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: On 14.04.2020 18:54, Stefano Stabellini wrote: > On Tue, 14 Apr 2020, Jan Beulich wrote: >> On 10.04.2020 18:49, Stefano Stabellini wrote: > >>> [snip] >>> +Backport: all >>> + >>> +It marks a commit for being a candidate for backports to all relevant >>> +trees. >> >> I'm unconvinced of the utility of this form - what "all" resolves to >> changes over time. There's almost always a first version where a >> particular issue was introduced. If we want this to be generally >> useful, imo we shouldn't limit the scope of the tag to the upstream >> maintained stable trees. > > The reason why I suggested also to have a "wildcard" version of this > tag, is that the person adding the tag (could be the contributor trying > to be helpful) might not know exactly to which stable trees the patch > should be backported to. > > Writing this sentence, I realize that I really meant "any" rather than > "all". Would you prefer if I used "any"? Or we could even suggest to leave > it black like this: > > Backport: > > But it looks a bit weird. Indeed. Instead of "all" or "any", how about "yes", "unspecified", or "unknown"? Nevertheless, I still think people asking for a backport should be nudged towards determining the applicable range; them not doing so effectively pushes the burden to the general maintainers or the stable tree ones, both of which scales less well. Omitting the tag if they don't want to invest the time would to me then seem to be the cleanest alternative. Albeit I'm sure views here will vary. >>> FWIW asking people adding the tag to do the work of figuring out which >>> versions to backport to makes sense to me. >> >> If you ask the contributor to do the work then you need to give guidance on >> the "older" version you can specify in Backport. >> >> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the >> user to specify Backport: 4.2+ or should it be 4.11+? >> >> I would favor the former as this helps for downstream user which haven't yet >> moved to the supported stable tree. > > I agree that specifying the oldest revision possible would be helpful. > > However, I don’t think finding the absolute oldest revision should be > *required* — imagine a bug that was introduced between 3.2 and 3.3. It’s > also perfectly fine if you go all the way back to 4.2 and stop because you > get bored, not because you found out that 4.1 didn’t need it. In which case I'd like there to be a (canonical?) way of expressing this, like in XSAs we say "at least back to" in such a case. Jan
Re: [XEN PATCH v4] hvmloader: Enable MMIO and I/O decode, after all resource allocation
On 15.04.2020 11:32, Shamsundara Havanur, Harsha wrote:
> On Wed, 2020-04-15 at 09:13 +0200, Jan Beulich wrote:
>> On 14.04.2020 19:15, Harsha Shamsundara Havanur wrote:
>>> @@ -526,10 +538,17 @@ void pci_setup(void)
>>> * has IO enabled, even if there is no I/O BAR on that
>>> * particular device.
>>> */
>>> -cmd = pci_readw(vga_devfn, PCI_COMMAND);
>>> -cmd |= PCI_COMMAND_IO;
>>> -pci_writew(vga_devfn, PCI_COMMAND, cmd);
>>> +pci_devfn_decode_type[vga_devfn] |= PCI_COMMAND_IO;
>>> }
>>> +
>>> +/* Enable bus master, memory and I/O decode. */
>>> +for ( devfn = 0; devfn < 256; devfn++ )
>>> +if ( pci_devfn_decode_type[devfn] )
>>> +{
>>> +cmd = pci_readw(devfn, PCI_COMMAND);
>>> +cmd |= (PCI_COMMAND_MASTER |
>>> pci_devfn_decode_type[devfn]);
>>> +pci_writew(devfn, PCI_COMMAND, cmd);
>>> +}
>>
>> This still regresses the setting of MASTER afaict: You only set
>> that bit now if either IO or MEMORY would also get set. But be
>> sure to honor the original code not doing the write when vendor/
>> device IDs are all ones.
>>
> If condition ensures that for devices with vendor/device IDs all ones
> are skipped as it would evaluate to false. But this would also skip
> enabling Bus master for devices whose vendor/device IDs are not all
> ones but IO or memory BARs are not programmed for them. Is there a
> possibility of this happening?
I think so, yes - programming of DMA requests can in principle also
be done via custom config space fields.
Jan
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
On 15.04.2020 11:49, Julien Grall wrote: > > > On 15/04/2020 10:43, George Dunlap wrote: >> >> >>> On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: >>> >>> On 14.04.2020 18:54, Stefano Stabellini wrote: On Tue, 14 Apr 2020, Jan Beulich wrote: > On 10.04.2020 18:49, Stefano Stabellini wrote: >> [snip] >> + Backport: all >> + >> +It marks a commit for being a candidate for backports to all relevant >> +trees. > > I'm unconvinced of the utility of this form - what "all" resolves to > changes over time. There's almost always a first version where a > particular issue was introduced. If we want this to be generally > useful, imo we shouldn't limit the scope of the tag to the upstream > maintained stable trees. The reason why I suggested also to have a "wildcard" version of this tag, is that the person adding the tag (could be the contributor trying to be helpful) might not know exactly to which stable trees the patch should be backported to. Writing this sentence, I realize that I really meant "any" rather than "all". Would you prefer if I used "any"? Or we could even suggest to leave it black like this: Backport: But it looks a bit weird. >>> >>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or >>> "unknown"? Nevertheless, I still think people asking for a backport >>> should be nudged towards determining the applicable range; them not >>> doing so effectively pushes the burden to the general maintainers or >>> the stable tree ones, both of which scales less well. Omitting the >>> tag if they don't want to invest the time would to me then seem to >>> be the cleanest alternative. Albeit I'm sure views here will vary. >> >> FWIW asking people adding the tag to do the work of figuring out which >> versions to backport to makes sense to me. > > If you ask the contributor to do the work then you need to give guidance on > the "older" version you can specify in Backport. > > For instance, let say the bug was introduced in Xen 4.2. Are we allowing the > user to specify Backport: 4.2+ or should it be 4.11+? > > I would favor the former as this helps for downstream user which haven't yet > moved to the supported stable tree. In an earlier reply I did suggest the same, for the same reason. Jan
[xen-unstable-coverity test] 149674: all pass - PUSHED
flight 149674 xen-unstable-coverity real [real] http://logs.test-lab.xenproject.org/osstest/logs/149674/ Perfect :-) All tests in this flight passed as required version targeted for testing: xen fcd06227f83643194f8018f8dd37adce57763a61 baseline version: xen 7372466b21c3b6c96bb7a52754e432bac883a1e3 Last test of basis 149630 2020-04-12 09:19:20 Z3 days Testing same since 149674 2020-04-15 09:19:13 Z0 days1 attempts People who touched revisions under test: Alexandru Isaila Andrew Cooper Jan Beulich Julien Grall Pawel Wieczorkiewicz Ross Lagerwall Wei Liu jobs: coverity-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/xen.git 7372466b21..fcd06227f8 fcd06227f83643194f8018f8dd37adce57763a61 -> coverity-tested/smoke
Re: [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling
On 15.04.2020 11:59, Hongyan Xia wrote: > On Wed, 2020-04-15 at 10:23 +0200, Jan Beulich wrote: >> @@ -627,16 +612,10 @@ void __init paging_init(void) >> #undef MFN >> >> /* Create user-accessible L2 directory to map the MPT for compat >> guests. */ >> -BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) != >> - l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)); >> -l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset( >> -HIRO_COMPAT_MPT_VIRT_START)]); >> if ( (l2_ro_mpt = alloc_xen_pagetable()) == NULL ) >> goto nomem; >> compat_idle_pg_table_l2 = l2_ro_mpt; >> clear_page(l2_ro_mpt); >> -l3e_write(_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START) >> ], >> - l3e_from_paddr(__pa(l2_ro_mpt), >> __PAGE_HYPERVISOR_RO)); >> l2_ro_mpt += l2_table_offset(HIRO_COMPAT_MPT_VIRT_START); >> /* Allocate and map the compatibility mode machine-to-phys >> table. */ >> mpt_size = (mpt_size >> 1) + (1UL << (L2_PAGETABLE_SHIFT - 1)); > > The code around here, I am wondering if there is a reason to put it in > this patch. If we bisect, we can end up in a commit which says the > address range of compat is still there in config.h, but actually it's > gone, so this probably belongs to the 2nd patch. I could be done either way, I guess. Note though how patch 2's description starts with "Now that we don't properly hook things up into the page tables anymore". I.e. after this patch the address range continues to exist solely for calculation purposes. > Apart from that, > Reviewed-by: Hongyan Xia Thanks. > I would like to drop relevant map/unmap patches and replace them with > the new clean-up ones (and place them at the beginning of the series), > if there is no objection with that. Depending on turnaround, I'd much rather see this go in before you re-post. But if you feel like making it part of your series, go ahead. Jan
Re: [PATCH 02/12] xen/arm: introduce arch_xen_dom_flags and direct_map
On 15.04.2020 03:02, Stefano Stabellini wrote:
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -682,6 +682,7 @@ int arch_domain_create(struct domain *d,
> return 0;
>
> ASSERT(config != NULL);
> +d->arch.direct_map = flags != NULL ? flags->arch.is_direct_map : false;
Shouldn't "true" here imply ->disable_migrate also getting
set to true? Or is this already the case anyway for domains
created right at boot?
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2527,6 +2527,7 @@ int __init construct_dom0(struct domain *d)
>
> iommu_hwdom_init(d);
>
> +d->arch.direct_map = true;
Shouldn't this get set via arch_domain_create() instead?
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -418,6 +418,8 @@ struct arch_domain
> uint32_t emulation_flags;
> } __cacheline_aligned;
>
> +struct arch_xen_dom_flags {};
This trivial x86 change
Acked-by: Jan Beulich
Jan
Re: [PATCH] docs: update xenstore migration design document
On Tue, 2020-04-14 at 17:59 +0200, Juergen Gross wrote: > In the past there have been several attempts to make Xenstore > restartable. This requires to transfer the internal Xenstore state to > the new instance. With the Xenstore migration protocol added recently > to Xen's documentation a first base has been defined to represent the > state of Xenstore. This can be expanded a little bit in order to have > a full state representation which is needed as a first step for live > updating Xenstore. > > Add some definitions to designs/xenstore-migration.md which are > needed > for live update of xenstored. > > Signed-off-by: Juergen Gross > --- > docs/designs/xenstore-migration.md | 90 > -- > 1 file changed, 87 insertions(+), 3 deletions(-) > > diff --git a/docs/designs/xenstore-migration.md > b/docs/designs/xenstore-migration.md > index 6ab351e8fe..09bb4700b4 100644 > --- a/docs/designs/xenstore-migration.md > +++ b/docs/designs/xenstore-migration.md > @@ -9,6 +9,10 @@ records must include details of registered xenstore > watches as well as > content; information that cannot currently be recovered from > `xenstored`, > and hence some extension to the xenstore protocol[2] will also be > required. > > +As a similar set of data is needed for transferring xenstore data > from > +one instance to another when live updating xenstored the same > definitions > +are being used. > + > The *libxenlight Domain Image Format* specification[3] already > defines a > record type `EMULATOR_XENSTORE_DATA` but this is not suitable for > transferring xenstore data pertaining to the domain directly as it > is > @@ -48,7 +52,10 @@ where type is one of the following values > || 0x0001: NODE_DATA| > || 0x0002: WATCH_DATA | > || 0x0003: TRANSACTION_DATA | > -|| 0x0004 - 0x: reserved for future use | > +|| 0x0004: TRANSACTION_NODE_DATA| > +|| 0x0005: GUEST_RING_DATA | > +|| 0x0006: DOMAIN_START (live update only) | > +|| 0x0007 - 0x: reserved for future use | > > > and data is one of the record data formats described in the > following > @@ -79,7 +86,7 @@ as follows: > +---+ > | perm count (N)| > +---+ > -| perm0 | > +| perm1 | > +---+ > ... > +---+ > @@ -93,7 +100,7 @@ as follows: > +---+ > ``` > > -where perm0..N are formatted as follows: > +where perm1..N are formatted as follows: > > > ``` > @@ -164,6 +171,83 @@ as follows: > where tx_id is the non-zero identifier values of an open > transaction. > > > +**TRANSACTION_NODE_DATA** > + > + > +Each TRANSACTION_NODE_DATA record specifies a transaction local > xenstore > +node. Its is similar to the NODE_DATA record with the addition of a > +transaction id: > + > +``` > +0 1 2 3 octet > ++---+---+---+---+ > +| TRANSACTION_NODE_DATA | > ++---+ > +| tx_id | > ++---+ > +| path length | > ++---+ > +| path data | > +... > +| pad (0 to 3 octets) | > ++---+ > +| perm count (N)| > ++---+ > +| perm1 | > ++---+ > +... > ++---+ > +| permN | > ++---+ > +| value length | > ++---+ > +| value data| > +... > +| pad (0 to 3 octets) | > ++---+ > +``` > + > +where perm1..N are formatted as specified in the NODE_DATA record. A > perm > +count of 0 denotes a node having been deleted in the transaction. oxenstored also tracks the number of operations that a transaction has performed, which includes readonly operations AFAICT, which cannot be inferred from counting TRANSACTION_NODE_DATA entries. I think the operation count would have to be serialized as part of TRANSACTION_DATA. > + > + > +**GUEST_RING_DATA** > + > + > +The GUEST_RING_DATA record is used to transfer data which is pending > to be > +written to the guest's xenstore ring buffer. It si formatted as typo: s/si/is/ > follows: > + > + > +``` > ++---+---+---+---+ > +| GUEST_RING_DATA | > ++---+ > +| value length | > ++---+ > +| value data| > +... > +| pad (0 to 3 octets) | > ++---+ >
[PATCH OSSTEST v2 1/2] exanime: test for SMT and add a host flag
Check if hosts have SMT based on the number of threads per core. A
value of threads per core greater than 1 implies SMT support.
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Fix regex and set SMT if number of threads per core is > 1.
---
sg-run-job | 1 +
ts-examine-cpu | 32
2 files changed, 33 insertions(+)
create mode 100755 ts-examine-cpu
diff --git a/sg-run-job b/sg-run-job
index 97011843..aa7953ac 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
if {$ok} {
run-ts -. = ts-examine-serial-post + host
run-ts . = ts-examine-iommu + host
+ run-ts . = ts-examine-cpu + host
run-ts . = ts-examine-logs-save + host
run-ts . = ts-examine-hostprops-save
}
diff --git a/ts-examine-cpu b/ts-examine-cpu
new file mode 100755
index ..c30311ab
--- /dev/null
+++ b/ts-examine-cpu
@@ -0,0 +1,32 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2020 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see
Re: [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling
On Wed, 2020-04-15 at 10:23 +0200, Jan Beulich wrote:
> We have a global variable where the necessary L2 table is recorded;
> no
> need to inspect L4 and L3 tables (and this way a few less places will
> eventually need adjustment when we want to support 5-level page
> tables).
> Also avoid setting up the L3 entry, as the address range never gets
> used
> anyway (it'll be dropped altogether in a subsequent patch).
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -219,9 +219,7 @@ static void destroy_compat_m2p_mapping(s
> {
> unsigned long i, va, rwva, pt_pfn;
> unsigned long smap = info->spfn, emap = info->spfn;
> -
> -l3_pgentry_t *l3_ro_mpt;
> -l2_pgentry_t *l2_ro_mpt;
> +l2_pgentry_t *l2_ro_mpt = compat_idle_pg_table_l2;
>
> if ( smap > ((RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2) )
> return;
> @@ -229,12 +227,6 @@ static void destroy_compat_m2p_mapping(s
> if ( emap > ((RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2) )
> emap = (RDWR_COMPAT_MPT_VIRT_END -
> RDWR_COMPAT_MPT_VIRT_START) >> 2;
>
> -l3_ro_mpt =
> l4e_to_l3e(idle_pg_table[l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)]
> );
> -
> -ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_V
> IRT_START)]) & _PAGE_PRESENT);
> -
> -l2_ro_mpt =
> l3e_to_l2e(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
> -
> for ( i = smap; i < emap; )
> {
> va = HIRO_COMPAT_MPT_VIRT_START +
> @@ -323,7 +315,6 @@ static int setup_compat_m2p_table(struct
> unsigned long i, va, smap, emap, rwva, epfn = info->epfn;
> mfn_t mfn;
> unsigned int n;
> -l3_pgentry_t *l3_ro_mpt = NULL;
> l2_pgentry_t *l2_ro_mpt = NULL;
> int err = 0;
>
> @@ -342,13 +333,7 @@ static int setup_compat_m2p_table(struct
> emap = ( (epfn + ((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1 )) &
> ~((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1) );
>
> -va = HIRO_COMPAT_MPT_VIRT_START +
> - smap * sizeof(*compat_machine_to_phys_mapping);
> -l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
> -
> -ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
> _PAGE_PRESENT);
> -
> -l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> +l2_ro_mpt = compat_idle_pg_table_l2;
>
> #define MFN(x) (((x) << L2_PAGETABLE_SHIFT) / sizeof(unsigned int))
> #define CNT ((sizeof(*frame_table) & -sizeof(*frame_table)) / \
> @@ -627,16 +612,10 @@ void __init paging_init(void)
> #undef MFN
>
> /* Create user-accessible L2 directory to map the MPT for compat
> guests. */
> -BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !=
> - l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
> -l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(
> -HIRO_COMPAT_MPT_VIRT_START)]);
> if ( (l2_ro_mpt = alloc_xen_pagetable()) == NULL )
> goto nomem;
> compat_idle_pg_table_l2 = l2_ro_mpt;
> clear_page(l2_ro_mpt);
> -l3e_write(_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)
> ],
> - l3e_from_paddr(__pa(l2_ro_mpt),
> __PAGE_HYPERVISOR_RO));
> l2_ro_mpt += l2_table_offset(HIRO_COMPAT_MPT_VIRT_START);
> /* Allocate and map the compatibility mode machine-to-phys
> table. */
> mpt_size = (mpt_size >> 1) + (1UL << (L2_PAGETABLE_SHIFT - 1));
The code around here, I am wondering if there is a reason to put it in
this patch. If we bisect, we can end up in a commit which says the
address range of compat is still there in config.h, but actually it's
gone, so this probably belongs to the 2nd patch.
Apart from that,
Reviewed-by: Hongyan Xia
I would like to drop relevant map/unmap patches and replace them with
the new clean-up ones (and place them at the beginning of the series),
if there is no objection with that.
Hongyan
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
> On Apr 15, 2020, at 10:49 AM, Julien Grall wrote: > > > > On 15/04/2020 10:43, George Dunlap wrote: >>> On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: >>> >>> On 14.04.2020 18:54, Stefano Stabellini wrote: On Tue, 14 Apr 2020, Jan Beulich wrote: > On 10.04.2020 18:49, Stefano Stabellini wrote: >> [snip] >> +Backport: all >> + >> +It marks a commit for being a candidate for backports to all relevant >> +trees. > > I'm unconvinced of the utility of this form - what "all" resolves to > changes over time. There's almost always a first version where a > particular issue was introduced. If we want this to be generally > useful, imo we shouldn't limit the scope of the tag to the upstream > maintained stable trees. The reason why I suggested also to have a "wildcard" version of this tag, is that the person adding the tag (could be the contributor trying to be helpful) might not know exactly to which stable trees the patch should be backported to. Writing this sentence, I realize that I really meant "any" rather than "all". Would you prefer if I used "any"? Or we could even suggest to leave it black like this: Backport: But it looks a bit weird. >>> >>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or >>> "unknown"? Nevertheless, I still think people asking for a backport >>> should be nudged towards determining the applicable range; them not >>> doing so effectively pushes the burden to the general maintainers or >>> the stable tree ones, both of which scales less well. Omitting the >>> tag if they don't want to invest the time would to me then seem to >>> be the cleanest alternative. Albeit I'm sure views here will vary. >> FWIW asking people adding the tag to do the work of figuring out which >> versions to backport to makes sense to me. > > If you ask the contributor to do the work then you need to give guidance on > the "older" version you can specify in Backport. > > For instance, let say the bug was introduced in Xen 4.2. Are we allowing the > user to specify Backport: 4.2+ or should it be 4.11+? > > I would favor the former as this helps for downstream user which haven't yet > moved to the supported stable tree. I agree that specifying the oldest revision possible would be helpful. However, I don’t think finding the absolute oldest revision should be *required* — imagine a bug that was introduced between 3.2 and 3.3. It’s also perfectly fine if you go all the way back to 4.2 and stop because you get bored, not because you found out that 4.1 didn’t need it. On the other hand, contributors should be expected to find out if it needs to be backported *at least* to fully-supported releases. I think whatever text we come up with should probably say that contributors are “expected” (or “required”) to specify which currently-supported releases need the backport; but “encouraged” to specify a release as far back as possible. -George
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
On 15/04/2020 10:43, George Dunlap wrote: On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: On 14.04.2020 18:54, Stefano Stabellini wrote: On Tue, 14 Apr 2020, Jan Beulich wrote: On 10.04.2020 18:49, Stefano Stabellini wrote: [snip] +Backport: all + +It marks a commit for being a candidate for backports to all relevant +trees. I'm unconvinced of the utility of this form - what "all" resolves to changes over time. There's almost always a first version where a particular issue was introduced. If we want this to be generally useful, imo we shouldn't limit the scope of the tag to the upstream maintained stable trees. The reason why I suggested also to have a "wildcard" version of this tag, is that the person adding the tag (could be the contributor trying to be helpful) might not know exactly to which stable trees the patch should be backported to. Writing this sentence, I realize that I really meant "any" rather than "all". Would you prefer if I used "any"? Or we could even suggest to leave it black like this: Backport: But it looks a bit weird. Indeed. Instead of "all" or "any", how about "yes", "unspecified", or "unknown"? Nevertheless, I still think people asking for a backport should be nudged towards determining the applicable range; them not doing so effectively pushes the burden to the general maintainers or the stable tree ones, both of which scales less well. Omitting the tag if they don't want to invest the time would to me then seem to be the cleanest alternative. Albeit I'm sure views here will vary. FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me. If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport. For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+? I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree. Cheers, -- Julien Grall
Re: [XEN PATCH v4] hvmloader: Enable MMIO and I/O decode, after all resource allocation
On Wed, 2020-04-15 at 09:13 +0200, Jan Beulich wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
>
>
>
> On 14.04.2020 19:15, Harsha Shamsundara Havanur wrote:
> > @@ -120,6 +121,11 @@ void pci_setup(void)
> > */
> > bool allow_memory_relocate = 1;
> >
> > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MEMOR
> > Y
> > +!= PCI_COMMAND_MEMORY);
> > +BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_IO
> > +!= PCI_COMMAND_IO);
>
> This still isn't in line with our default style, you will want eg:
>
> BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_MEMORY
> !=
> PCI_COMMAND_MEMORY);
> BUILD_BUG_ON((typeof(*pci_devfn_decode_type))PCI_COMMAND_IO !=
> PCI_COMMAND_IO);
>
> > @@ -208,6 +214,20 @@ void pci_setup(void)
> > break;
> > }
> >
> > +/*
> > + * Disable memory and I/O decode,
> > + * It is recommended that BAR programming be done whilst
> > + * decode bits are cleared to avoid incorrect mappings
> > being created,
> > + * when 64-bit memory BAR is programmed first by writing
> > the lower half
> > + * and then the upper half, which first maps to an address
> > under 4G
> > + * replacing any RAM mapped in that address, which is not
> > restored
> > + * back after the upper half is written and PCI memory is
> > correctly
> > + * mapped to its intended high mem address.
> > + */
>
> Please can you bring this comment into shape? The comma on the first
> line doesn't fit with the capital letter the second line starts with.
> Plus, if you really mean what is now on the second line to start on a
> new one, then please also insert a line consisting of just * at the
> properly indented position between the two. Additionally there's at
> least one line exceeding the 80-chars-per-line limit.
>
> > @@ -289,10 +309,6 @@ void pci_setup(void)
> > devfn>>3, devfn&7, 'A'+pin-1, isa_irq);
> > }
> >
> > -/* Enable bus mastering. */
> > -cmd = pci_readw(devfn, PCI_COMMAND);
> > -cmd |= PCI_COMMAND_MASTER;
> > -pci_writew(devfn, PCI_COMMAND, cmd);
>
> The movement of this wants mentioning in the description.
>
> > @@ -526,10 +538,17 @@ void pci_setup(void)
> > * has IO enabled, even if there is no I/O BAR on that
> > * particular device.
> > */
> > -cmd = pci_readw(vga_devfn, PCI_COMMAND);
> > -cmd |= PCI_COMMAND_IO;
> > -pci_writew(vga_devfn, PCI_COMMAND, cmd);
> > +pci_devfn_decode_type[vga_devfn] |= PCI_COMMAND_IO;
> > }
> > +
> > +/* Enable bus master, memory and I/O decode. */
> > +for ( devfn = 0; devfn < 256; devfn++ )
> > +if ( pci_devfn_decode_type[devfn] )
> > +{
> > +cmd = pci_readw(devfn, PCI_COMMAND);
> > +cmd |= (PCI_COMMAND_MASTER |
> > pci_devfn_decode_type[devfn]);
> > +pci_writew(devfn, PCI_COMMAND, cmd);
> > +}
>
> This still regresses the setting of MASTER afaict: You only set
> that bit now if either IO or MEMORY would also get set. But be
> sure to honor the original code not doing the write when vendor/
> device IDs are all ones.
>
If condition ensures that for devices with vendor/device IDs all ones
are skipped as it would evaluate to false. But this would also skip
enabling Bus master for devices whose vendor/device IDs are not all
ones but IO or memory BARs are not programmed for them. Is there a
possibility of this happening?
> Jan
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
> On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: > > On 14.04.2020 18:54, Stefano Stabellini wrote: >> On Tue, 14 Apr 2020, Jan Beulich wrote: >>> On 10.04.2020 18:49, Stefano Stabellini wrote: >> [snip] +Backport: all + +It marks a commit for being a candidate for backports to all relevant +trees. >>> >>> I'm unconvinced of the utility of this form - what "all" resolves to >>> changes over time. There's almost always a first version where a >>> particular issue was introduced. If we want this to be generally >>> useful, imo we shouldn't limit the scope of the tag to the upstream >>> maintained stable trees. >> >> The reason why I suggested also to have a "wildcard" version of this >> tag, is that the person adding the tag (could be the contributor trying >> to be helpful) might not know exactly to which stable trees the patch >> should be backported to. >> >> Writing this sentence, I realize that I really meant "any" rather than >> "all". Would you prefer if I used "any"? Or we could even suggest to leave >> it black like this: >> >> Backport: >> >> But it looks a bit weird. > > Indeed. Instead of "all" or "any", how about "yes", "unspecified", or > "unknown"? Nevertheless, I still think people asking for a backport > should be nudged towards determining the applicable range; them not > doing so effectively pushes the burden to the general maintainers or > the stable tree ones, both of which scales less well. Omitting the > tag if they don't want to invest the time would to me then seem to > be the cleanest alternative. Albeit I'm sure views here will vary. FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me. -George
Re: [PATCH 01/12] xen: introduce xen_dom_flags
On 15.04.2020 03:02, Stefano Stabellini wrote:
> We are passing an extra special boolean flag at domain creation to
> specify whether we want to the domain to be privileged (i.e. dom0) or
> not. Another flag will be introduced later in this series.
>
> Introduce a new struct xen_dom_flags and move the privileged flag to it.
> Other flags will be added to struct xen_dom_flags.
I'm unsure whether introducing a 2nd structure is worth it here.
We could as well define some internal-use-only flags for
struct xen_domctl_createdomain's respective field.
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -529,7 +529,8 @@ static bool emulation_flags_ok(const struct domain *d,
> uint32_t emflags)
> }
>
> int arch_domain_create(struct domain *d,
> - struct xen_domctl_createdomain *config)
> + struct xen_domctl_createdomain *config,
> + struct xen_dom_flags *flags)
const (also elsewhere)?
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -706,6 +706,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> .max_maptrack_frames = -1,
> };
> const char *hypervisor_name;
> +struct xen_dom_flags flags = { !pv_shim };
Here and elsewhere please use field designators right away, even if
there's only a single field now.
> @@ -363,7 +363,7 @@ struct domain *domain_create(domid_t domid,
> ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
>
> /* Sort out our idea of is_control_domain(). */
> -d->is_privileged = is_priv;
> +d->is_privileged = flags ? flags->is_priv : false;
Stray double blanks.
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -364,6 +364,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> bool_t copyback = 0;
> struct xen_domctl curop, *op =
> struct domain *d;
> +struct xen_dom_flags flags ={ false };
Missing blank.
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -63,8 +63,13 @@ void arch_vcpu_destroy(struct vcpu *v);
> int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
> void unmap_vcpu_info(struct vcpu *v);
>
> +struct xen_dom_flags {
> +bool is_priv;
Use a single bit bitfield instead? May even want to consider passing
this struct by value then.
Jan
Re: [Xen-devel] [PATCH] sched/core: Fix bug when moving a domain between cpupools
On 27.03.20 20:30, Jeff Kubascik wrote: For each UNIT, sched_set_affinity is called before unit->priv is updated to the new cpupool private UNIT data structure. The issue is sched_set_affinity will call the adjust_affinity method of the cpupool. If defined, the new cpupool may use unit->priv (e.g. credit), which at this point still references the old cpupool private UNIT data structure. This change fixes the bug by moving the switch of unit->priv earler in the function. Signed-off-by: Jeff Kubascik Reviewed-by: Juergen Gross Juergen
Re: xenoprof
On 13.03.2020 18:28, Paul Durrant wrote: > I'm trying to determine the status of HYPERVISOR_xenoprof_op. The code > behind it appears to be unmaintained and I cannot find any > support statement for it. Googling around finds some mentions of Xen and > oprofile but it's not clear whether it works and most > references I find are quite old. Is it time to remove it? Well, in the course of XSA-313 inside the security team we asked ourselves the same, but came to the conclusion that we're better off keeping it, probably. But clarifying its status in SUPPORT.md would likely be a good idea. We may want to go as far as removing security support from it, which then imo ought to be accompanied by changing its Kconfig option to default-off. The effect of doing such on older, in particular no longer fully maintained releases is unclear to me though. Jan
Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
On 15/04/2020 09:50, Paul Durrant wrote: >> -Original Message- >> From: Jan Beulich >> Sent: 15 April 2020 09:45 >> To: xen-devel@lists.xenproject.org >> Cc: Andrew Cooper ; George Dunlap >> ; Ian Jackson >> ; Julien Grall ; Stefano >> Stabellini >> ; Wei Liu ; Paul Durrant >> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up >> >> Patch 1 was considered to become part of the XSA, but it was then >> decided against. The other two are a little bit of cleanup, albeit >> there's certainly far more room for tidying. Yet then again Paul, >> in his mail from Mar 13, was asking whether we shouldn't drop >> xenoprof altogether, at which point cleaning up the code would be >> wasted effort. >> > That's still my opinion. This is a large chunk of (only passively maintained) > code which I think is of very limited value (since it relates to an old tool, > and it only works for PV domains). ... and yet, noone has bothered getting any other profiler in to a half-usable state. You can already Kconfig it out, and yes it is a PITA to use on modern systems where at the minimum, you need to patch the CPU model whitelist, and in some cases extend the MSR whitelist in Xen, but at this point where there are 0 viable alternatives for profiling, removing it would be a deeply short-sighted move. ~Andrew
[PATCH OSSTEST 2/2] make-flight: add a core scheduling job
Run a simple core scheduling tests on a host that has SMT support.
This is only enabled for Xen >= 4.13.
The runvar difference is:
+test-amd64-coresched-amd64-xl all_host_di_version 2020-02-10
+test-amd64-coresched-i386-xl all_host_di_version 2020-02-10
+test-amd64-coresched-amd64-xl all_host_suite stretch
+test-amd64-coresched-i386-xl all_host_suite stretch
+test-amd64-coresched-amd64-xl all_hostflags
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,smt
+test-amd64-coresched-i386-xl all_hostflags
arch-i386,arch-xen-amd64,suite-stretch,purpose-test,smt
+test-amd64-coresched-amd64-xl archamd64
+test-amd64-coresched-i386-xl archi386
+test-amd64-coresched-amd64-xl buildjobbuild-amd64
+test-amd64-coresched-i386-xl buildjobbuild-i386
+test-amd64-coresched-amd64-xl debian_arch amd64
+test-amd64-coresched-i386-xl debian_arch i386
+test-amd64-coresched-amd64-xl debian_kernkind pvops
+test-amd64-coresched-i386-xl debian_kernkind pvops
+test-amd64-coresched-amd64-xl debian_suitestretch
+test-amd64-coresched-i386-xl debian_suitestretch
+test-amd64-coresched-amd64-xl kernbuildjobbuild-amd64-pvops
+test-amd64-coresched-i386-xl kernbuildjobbuild-i386-pvops
+test-amd64-coresched-amd64-xl kernkindpvops
+test-amd64-coresched-i386-xl kernkindpvops
+test-amd64-coresched-amd64-xl toolstack xl
+test-amd64-coresched-i386-xl toolstack xl
+test-amd64-coresched-amd64-xl xen_boot_append sched-gran=core
+test-amd64-coresched-i386-xl xen_boot_append sched-gran=core
+test-amd64-coresched-amd64-xl xenbuildjob build-amd64
+test-amd64-coresched-i386-xl xenbuildjob build-amd64
Signed-off-by: Roger Pau Monné
---
make-flight | 20
1 file changed, 20 insertions(+)
diff --git a/make-flight b/make-flight
index 2f445d95..e656125e 100755
--- a/make-flight
+++ b/make-flight
@@ -763,6 +763,17 @@ test_matrix_do_one () {
*)test_dom0pvh=n ;;
esac
+ # core scheduling tests for versions >= 4.13 only
+ case "$xenbranch" in
+ xen-3.*-testing) test_coresched=n ;;
+ xen-4.?-testing) test_coresched=n ;;
+ xen-4.10-testing) test_coresched=n ;;
+ xen-4.11-testing) test_coresched=n ;;
+ xen-4.12-testing) test_coresched=n ;;
+ *)test_coresched=y ;;
+ esac
+
+
# xend PV guest test on x86 only
if [ x$test_xend = xy -a \( $dom0arch = "i386" -o $dom0arch = "amd64" \) ];
then
job_create_test test-$xenarch$kern-$dom0arch-pv test-debian xend \
@@ -894,6 +905,15 @@ test_matrix_do_one () {
fi
+ # Core-scheduling tests are x86 only
+ if [ x$test_coresched = xy -a $xenarch = amd64 ]; then
+job_create_test test-$xenarch$kern-coresched-$dom0arch-xl \
+test-debian xl $xenarch $dom0arch $debian_runvars \
+all_hostflags=$most_hostflags,smt \
+xen_boot_append='sched-gran=core'
+
+ fi
+
#do_passthrough_tests
do_pygrub_tests
--
2.26.0
[PATCH 2/3] xenoprof: drop unused struct xenoprof fields
Both is_primary and domain_ready are only ever written to. Drop both
fields and restrict structure visibility to just the one involved CU.
While doing so (and just for starters) make "is_compat" properly bool.
Signed-off-by: Jan Beulich
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -33,6 +33,23 @@ static DEFINE_SPINLOCK(pmu_owner_lock);
int pmu_owner = 0;
int pmu_hvm_refcount = 0;
+struct xenoprof_vcpu {
+int event_size;
+xenoprof_buf_t *buffer;
+};
+
+struct xenoprof {
+char *rawbuf;
+int npages;
+int nbuf;
+int bufsize;
+int domain_type;
+#ifdef CONFIG_COMPAT
+bool is_compat;
+#endif
+struct xenoprof_vcpu *vcpu;
+};
+
static struct domain *active_domains[MAX_OPROF_DOMAINS];
static int active_ready[MAX_OPROF_DOMAINS];
static unsigned int adomains;
@@ -259,7 +276,6 @@ static int alloc_xenoprof_struct(
d->xenoprof->npages = npages;
d->xenoprof->nbuf = nvcpu;
d->xenoprof->bufsize = bufsize;
-d->xenoprof->domain_ready = 0;
d->xenoprof->domain_type = XENOPROF_DOMAIN_IGNORED;
/* Update buffer pointers for active vcpus */
@@ -327,7 +343,6 @@ static int set_active(struct domain *d)
if ( x == NULL )
return -EPERM;
-x->domain_ready = 1;
x->domain_type = XENOPROF_DOMAIN_ACTIVE;
active_ready[ind] = 1;
activated++;
@@ -348,7 +363,6 @@ static int reset_active(struct domain *d
if ( x == NULL )
return -EPERM;
-x->domain_ready = 0;
x->domain_type = XENOPROF_DOMAIN_IGNORED;
active_ready[ind] = 0;
active_domains[ind] = NULL;
@@ -655,12 +669,7 @@ static int xenoprof_op_get_buffer(XEN_GU
return ret;
}
else
-{
-d->xenoprof->domain_ready = 0;
d->xenoprof->domain_type = XENOPROF_DOMAIN_IGNORED;
-}
-
-d->xenoprof->is_primary = (xenoprof_primary_profiler == d);
ret = share_xenoprof_page_with_guest(
d, virt_to_mfn(d->xenoprof->rawbuf), d->xenoprof->npages);
--- a/xen/include/xen/xenoprof.h
+++ b/xen/include/xen/xenoprof.h
@@ -40,25 +40,6 @@ typedef union {
} xenoprof_buf_t;
#endif
-struct xenoprof_vcpu {
-int event_size;
-xenoprof_buf_t *buffer;
-};
-
-struct xenoprof {
-char *rawbuf;
-int npages;
-int nbuf;
-int bufsize;
-int domain_type;
-int domain_ready;
-int is_primary;
-#ifdef CONFIG_COMPAT
-int is_compat;
-#endif
-struct xenoprof_vcpu *vcpu;
-};
-
#ifndef CONFIG_COMPAT
#define XENOPROF_COMPAT(x) 0
#define xenoprof_buf(d, b, field) ACCESS_ONCE((b)->field)
[PATCH OSSTEST 1/2] exanime: test for SMT and add a host flag
Check if hosts have SMT based on the number of threads per core. A
value of threads per core different than 0 implies SMT support.
Signed-off-by: Roger Pau Monné
---
sg-run-job | 1 +
ts-examine-cpu | 32
2 files changed, 33 insertions(+)
create mode 100755 ts-examine-cpu
diff --git a/sg-run-job b/sg-run-job
index 97011843..aa7953ac 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
if {$ok} {
run-ts -. = ts-examine-serial-post + host
run-ts . = ts-examine-iommu + host
+ run-ts . = ts-examine-cpu + host
run-ts . = ts-examine-logs-save + host
run-ts . = ts-examine-hostprops-save
}
diff --git a/ts-examine-cpu b/ts-examine-cpu
new file mode 100755
index ..98ffab59
--- /dev/null
+++ b/ts-examine-cpu
@@ -0,0 +1,32 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2020 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see
[PATCH 3/3] xenoprof: limit scope of types and #define-s
Quite a few of the items are used by xenoprof.c only, so move them there
to limit their visibility as well as the amount of re-building needed in
case of changes. Also drop the inclusion of the public header there.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -19,7 +19,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -23,6 +23,32 @@
#undef virt_to_mfn
#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+#define XENOPROF_DOMAIN_IGNORED0
+#define XENOPROF_DOMAIN_ACTIVE 1
+#define XENOPROF_DOMAIN_PASSIVE2
+
+#define XENOPROF_IDLE 0
+#define XENOPROF_INITIALIZED 1
+#define XENOPROF_COUNTERS_RESERVED 2
+#define XENOPROF_READY 3
+#define XENOPROF_PROFILING 4
+
+#ifndef CONFIG_COMPAT
+#define XENOPROF_COMPAT(x) false
+typedef struct xenoprof_buf xenoprof_buf_t;
+#define xenoprof_buf(d, b, field) ACCESS_ONCE((b)->field)
+#else
+#include
+#define XENOPROF_COMPAT(x) ((x)->is_compat)
+typedef union {
+struct xenoprof_buf native;
+struct compat_oprof_buf compat;
+} xenoprof_buf_t;
+#define xenoprof_buf(d, b, field) ACCESS_ONCE(*(!(d)->xenoprof->is_compat \
+? &(b)->native.field \
+: &(b)->compat.field))
+#endif
+
/* Limit amount of pages used for shared buffer (per domain) */
#define MAX_OPROF_SHARED_PAGES 32
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
--- a/xen/include/asm-x86/xenoprof.h
+++ b/xen/include/asm-x86/xenoprof.h
@@ -26,6 +26,8 @@ struct vcpu;
#ifdef CONFIG_XENOPROF
+#include
+
int nmi_reserve_counters(void);
int nmi_setup_events(void);
int nmi_enable_virq(void);
--- a/xen/include/xen/xenoprof.h
+++ b/xen/include/xen/xenoprof.h
@@ -11,7 +11,6 @@
#define __XEN_XENOPROF_H__
#include
-#include
#include
#define PMU_OWNER_NONE 0
@@ -20,37 +19,9 @@
#ifdef CONFIG_XENOPROF
-#define XENOPROF_DOMAIN_IGNORED0
-#define XENOPROF_DOMAIN_ACTIVE 1
-#define XENOPROF_DOMAIN_PASSIVE2
-
-#define XENOPROF_IDLE 0
-#define XENOPROF_INITIALIZED 1
-#define XENOPROF_COUNTERS_RESERVED 2
-#define XENOPROF_READY 3
-#define XENOPROF_PROFILING 4
-
-#ifndef CONFIG_COMPAT
-typedef struct xenoprof_buf xenoprof_buf_t;
-#else
-#include
-typedef union {
- struct xenoprof_buf native;
- struct compat_oprof_buf compat;
-} xenoprof_buf_t;
-#endif
-
-#ifndef CONFIG_COMPAT
-#define XENOPROF_COMPAT(x) 0
-#define xenoprof_buf(d, b, field) ACCESS_ONCE((b)->field)
-#else
-#define XENOPROF_COMPAT(x) ((x)->is_compat)
-#define xenoprof_buf(d, b, field) ACCESS_ONCE(*(!(d)->xenoprof->is_compat \
-? &(b)->native.field \
-: &(b)->compat.field))
-#endif
-
struct domain;
+struct vcpu;
+struct cpu_user_regs;
int acquire_pmu_ownership(int pmu_ownership);
void release_pmu_ownership(int pmu_ownership);
[PATCH 1/3] xenoprof: adjust ordering of page sharing vs domain type setting
Buffer pages should be shared with "ignored" or "active" guests only
(besides, obviously, the primary profiling domain). Hence domain type
should be set to "ignored" before unsharing from the primary domain
(which implies even a previously "passive" domain may then access its
buffers, albeit that's not very useful unless it gets promoted to
"active" subsequently), i.e. such that no further writes of records to
the buffer would occur, and (at least for consistency) also before
sharing it (with the calling domain) from the XENOPROF_get_buffer path.
Signed-off-by: Jan Beulich
Reviewed-by: Wei Liu
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -372,8 +372,8 @@ static void reset_passive(struct domain
if ( x == NULL )
return;
-unshare_xenoprof_page_with_guest(x);
x->domain_type = XENOPROF_DOMAIN_IGNORED;
+unshare_xenoprof_page_with_guest(x);
}
static void reset_active_list(void)
@@ -654,6 +654,13 @@ static int xenoprof_op_get_buffer(XEN_GU
if ( ret < 0 )
return ret;
}
+else
+{
+d->xenoprof->domain_ready = 0;
+d->xenoprof->domain_type = XENOPROF_DOMAIN_IGNORED;
+}
+
+d->xenoprof->is_primary = (xenoprof_primary_profiler == d);
ret = share_xenoprof_page_with_guest(
d, virt_to_mfn(d->xenoprof->rawbuf), d->xenoprof->npages);
@@ -662,10 +669,6 @@ static int xenoprof_op_get_buffer(XEN_GU
xenoprof_reset_buf(d);
-d->xenoprof->domain_type = XENOPROF_DOMAIN_IGNORED;
-d->xenoprof->domain_ready = 0;
-d->xenoprof->is_primary = (xenoprof_primary_profiler == current->domain);
-
xenoprof_get_buffer.nbuf = d->xenoprof->nbuf;
xenoprof_get_buffer.bufsize = d->xenoprof->bufsize;
if ( !paging_mode_translate(d) )
RE: [PATCH 0/3] xenoprof: XSA-313 follow-up
> -Original Message- > From: Jan Beulich > Sent: 15 April 2020 09:45 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper ; George Dunlap > ; Ian Jackson > ; Julien Grall ; Stefano Stabellini > ; Wei Liu ; Paul Durrant > Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up > > Patch 1 was considered to become part of the XSA, but it was then > decided against. The other two are a little bit of cleanup, albeit > there's certainly far more room for tidying. Yet then again Paul, > in his mail from Mar 13, was asking whether we shouldn't drop > xenoprof altogether, at which point cleaning up the code would be > wasted effort. > That's still my opinion. This is a large chunk of (only passively maintained) code which I think is of very limited value (since it relates to an old tool, and it only works for PV domains). Paul
[xen-4.9-testing test] 149649: tolerable trouble: fail/pass/starved - PUSHED
flight 149649 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149649/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-ws16-amd64 14 guest-localmigrate fail REGR. vs. 146210 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-ws16-amd64 18 guest-start/win.repeat fail blocked in 146210 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 146097 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail like 146097 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 146139 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 146210 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 146210 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 146210 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 146210 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 2 hosts-allocate starved n/a version targeted for testing: xen 45c90737d5f0c8bf479adcd8cb88450f1998e55c baseline version: xen cf2e9cc0ba0432f05cdca36dcd46be5fdfd7ca0c Last test of basis 146210 2020-01-18 02:13:02 Z 88 days Testing same since 149649 2020-04-14 13:35:25 Z0 days1 attempts People who touched revisions under test: Jan Beulich Julien Grall Pawel Wieczorkiewicz Ross Lagerwall jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass
[PATCH 0/3] xenoprof: XSA-313 follow-up
Patch 1 was considered to become part of the XSA, but it was then decided against. The other two are a little bit of cleanup, albeit there's certainly far more room for tidying. Yet then again Paul, in his mail from Mar 13, was asking whether we shouldn't drop xenoprof altogether, at which point cleaning up the code would be wasted effort. 1: adjust ordering of page sharing vs domain type setting 2: drop unused struct xenoprof fields 3: limit scope of types and #define-s Jan
Re: [PATCH] docs: update xenstore migration design document
On 15.04.20 10:09, Paul Durrant wrote: -Original Message- From: Xen-devel On Behalf Of Juergen Gross Sent: 14 April 2020 17:00 To: xen-devel@lists.xenproject.org Cc: Juergen Gross ; Stefano Stabellini ; Julien Grall ; Wei Liu ; Andrew Cooper ; Ian Jackson ; George Dunlap ; Jan Beulich Subject: [PATCH] docs: update xenstore migration design document In the past there have been several attempts to make Xenstore restartable. This requires to transfer the internal Xenstore state to the new instance. With the Xenstore migration protocol added recently to Xen's documentation a first base has been defined to represent the state of Xenstore. This can be expanded a little bit in order to have a full state representation which is needed as a first step for live updating Xenstore. Add some definitions to designs/xenstore-migration.md which are needed for live update of xenstored. Signed-off-by: Juergen Gross --- docs/designs/xenstore-migration.md | 90 -- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md index 6ab351e8fe..09bb4700b4 100644 --- a/docs/designs/xenstore-migration.md +++ b/docs/designs/xenstore-migration.md @@ -9,6 +9,10 @@ records must include details of registered xenstore watches as well as content; information that cannot currently be recovered from `xenstored`, and hence some extension to the xenstore protocol[2] will also be required. +As a similar set of data is needed for transferring xenstore data from +one instance to another when live updating xenstored the same definitions +are being used. + That makes sense, but using an external entity to extract the state makes less sense in the context of live update so, going forward, I suggest dropping the section on extra commands. Right, this (if still needed) should rather go to docs/misc/xenstore.txt Also, we should define a separate 'xenstore domain image format' which can be included as an opaque entity in the migration stream, in the same way as the qemu save image. Fine with me. The *libxenlight Domain Image Format* specification[3] already defines a record type `EMULATOR_XENSTORE_DATA` but this is not suitable for transferring xenstore data pertaining to the domain directly as it is @@ -48,7 +52,10 @@ where type is one of the following values || 0x0001: NODE_DATA| || 0x0002: WATCH_DATA | || 0x0003: TRANSACTION_DATA | -|| 0x0004 - 0x: reserved for future use | +|| 0x0004: TRANSACTION_NODE_DATA| +|| 0x0005: GUEST_RING_DATA | +|| 0x0006: DOMAIN_START (live update only) | +|| 0x0007 - 0x: reserved for future use | and data is one of the record data formats described in the following @@ -79,7 +86,7 @@ as follows: +---+ | perm count (N)| +---+ -| perm0 | +| perm1 | +---+ ... +---+ @@ -93,7 +100,7 @@ as follows: +---+ ``` -where perm0..N are formatted as follows: +where perm1..N are formatted as follows: ``` @@ -164,6 +171,83 @@ as follows: where tx_id is the non-zero identifier values of an open transaction. +**TRANSACTION_NODE_DATA** + + +Each TRANSACTION_NODE_DATA record specifies a transaction local xenstore +node. Its is similar to the NODE_DATA record with the addition of a +transaction id: + +``` +0 1 2 3 octet ++---+---+---+---+ +| TRANSACTION_NODE_DATA | ++---+ +| tx_id | ++---+ +| path length | ++---+ +| path data | +... +| pad (0 to 3 octets) | ++---+ +| perm count (N)| ++---+ +| perm1 | ++---+ +... ++---+ +| permN | ++---+ +| value length | ++---+ +| value data| +... +| pad (0 to 3 octets) | ++---+ +``` + +where perm1..N are formatted as specified in the NODE_DATA record. A perm +count of 0 denotes a node having been deleted in the transaction. + Transferring this state means we can presumably drop the TRANSACTION_DATA, since we can infer open transactions from the presence of these records? No, I don't think so. Imagine a case where just a transaction has been started without any further activity in this
Re: [PATCH v2 5/5] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
On 08.04.2020 15:36, Hongyan Xia wrote: > @@ -290,26 +291,30 @@ static void destroy_m2p_mapping(struct mem_hotadd_info > *info) > continue; > } > > -l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]); > -if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT)) > +l2_ro_mpt = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]) + > +l2_table_offset(va); Either the name of the variable should be changed or you shouldn't add in l2_table_offset(va) here. Personally I'd go with renaming to just pl2e. Jan
Re: [PATCH v2 4/5] x86_64/mm: map and unmap page tables in destroy_compat_m2p_mapping
On 08.04.2020 15:36, Hongyan Xia wrote: > From: Wei Liu > > Signed-off-by: Wei Liu > Signed-off-by: Hongyan Xia > > --- > Changed in v2: > - there is pretty much no point in keeping l3_ro_mpt mapped, just fetch > the l3e instead, which also cleans up the code. Even better, there's no need to do any mapping her at all. I've posted a pair of patches, the first of which can be seen as a suggested replacement for the one here. Jan
[PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling
We have a global variable where the necessary L2 table is recorded; no
need to inspect L4 and L3 tables (and this way a few less places will
eventually need adjustment when we want to support 5-level page tables).
Also avoid setting up the L3 entry, as the address range never gets used
anyway (it'll be dropped altogether in a subsequent patch).
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -219,9 +219,7 @@ static void destroy_compat_m2p_mapping(s
{
unsigned long i, va, rwva, pt_pfn;
unsigned long smap = info->spfn, emap = info->spfn;
-
-l3_pgentry_t *l3_ro_mpt;
-l2_pgentry_t *l2_ro_mpt;
+l2_pgentry_t *l2_ro_mpt = compat_idle_pg_table_l2;
if ( smap > ((RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2)
)
return;
@@ -229,12 +227,6 @@ static void destroy_compat_m2p_mapping(s
if ( emap > ((RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2)
)
emap = (RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2;
-l3_ro_mpt =
l4e_to_l3e(idle_pg_table[l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
-
-
ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]) &
_PAGE_PRESENT);
-
-l2_ro_mpt =
l3e_to_l2e(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
-
for ( i = smap; i < emap; )
{
va = HIRO_COMPAT_MPT_VIRT_START +
@@ -323,7 +315,6 @@ static int setup_compat_m2p_table(struct
unsigned long i, va, smap, emap, rwva, epfn = info->epfn;
mfn_t mfn;
unsigned int n;
-l3_pgentry_t *l3_ro_mpt = NULL;
l2_pgentry_t *l2_ro_mpt = NULL;
int err = 0;
@@ -342,13 +333,7 @@ static int setup_compat_m2p_table(struct
emap = ( (epfn + ((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1 )) &
~((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1) );
-va = HIRO_COMPAT_MPT_VIRT_START +
- smap * sizeof(*compat_machine_to_phys_mapping);
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
-
-ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) & _PAGE_PRESENT);
-
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
+l2_ro_mpt = compat_idle_pg_table_l2;
#define MFN(x) (((x) << L2_PAGETABLE_SHIFT) / sizeof(unsigned int))
#define CNT ((sizeof(*frame_table) & -sizeof(*frame_table)) / \
@@ -627,16 +612,10 @@ void __init paging_init(void)
#undef MFN
/* Create user-accessible L2 directory to map the MPT for compat guests. */
-BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !=
- l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(
-HIRO_COMPAT_MPT_VIRT_START)]);
if ( (l2_ro_mpt = alloc_xen_pagetable()) == NULL )
goto nomem;
compat_idle_pg_table_l2 = l2_ro_mpt;
clear_page(l2_ro_mpt);
-l3e_write(_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)],
- l3e_from_paddr(__pa(l2_ro_mpt), __PAGE_HYPERVISOR_RO));
l2_ro_mpt += l2_table_offset(HIRO_COMPAT_MPT_VIRT_START);
/* Allocate and map the compatibility mode machine-to-phys table. */
mpt_size = (mpt_size >> 1) + (1UL << (L2_PAGETABLE_SHIFT - 1));
[PATCH 2/2] x86: drop high compat r/o M2P table address range
Now that we don't properly hook things up into the page tables anymore
we also don't need to set aside an address range. Drop it, using
compat_idle_pg_table_l2[] simply (explicitly) from slot 0.
While doing the re-arrangement, which is accompanied by the dropping or
replacing of some local variables, restrict the scopes of some further
ones at the same time.
Signed-off-by: Jan Beulich
---
TBD: With the changed usage perhaps we want to also rename
compat_idle_pg_table_l2[] (to e.g. compat_idle_l2_entries[])?
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1454,8 +1454,7 @@ static bool pae_xen_mappings_check(const
void init_xen_pae_l2_slots(l2_pgentry_t *l2t, const struct domain *d)
{
memcpy([COMPAT_L2_PAGETABLE_FIRST_XEN_SLOT(d)],
- _idle_pg_table_l2[
- l2_table_offset(HIRO_COMPAT_MPT_VIRT_START)],
+ compat_idle_pg_table_l2,
COMPAT_L2_PAGETABLE_XEN_SLOTS(d) * sizeof(*l2t));
}
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -217,9 +217,7 @@ static int share_hotadd_m2p_table(struct
static void destroy_compat_m2p_mapping(struct mem_hotadd_info *info)
{
-unsigned long i, va, rwva, pt_pfn;
-unsigned long smap = info->spfn, emap = info->spfn;
-l2_pgentry_t *l2_ro_mpt = compat_idle_pg_table_l2;
+unsigned long i, smap = info->spfn, emap = info->spfn;
if ( smap > ((RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2)
)
return;
@@ -229,18 +227,19 @@ static void destroy_compat_m2p_mapping(s
for ( i = smap; i < emap; )
{
-va = HIRO_COMPAT_MPT_VIRT_START +
- i * sizeof(*compat_machine_to_phys_mapping);
-rwva = RDWR_COMPAT_MPT_VIRT_START +
- i * sizeof(*compat_machine_to_phys_mapping);
-if ( l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT )
+unsigned int off = i * sizeof(*compat_machine_to_phys_mapping);
+l2_pgentry_t *pl2e = compat_idle_pg_table_l2 + l2_table_offset(off);
+
+if ( l2e_get_flags(*pl2e) & _PAGE_PRESENT )
{
-pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
+unsigned long pt_pfn = l2e_get_pfn(*pl2e);
+
if ( hotadd_mem_valid(pt_pfn, info) )
{
-destroy_xen_mappings(rwva, rwva +
-(1UL << L2_PAGETABLE_SHIFT));
-l2e_write(_ro_mpt[l2_table_offset(va)], l2e_empty());
+unsigned long rwva = RDWR_COMPAT_MPT_VIRT_START + off;
+
+destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
+l2e_write(pl2e, l2e_empty());
}
}
@@ -312,10 +311,9 @@ static void destroy_m2p_mapping(struct m
*/
static int setup_compat_m2p_table(struct mem_hotadd_info *info)
{
-unsigned long i, va, smap, emap, rwva, epfn = info->epfn;
+unsigned long i, smap, emap, epfn = info->epfn;
mfn_t mfn;
unsigned int n;
-l2_pgentry_t *l2_ro_mpt = NULL;
int err = 0;
smap = info->spfn & (~((1UL << (L2_PAGETABLE_SHIFT - 2)) -1));
@@ -333,8 +331,6 @@ static int setup_compat_m2p_table(struct
emap = ( (epfn + ((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1 )) &
~((1UL << (L2_PAGETABLE_SHIFT - 2)) - 1) );
-l2_ro_mpt = compat_idle_pg_table_l2;
-
#define MFN(x) (((x) << L2_PAGETABLE_SHIFT) / sizeof(unsigned int))
#define CNT ((sizeof(*frame_table) & -sizeof(*frame_table)) / \
sizeof(*compat_machine_to_phys_mapping))
@@ -343,13 +339,11 @@ static int setup_compat_m2p_table(struct
for ( i = smap; i < emap; i += (1UL << (L2_PAGETABLE_SHIFT - 2)) )
{
-va = HIRO_COMPAT_MPT_VIRT_START +
- i * sizeof(*compat_machine_to_phys_mapping);
-
-rwva = RDWR_COMPAT_MPT_VIRT_START +
-i * sizeof(*compat_machine_to_phys_mapping);
+unsigned int off = i * sizeof(*compat_machine_to_phys_mapping);
+l2_pgentry_t *pl2e = compat_idle_pg_table_l2 + l2_table_offset(off);
+unsigned long rwva = RDWR_COMPAT_MPT_VIRT_START + off;
-if (l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT)
+if ( l2e_get_flags(*pl2e) & _PAGE_PRESENT )
continue;
for ( n = 0; n < CNT; ++n)
@@ -366,8 +360,7 @@ static int setup_compat_m2p_table(struct
/* Fill with INVALID_M2P_ENTRY. */
memset((void *)rwva, 0xFF, 1UL << L2_PAGETABLE_SHIFT);
/* NB. Cannot be GLOBAL as the ptes get copied into per-VM space. */
-l2e_write(_ro_mpt[l2_table_offset(va)],
- l2e_from_mfn(mfn, _PAGE_PSE|_PAGE_PRESENT));
+l2e_write(pl2e, l2e_from_mfn(mfn, _PAGE_PSE|_PAGE_PRESENT));
}
#undef CNT
#undef MFN
@@ -616,7 +609,6 @@ void __init paging_init(void)
goto nomem;
compat_idle_pg_table_l2 = l2_ro_mpt;
clear_page(l2_ro_mpt);
-l2_ro_mpt += l2_table_offset(HIRO_COMPAT_MPT_VIRT_START);
/* Allocate and map the compatibility mode
