Re: [Xen-devel] [PATCH v2] tboot: Avoid recursive fault in early boot panic with tboot
> On Aug 28, 2018, at 12:44 AM, Jason Andryuk wrote: > >> On Thu, Jul 19, 2018 at 6:39 AM Jason Andryuk wrote: >> >> If panic is called before init_idle_domain on a tboot-launched system, >> then Xen recursively faults in write_ptbase as seen below. >> >> (XEN)[] write_ptbase+0/0x10 >> (XEN)[] tboot_shutdown+0x6b/0x260 >> (XEN)[] machine_restart+0xac/0x2d0 >> (XEN)[] write_ptbase+0/0x10 >> (XEN)[] panic+0x111/0x120 >> (XEN)[] do_general_protection+0x171/0x1f0 >> (XEN)[] mm.c#virt_to_xen_l2e+0x12/0x1c0 >> (XEN)[] x86_64/entry.S#handle_exception_saved+0x66/0xa4 >> (XEN)[] write_ptbase+0/0x10 >> (XEN)[] tboot_shutdown+0x6b/0x260 >> (XEN)[] machine_restart+0xac/0x2d0 >> (XEN)[] panic+0x111/0x120 >> (XEN)[] setup.c#bootstrap_map+0/0x11a >> (XEN)[] flask_op.c#parse_flask_param+0/0xb0 >> (XEN)[] setup.c#bootstrap_map+0/0x11a >> (XEN)[] xsm_multiboot_init+0x7c/0xb0 >> (XEN)[] __start_xen+0x1d2b/0x2da0 >> (XEN)[] __high_start+0x53/0x60 >> >> idle_vcpu[0] is still poisoned with INVALID_VCPU, so write_ptbase faults >> dereferencing the pointer. This fault calls panic and recurses through >> the same code path. >> >> If tboot_shutdown is called while idle_vcpu[0] == INVALID_VCPU, then we >> are still operating with the initial page tables. Therefore changing >> page tables with write_ptbase is unnecessary. >> >> An easy way to reproduce this is to use tboot to launch an XSM-enabled >> Xen without an XSM policy. >> >> Signed-off-by: Jason Andryuk > > Ping, Shane and Gang? Acked-by: Gang Wei > >> --- >> v2: Correct comment and brace style >> --- >> xen/arch/x86/tboot.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c >> index fb4616ae83..d5a5292d7e 100644 >> --- a/xen/arch/x86/tboot.c >> +++ b/xen/arch/x86/tboot.c >> @@ -391,7 +391,12 @@ void tboot_shutdown(uint32_t shutdown_type) >> tboot_gen_xenheap_integrity(g_tboot_shared->s3_key, _mac); >> } >> >> -write_ptbase(idle_vcpu[0]); >> +/* >> + * During early boot, we can be called by panic before idle_vcpu[0] is >> + * setup, but in that case we don't need to change page tables. >> + */ >> +if ( idle_vcpu[0] != INVALID_VCPU ) >> +write_ptbase(idle_vcpu[0]); >> >> ((void(*)(void))(unsigned long)g_tboot_shared->shutdown_entry)(); >> >> -- >> 2.17.1 >> ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] build: remove tboot make targets
在 2018年8月28日，上午12:24，Doug Goldstein 写道： > > On Mon, Aug 20, 2018 at 02:33:10AM -0600, Jan Beulich wrote: > On 19.08.18 at 04:22, wrote: >>> The tboot targets are woefully out of date. These should really be >>> retired because setting up tboot is more complex than the build process >>> for it. >>> >>> Signed-off-by: Doug Goldstein >> >> Acked-by: Jan Beulich >> >> But I think you would better have Cc-ed the TXT/TBOOT maintainers >> (now done), as it was presumably them having introduced these >> targets. Even if the chance if high to not see any response, we >> should at least give them a chance to chime in. > > Just a one week ping. Acked-by: Gang Wei I agree that referring to the document to do the tboot deployment for Xen will be better. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 126730: regressions - FAIL
flight 126730 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/126730/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 123814 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 123814 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 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-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a version targeted for testing: libvirt 3b89e1f9621895d5789b024caf93fd3b17654610 baseline version: libvirt 076a2b409667dd9f716a2a2085e1ffea9d58fe8b Last test of basis 123814 2018-06-05 04:19:23 Z 83 days Failing since123840 2018-06-06 04:19:28 Z 82 days 63 attempts Testing same since 126623 2018-08-25 14:39:40 Z2 days2 attempts People who touched revisions under test: Ales Musil Andrea Bolognani Anya Harter Bing Niu Bjoern Walk Bobo Du Boris Fiuczynski Brijesh Singh Changkuo Shi Chen Hanxiao Christian Ehrhardt Clementine Hayat Cole Robinson Dan Kenigsberg Daniel Nicoletti Daniel P. Berrangé Daniel Veillard Eric Blake Erik Skultety Fabiano Fidêncio Filip Alac Han Han intrigeri intrigeri Jamie Strandboge Jie Wang Jim Fehlig Jiri Denemark John Ferlan Julio Faracco Ján Tomko Kashyap Chamarthy Katerina Koukiou Laine Stump Laszlo Ersek Lubomir Rintel Luyao Huang Marc Hartmayer Marc Hartmayer Marcos Paulo de Souza Marek Marczykowski-Górecki Martin Kletzander Matthias Bolte Michal Privoznik Michal Prívozník Nikolay Shirokovskiy Pavel Hrdina Peter Krempa Pino Toscano Radostin Stoyanov Ramy Elkest ramyelkest Richard W.M. Jones Roman Bogorodskiy Shi Lei Shichangkuo Shivaprasad G Bhat Simon Kobyda Stefan Bader Stefan Berger Sukrit Bhatnagar Tomáš Golembiovský Vitaly Kuznetsov w00251574 Weilun Zhu xinhua.Cao jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-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-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairblocked test-amd64-i386-libvirt-pair blocked test-armhf-armhf-libvirt-raw blocked test-amd64-amd64-libvirt-vhd blocked 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
[Xen-devel] [ovmf baseline-only test] 75130: tolerable FAIL
This run is configured for baseline tests only. flight 75130 ovmf real [real] http://osstest.xensource.com/osstest/logs/75130/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail like 75123 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail like 75123 version targeted for testing: ovmf 983f5abb9a0d6cf9cfb5e16d671f15e5dc7510d8 baseline version: ovmf 6861765935d5b69803321ba6e43240845c7ab0e5 Last test of basis75123 2018-08-25 19:21:23 Z2 days Testing same since75130 2018-08-27 12:55:15 Z0 days1 attempts People who touched revisions under test: Ruiyu Ni 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 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. commit 983f5abb9a0d6cf9cfb5e16d671f15e5dc7510d8 Author: Ruiyu Ni Date: Thu Aug 23 10:50:16 2018 +0800 MdeModulePkg/PciBus: Restrict one VGA per HostBridge not Segment REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1109 Today's restriction of VGA device is to have only one VGA device enabled per PCI segment. It's not correct because several segments may share one IO / MMIO address space. We should restrict to have one VGA per Host Bridge because each Host Bridge has its only IO / MMIO address space. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Reviewed-by: Star Zeng commit 06da1e310bcea971073a8dabc5c3d35bc190847c Author: Ruiyu Ni Date: Thu Aug 23 10:34:33 2018 +0800 MdeModulePkg/PciBus: Refine ActiveVGADeviceOnTheRootBridge REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1109 The patch doesn't change any behavior of this function. It just renames the function to LocateVgaDevice() and renames some parameters and local variables. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Reviewed-by: Star Zeng ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 test] 126711: regressions - FAIL
flight 126711 linux-3.18 real [real] http://logs.test-lab.xenproject.org/osstest/logs/126711/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-rumprun-i386 12 guest-start fail REGR. vs. 126042 test-amd64-i386-xl-xsm 12 guest-start fail REGR. vs. 126042 test-amd64-i386-qemut-rhel6hvm-intel 10 redhat-install fail REGR. vs. 126042 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 126042 test-amd64-amd64-rumprun-amd64 12 guest-startfail REGR. vs. 126042 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 126042 test-amd64-amd64-xl-shadow 12 guest-start fail REGR. vs. 126042 test-amd64-i386-xl-shadow12 guest-start fail REGR. vs. 126042 test-amd64-amd64-xl-credit2 12 guest-start fail REGR. vs. 126042 test-amd64-amd64-xl-multivcpu 12 guest-start fail REGR. vs. 126042 test-amd64-i386-xl 12 guest-start fail REGR. vs. 126042 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 126042 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 126042 test-amd64-amd64-pair21 guest-start/debian fail REGR. vs. 126042 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 126042 test-amd64-amd64-xl-pvshim 12 guest-start fail REGR. vs. 126042 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 126042 test-amd64-i386-pair 21 guest-start/debian fail REGR. vs. 126042 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 126042 test-amd64-amd64-xl 12 guest-start fail REGR. vs. 126042 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail REGR. vs. 126042 test-amd64-amd64-xl-xsm 12 guest-start fail REGR. vs. 126042 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 126042 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 126042 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 126042 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 126042 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-pygrub 10 debian-di-installfail REGR. vs. 126042 test-amd64-amd64-amd64-pvgrub 10 debian-di-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-i386-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-armhf-armhf-xl-arndale 12 guest-start fail REGR. vs. 126042 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 126042 test-amd64-amd64-i386-pvgrub 10 debian-di-installfail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 126042 test-amd64-i386-xl-raw 10 debian-di-installfail REGR. vs. 126042 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 126042 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 126042 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 126042 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 126042 test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install fail REGR. vs. 126042 test-amd64-i386-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 126042 test-armhf-armhf-xl-credit2 12 guest-start fail REGR. vs. 126042 test-armhf-armhf-xl-multivcpu 12 guest-start fail REGR. vs. 126042 test-armhf-armhf-libvirt 12 guest-start
[Xen-devel] [xen-4.7-testing test] 126716: regressions - FAIL
flight 126716 xen-4.7-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/126716/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 125057 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 125057 Tests which are failing intermittently (not blocking): test-xtf-amd64-amd64-3 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 126514 pass in 126716 test-armhf-armhf-xl-credit2 12 guest-start fail in 126514 pass in 126716 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 126514 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 16 guest-localmigrate/x10 fail pass in 126604 Tests which did not succeed, but are not blocking: test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 126514 like 125057 test-xtf-amd64-amd64-2 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 126514 like 125057 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 126514 like 125057 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125057 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125057 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125057 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125057 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125057 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 125057 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 125057 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125057 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125057 test-amd64-amd64-xl-rtds 10 debian-install fail like 125057 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125057 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-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 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-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-amd64-amd64-libvirt-vhd 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-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 7ba1c7df881855422f9a475862565e94c8421b75 baseline version: xen 280a5568939c4a5832be787c8e0c23a19f30935a Last test of basis 125057 2018-07-09 08:36:04 Z 49 days Failing since125685 2018-07-30 12:39:38 Z 28 days 19 attempts Testing same since 125931 2018-08-15 22:51:23 Z 11 days8 attempts People who touched revisions under test: Andrew Cooper Christian Lindig George Dunlap Jan Beulich Juergen Gross Julien Grall Kevin Tian Stefano Stabellini Wei Liu jobs: build-amd64-xsm pass build-i386-xsm
[Xen-devel] [qemu-mainline baseline-only test] 75128: tolerable FAIL
This run is configured for baseline tests only. flight 75128 qemu-mainline real [real] http://osstest.xensource.com/osstest/logs/75128/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-i386-pvgrub 19 guest-start/debian.repeat fail blocked in 75116 test-amd64-i386-xl-raw 19 guest-start/debian.repeat fail blocked in 75116 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail like 75116 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail like 75116 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail like 75116 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail like 75116 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail like 75116 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail like 75116 test-amd64-amd64-xl-pvshim 12 guest-start fail like 75116 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail like 75116 test-armhf-armhf-libvirt 12 guest-start fail like 75116 test-armhf-armhf-xl-credit2 12 guest-start fail like 75116 test-armhf-armhf-xl 12 guest-start fail like 75116 test-armhf-armhf-xl-rtds 12 guest-start fail like 75116 test-armhf-armhf-xl-midway 12 guest-start fail like 75116 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail like 75116 test-armhf-armhf-xl-multivcpu 12 guest-start fail like 75116 test-armhf-armhf-xl-vhd 10 debian-di-installfail like 75116 test-armhf-armhf-libvirt-raw 10 debian-di-installfail like 75116 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 75116 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 75116 test-amd64-i386-xl-qemuu-win10-i386 17 guest-stop fail like 75116 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-xsm 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-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-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 version targeted for testing: qemuu235c82acca0491465e94be3cae2583b42d37c859 baseline version: qemuu13b7b188501d419a7d63c016e00065bcc693b7d4 Last test of basis75116 2018-08-24 09:51:51 Z3 days Testing same since75128 2018-08-27 05:24:50 Z0 days1 attempts People who touched revisions under test: "Aleksandar Markovic " Aleksandar Markovic Aleksandar Rikalo Alex Williamson Andrew Morton Andrew Oates Aviad Yehezkel Bandan Das chaiwen Christian Borntraeger Christian Ehrhardt Corey Minyard Cornelia Huck David Gibson David Hildenbrand Dimitrije Nikolic Dr. David Alan Gilbert Eduardo Habkost Eduardo Otubo Emilio G. Cota Fam Zheng Gal Shachaf George Kennedy Gerd Hoffmann Greg Edwards Greg Kurz Guenter Roeck Heinrich Schuchardt James Hogan Jeff Cody jialina01 Joe Perches Juan Quintela Julia Suvorova Laurent Vivier Li Qiang Lidong Chen Lidong Chen Linus Torvalds Marc-André Lureau Marc-André Lureau Markus Armbruster Matthew Fortune Murilo Opsfelder Araujo npes87184 Paolo Bonzini Paul Burton Peter Maydell Prasad Singamsetty Richard Henderson ryang Sebastian Bauer Stefan Markovic Thomas Huth Xiao Guangrong Yongbok Kim jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops
Re: [Xen-devel] [Qemu-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
Hi
On Mon, Aug 27, 2018 at 10:10 AM Markus Armbruster wrote:
>
> Marc-André Lureau writes:
>
> > Hi
> > On Fri, Aug 24, 2018 at 9:37 AM Markus Armbruster wrote:
> >>
> >> Marc-André Lureau writes:
> >>
> >> > This is mostly for readability of the code. Let's make it clear which
> >> > callers can create an implicit monitor when the chardev is muxed.
> >> >
> >> > This will also enforce a safer behaviour, as we don't really support
> >> > creating monitor anywhere/anytime at the moment.
> >> >
> >> > There are documented cases, such as: -serial/-parallel/-virtioconsole
> >> > and to less extent -debugcon.
> >> >
> >> > Less obvious and questionnable ones are -gdb and Xen console. Add a
> >> > FIXME note for those, but keep the support for now.
> >> >
> >> > Other qemu_chr_new() callers either have a fixed parameter/filename
> >> > string, or do preliminary checks on the string (such as slirp), or do
> >> > not need it, such as -qtest.
> >> >
> >> > On a related note, the list of monitor creation places:
> >> >
> >> > - the chardev creators listed above: all from command line (except
> >> > perhaps Xen console?)
> >> >
> >> > - -gdb & hmp gdbserver will create a "GDB monitor command" chardev
> >> > that is wired to an HMP monitor.
> >> >
> >> > - -mon command line option
> >>
> >> Aside: the question "what does it mean to connect a monitor to a chardev
> >> that has an implicit monitor" crosses my mind. Next thought is "the
> >> day's too short to go there".
> >>
> >> > From this short study, I would like to think that a monitor may only
> >> > be created in the main thread today, though I remain skeptical :)
> >>
> >> Hah!
> >>
> >> > Signed-off-by: Marc-André Lureau
> >> > ---
> >> > include/chardev/char.h | 18 +-
> >> > chardev/char.c | 21 +
> >> > gdbstub.c | 6 +-
> >> > hw/char/xen_console.c | 5 -
> >> > vl.c | 8
> >> > 5 files changed, 47 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/include/chardev/char.h b/include/chardev/char.h
> >> > index 6f0576e214..be2b9b817e 100644
> >> > --- a/
> >> > +++ b/include/chardev/char.h
> >> > @@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> >> > * @qemu_chr_new:
> >> > *
> >> > * Create a new character backend from a URI.
> >> > + * Do not implicitely initialize a monitor if the chardev is muxed.
> >> > *
> >> > * @label the name of the backend
> >> > * @filename the URI
> >> > @@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> >> > */
> >> > Chardev *qemu_chr_new(const char *label, const char *filename);
> >> >
> >> > +/**
> >> > + * @qemu_chr_new_mux_mon:
> >> > + *
> >> > + * Create a new character backend from a URI.
> >> > + * Implicitely initialize a monitor if the chardev is muxed.
> >> > + *
> >> > + * @label the name of the backend
> >> > + * @filename the URI
> >>
> >> I'm no friend of GTK-Doc style, but where we use it, we should use it
> >> correctly: put a colon after @param.
> >
> > I copied existing comment... Should I fixed all others in the headers?
>
> Do we expect to actually *use* doc comments for anything? We haven't in
> a decade or so, but if we still expect to all the same, then not fixing
> them up now merely delays the inevitable.
>
> Do we think adding the colons makes the comments easier to read? For
> what it's worth, I do.
>
> Decision's up to you.
Let's improve it.
>
> >> > + *
> >> > + * Returns: a new character backend
> >> > + */
> >> > +Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename);
> >> > +
> >> > /**
> >> > * @qemu_chr_change:
> >> > *
> >> > @@ -138,10 +152,12 @@ void qemu_chr_cleanup(void);
> >> > *
> >> > * @label the name of the backend
> >> > * @filename the URI
> >> > + * @with_mux_mon if chardev is muxed, initialize a monitor
> >> > *
> >> > * Returns: a new character backend
> >> > */
> >> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
> >> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
> >> > + bool with_mux_mon);
> >> >
> >> > /**
> >> > * @qemu_chr_be_can_write:
> >> > diff --git a/chardev/char.c b/chardev/char.c
> >> > index 76d866e6fe..2c295ad676 100644
> >> > --- a/chardev/char.c
> >> > +++ b/chardev/char.c
> >> > @@ -683,7 +683,8 @@ out:
> >> > return chr;
> >> > }
> >> >
> >> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename)
> >> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
> >> > + bool with_mux_mon)
> >> > {
> >> > const char *p;
> >> > Chardev *chr;
> >> > @@ -702,17 +703,19 @@ Chardev *qemu_chr_new_noreplay(const char *label,
> >> > const char *filename)
> >> > if (err) {
> >> > error_report_err(err);
> >> > }
> >> > -if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
>
Re: [Xen-devel] [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
Hello, On 27/08/2018 20:24, Volodymyr Babchuk wrote: > > > On 27.08.18 09:44, Jan Beulich wrote: > > [...] > >>> xen/arch/arm/arm32/Makefile | 1 + >>> xen/arch/arm/arm32/smc.S | 39 >>> +++ >>> xen/arch/arm/arm64/Makefile | 1 + >>> xen/arch/arm/arm64/asm-offsets.c | 4 >>> xen/arch/arm/arm64/smc.S | 30 ++ >>> xen/include/asm-arm/processor.h | 11 +++ >>> 6 files changed, 86 insertions(+) >>> create mode 100644 xen/arch/arm/arm32/smc.S >>> create mode 100644 xen/arch/arm/arm64/smc.S >> >> With this diffstat, why did I end up on the _To_ list of this patch? >> I shouldn't even have been on the Cc list (and I'm going to blindly >> delete all other patches of this series where you've also >> apparently put me on the To list). Please remember that patches >> are supposed to be sent _To_ the list, with maintainers, reviewers >> (and perhaps others) Cc-ed. Together with people replying to >> mails often not adjusting To/Cc lists accordingly, I've now ended >> up with well over 30 mails in my primary mail folder which >> presumably I have no business with at all. > > My apologies for disturbing you. I really have no intention to upset you. > I used get_maintainer.pl script and it showed your e-mail address. So I > added you to To: list. > > Now I tried add_mandainers.pl and it nevertheless adds you to CCs. > So, I'm really sorry bothering you. But probably you might want to fix > mentioned scripts/MAINTAINERS file, to make sure you will never receive > unwanted emails? I did try add_maintainers.pl/get_maintainer.pl on that patch and I get only Stefano and I CCed. So more likely you are not using the scripts correctly. 42sh> ls *.patch 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch 42sh> scripts/get_maintainers.pl 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch Stefano Stabellini Julien Grall xen-devel@lists.xenproject.org 42sh> scripts/add_maintainers.pl -d . Processing: 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch Then perform: git send-email -to xen-devel@lists.xenproject.org ./*.patch 42sh> ack "Cc|To" 0001-arm-add-SMC-wrapper-that-is-compatible-with-SMCCC.patch To: xen-devel@lists.xenproject.org Cc: Stefano Stabellini Cc: Julien Grall Note that we fixed one issue in add_maintainers.pl that would lead to your problem. However, this was merged nearly 3 months ago and I would expect your series to be based on staging... So you probably want to do upstream development on staging or change your command line. If the documentation is misleading, then contribution will be appreciated. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86/xen: avoid 32-bit writes to PTEs in PV PAE guests
On 08/21/2018 11:37 AM, Juergen Gross wrote: > While the hypervisor emulates plain writes to PTEs happily, this is > much slower than issuing a hypercall for PTE modifcations. And writing > a PTE via two 32-bit write instructions (especially when clearing the > PTE) will result in an intermediate L1TF vulnerable PTE. > > Writes to PAE PTEs should always be done with 64-bit writes or via > hypercalls. > > Juergen Gross (2): > x86/xen: don't write ptes directly in 32-bit PV guests > x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear > > arch/x86/include/asm/pgtable-3level.h | 7 +++ > arch/x86/xen/mmu_pv.c | 7 +++ > 2 files changed, 6 insertions(+), 8 deletions(-) > Applied to for-linus-19b. (+stable.) -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
Hi Jan, On 27.08.18 09:44, Jan Beulich wrote: [...] xen/arch/arm/arm32/Makefile | 1 + xen/arch/arm/arm32/smc.S | 39 +++ xen/arch/arm/arm64/Makefile | 1 + xen/arch/arm/arm64/asm-offsets.c | 4 xen/arch/arm/arm64/smc.S | 30 ++ xen/include/asm-arm/processor.h | 11 +++ 6 files changed, 86 insertions(+) create mode 100644 xen/arch/arm/arm32/smc.S create mode 100644 xen/arch/arm/arm64/smc.S With this diffstat, why did I end up on the _To_ list of this patch? I shouldn't even have been on the Cc list (and I'm going to blindly delete all other patches of this series where you've also apparently put me on the To list). Please remember that patches are supposed to be sent _To_ the list, with maintainers, reviewers (and perhaps others) Cc-ed. Together with people replying to mails often not adjusting To/Cc lists accordingly, I've now ended up with well over 30 mails in my primary mail folder which presumably I have no business with at all. My apologies for disturbing you. I really have no intention to upset you. I used get_maintainer.pl script and it showed your e-mail address. So I added you to To: list. Now I tried add_mandainers.pl and it nevertheless adds you to CCs. So, I'm really sorry bothering you. But probably you might want to fix mentioned scripts/MAINTAINERS file, to make sure you will never receive unwanted emails? -- Volodymyr Babchuk ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 2/6] arm: add generic TEE mediator framework
Hi Julien, On 22.08.18 20:03, Julien Grall wrote: [...] if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) ) goto fail; + /* Notify TEE that new domain was created */ + tee_domain_create(d); My concern about domain creation is still not addressed. I would expect the toolstack to decide whether TEE should be initialized for a given guest and potentially return an error on failure (e.g maximum client ID has been reached). But very likely, you don't need to initialize TEE that early. This could be done in a separate DOMCTL as we did for VPL011. Yes, as we discussed in latter patches, I'll add DOMCTL support. But what to do with dom0 construction? I think, it should be configurable. But how? With commandline option? [...] } + printk(XENLOG_WARNING "No TEE mediator found\n"); Not having a TEE is a valid use case. So printing a warning seems a bit too much. I can change this to INFO. Or it is better to remove this print at all? Thank you for review, BTW. -- Volodymyr Babchuk ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 bisection] complete test-amd64-amd64-pair
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-pair testid guest-start/debian Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: a2d9a6fa1fcd2dfcfa9d1d34998156f9399a3eb2 Bug not present: 83fa6552cea112a900ec7891f8c170d022fe7e20 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/126795/ commit a2d9a6fa1fcd2dfcfa9d1d34998156f9399a3eb2 Author: Paul Durrant Date: Thu Aug 9 10:59:41 2018 +0100 tools/libxenctrl: use new xenforeignmemory API to seed grant table A previous patch added support for priv-mapping guest resources directly (rather than having to foreign-map, which requires P2M modification for HVM guests). This patch makes use of the new API to seed the guest grant table unless the underlying infrastructure (i.e. privcmd) doesn't support it, in which case the old scheme is used. NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was actually unnecessary, as the grant table has already been seeded by a prior call to xc_dom_gnttab_init() made by libxl__build_dom(). Signed-off-by: Paul Durrant Acked-by: Marek Marczykowski-Górecki Reviewed-by: Roger Pau Monné Acked-by: Wei Liu Reviewed-by: Andrew Cooper For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-4.9/test-amd64-amd64-pair.guest-start--debian.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/linux-4.9/test-amd64-amd64-pair.guest-start--debian --summary-out=tmp/126795.bisection-summary --basis-template=125183 --blessings=real,real-bisect linux-4.9 test-amd64-amd64-pair guest-start/debian Searching for failure / basis pass: 126677 fail [dst_host=baroque1,src_host=baroque0] / 126017 [dst_host=albana0,src_host=albana1] 125896 [dst_host=debina0,src_host=debina1] 125511 [dst_host=debina1,src_host=debina0] 125342 [dst_host=elbling0,src_host=elbling1] 125271 [dst_host=albana1,src_host=albana0] 125183 ok. Failure / basis pass flights: 126677 / 125183 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest e8d49e4292d9156a081752dee3f5a0cd12857da9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 4f080070a9809bde857851e68a3aeff0c4b9b6a6 3a2b8525b883baa87fe89b3da58f5c09fa599b99 Basis pass 060744011e93679f03932f050619744be895b772 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 41cb2db62627a7438d938aae487550c3f4acb1da Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git#060744011e93679f03932f050619744be895b772-e8d49e4292d9156a081752dee3f5a0cd12857da9 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-c8ea0457495342c417c3dc033bba25148b279f60 git://xenbits.xen.org/qemu-xen.git#43139135a8938de44f66333831d3a8655d07663a-4f080070a9809bde857851e68a3aeff0c4b9b6a6 git://xenbits.xen.org/xen.git#41cb2db62627a7438d938aae487550c3f4acb1da-3a2b8525b883baa87fe89b3da58f5c09fa599b99 adhoc-revtuple-generator: tree discontiguous: qemu-xen Loaded 2002 nodes in revision graph Searching for test results: 125156 [dst_host=godello1,src_host=godello0] 125183 pass 060744011e93679f03932f050619744be895b772 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 41cb2db62627a7438d938aae487550c3f4acb1da 125271 [dst_host=albana1,src_host=albana0] 125342 [dst_host=elbling0,src_host=elbling1] 125511 [dst_host=debina1,src_host=debina0] 125896 [dst_host=debina0,src_host=debina1] 126017 [dst_host=albana0,src_host=albana1] 126187 fail irrelevant 126345 fail irrelevant 126259 fail irrelevant 126440 fail 676054232ecfeaf9392201fd5fc3f74a6d39d9f3 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 4f080070a9809bde857851e68a3aeff0c4b9b6a6
[Xen-devel] [PATCH] xen: export device state to sysfs
Export device state to sysfs to allow for easier get device state.
Signed-off-by: Joe Jin
Boris Ostrovsky
Juergen Gross
---
drivers/xen/xenbus/xenbus_probe.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index 74888cacd0b0..52f99b80a1c0 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -402,10 +402,19 @@ static ssize_t modalias_show(struct device *dev,
}
static DEVICE_ATTR_RO(modalias);
+static ssize_t state_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%s\n",
+ xenbus_strstate(to_xenbus_device(dev)->state));
+}
+static DEVICE_ATTR_RO(state);
+
static struct attribute *xenbus_dev_attrs[] = {
_attr_nodename.attr,
_attr_devtype.attr,
_attr_modalias.attr,
+ _attr_state.attr,
NULL,
};
--
2.15.2 (Apple Git-101.1)
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 126773: all pass - PUSHED
flight 126773 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/126773/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 033949a810cd9cb4a604cf09af503459ea1d66dc baseline version: ovmf 983f5abb9a0d6cf9cfb5e16d671f15e5dc7510d8 Last test of basis 126742 2018-08-27 01:41:42 Z0 days Testing same since 126773 2018-08-27 12:46:50 Z0 days1 attempts People who touched revisions under test: Ruiyu Ni 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 983f5abb9a..033949a810 033949a810cd9cb4a604cf09af503459ea1d66dc -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 126761: tolerable FAIL - PUSHED
flight 126761 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/126761/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: build-amd64-xen-freebsd 7 xen-build-freebsdfail never pass version targeted for testing: freebsd 7cbaa4254c0f10f7c00014ce9591ca60abfa57fb baseline version: freebsd 6dafcae1dddb8858d56d3337f4ae49071dff752a Last test of basis 126535 2018-08-24 09:19:31 Z3 days Testing same since 126761 2018-08-27 09:19:01 Z0 days1 attempts People who touched revisions under test: alc brd bz cperciva delphij glebius imp kevans kib manu marius markj markm mmel np sbruno tuexen jobs: build-amd64-freebsd-againpass build-amd64-freebsd pass build-amd64-xen-freebsd fail 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/freebsd.git 6dafcae1ddd..7cbaa4254c0 7cbaa4254c0f10f7c00014ce9591ca60abfa57fb -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-sid test] 75129: tolerable FAIL
flight 75129 distros-debian-sid real [real] http://osstest.xensource.com/osstest/logs/75129/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-armhf-sid-netboot-pygrub 10 debian-di-install fail like 75096 test-amd64-amd64-i386-sid-netboot-pygrub 10 debian-di-install fail like 75096 test-amd64-i386-i386-sid-netboot-pvgrub 10 debian-di-install fail like 75096 test-amd64-i386-amd64-sid-netboot-pygrub 10 debian-di-install fail like 75096 test-amd64-amd64-amd64-sid-netboot-pvgrub 10 debian-di-install fail like 75096 baseline version: flight 75096 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-sid-netboot-pvgrubfail test-amd64-i386-i386-sid-netboot-pvgrub fail test-amd64-i386-amd64-sid-netboot-pygrub fail test-armhf-armhf-armhf-sid-netboot-pygrubfail test-amd64-amd64-i386-sid-netboot-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.9-testing test] 126710: regressions - FAIL
flight 126710 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/126710/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail REGR. vs. 124248 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 124328 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 10 debian-install fail REGR. vs. 124328 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail blocked in 124328 test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail like 124248 test-amd64-amd64-xl-qemuu-ws16-amd64 14 guest-localmigratefail like 124248 test-amd64-i386-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail like 124248 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 124328 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 124328 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 124328 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-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 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-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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 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-libvirt-vhd 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-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 71e51140fdeb98c8fefc3a7067b554212bb61ac9 baseline version: xen 238007d6fae9447bf5e8e73d67ae9fb844e7ff2a Last test of basis 124328 2018-06-17 23:39:07 Z 70 days Failing since124807 2018-06-28 17:38:04 Z 59 days 38 attempts Testing same since 126296 2018-08-21 01:12:38 Z6 days5 attempts People who touched revisions under test: Andrew Cooper Christian Lindig George Dunlap Ian Jackson Jan Beulich Juergen Gross Julien Grall Kevin Tian Lars Kurth Paul Durrant Stefano Stabellini Stefano Stabellini Stewart Hildebrand Wei Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-prev pass
Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-4.19 for 4.19..
On 8/27/18 10:19 AM, Konrad Rzeszutek Wilk wrote: > Hey Jens, > > Would you be OK pulling the following branch: > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git > stable/for-jens-4.19 > > which has a fix for flushing out persistent pages at a deterministic rate. > > Thanks to the L1TF I did not manage to send this email until today - but > hopefully it won't be an issue to push it to Linus after -rc1? > > It is based on your 'b86d865cb1ca (for-4.19/block) blkcg: Make > blkg_root_lookup() > work for queues in bypass mode' branch. Pulled, thanks. -- Jens Axboe ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/2] x86/spec-ctrl: add support for modifying SSBD via LS_CFG MSR
Adds support for modifying the LS_CFG MSR to enable SSBD on supporting
AMD CPUs. There needs to be locking logic for family 17h with SMT
enabled since both threads share the same MSR. Otherwise, a core just
needs to write to the LS_CFG MSR. For more information see:
https://developer.amd.com/wp-content/resources/124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
Signed-off-by: Brian Woods
---
xen/arch/x86/cpu/amd.c| 13 +--
xen/arch/x86/smpboot.c| 3 +
xen/arch/x86/spec_ctrl.c | 172 +-
xen/include/asm-x86/cpufeatures.h | 1 +
xen/include/asm-x86/spec_ctrl.h | 2 +
5 files changed, 181 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 6e65ae7427..e96f14268e 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -601,8 +601,8 @@ static void init_amd(struct cpuinfo_x86 *c)
}
/*
-* If the user has explicitly chosen to disable Memory Disambiguation
-* to mitigiate Speculative Store Bypass, poke the appropriate MSR.
+* Poke the LS_CFG MSR to see if the mitigation for Speculative
+* Store Bypass is available.
*/
if (!ssbd_amd_ls_cfg_mask) {
int bit = -1;
@@ -615,14 +615,9 @@ static void init_amd(struct cpuinfo_x86 *c)
if (bit >= 0)
ssbd_amd_ls_cfg_mask = 1ull << bit;
- }
- if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
- ssbd_amd_ls_cfg_av = true;
- if (opt_ssbd) {
- value |= ssbd_amd_ls_cfg_mask;
- wrmsr_safe(MSR_AMD64_LS_CFG, value);
- }
+ if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG,
value))
+ ssbd_amd_ls_cfg_av = true;
}
/* MFENCE stops RDTSC speculation */
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 7e76cc3d68..b103b46dee 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -376,6 +376,9 @@ void start_secondary(void *unused)
if ( boot_cpu_has(X86_FEATURE_IBRSB) )
wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
+if ( default_xen_ssbd_amd_ls_cfg_en )
+ssbd_amd_ls_cfg_set(true);
+
if ( xen_guest )
hypervisor_ap_setup();
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index b32c12c6c0..89cc444f56 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -58,8 +59,17 @@ paddr_t __read_mostly l1tf_addr_mask, __read_mostly
l1tf_safe_maddr;
static bool __initdata cpu_has_bug_l1tf;
static unsigned int __initdata l1d_maxphysaddr;
+/* for SSBD support for AMD via LS_CFG */
+#define SSBD_AMD_MAX_SOCKET 4
+struct ssbd_amd_ls_cfg_smt_status {
+spinlock_t lock;
+uint32_t mask;
+} __attribute__ ((aligned (64)));
+bool __read_mostly ssbd_amd_smt_en;
+bool __read_mostly default_xen_ssbd_amd_ls_cfg_en;
bool ssbd_amd_ls_cfg_av;
uint64_t __read_mostly ssbd_amd_ls_cfg_mask;
+struct ssbd_amd_ls_cfg_smt_status *ssbd_amd_smt_status[SSBD_AMD_MAX_SOCKET];
static int __init parse_bti(const char *s)
{
@@ -319,7 +329,7 @@ static void __init print_details(enum ind_thunk thunk,
uint64_t caps)
!boot_cpu_has(X86_FEATURE_SSBD) ? "" :
(default_xen_spec_ctrl & SPEC_CTRL_SSBD) ? " SSBD+" : " SSBD-",
!ssbd_amd_ls_cfg_av ? "" :
- opt_ssbd ? " LS_CFG_SSBD+" : "
LS_CFG_SSBD-",
+ default_xen_ssbd_amd_ls_cfg_en? " LS_CFG_SSBD+" : "
LS_CFG_SSBD-",
opt_ibpb ? " IBPB" : "",
opt_l1d_flush ? " L1D_FLUSH" : "");
@@ -725,6 +735,162 @@ static __init int parse_xpti(const char *s)
}
custom_param("xpti", parse_xpti);
+/*
+ * Enabling SSBD on AMD processers via the LS_CFG MSR
+ *
+ * For family 15h and 16h, there are no SMT enabled processors, so there
+ * is no need for locking, just setting an MSR bit. For 17h, it depends
+ * if SMT is enabled. If SMT, are two threads that share a single MSR
+ * so there needs to be a lock and a virtual bit for each thread,
+ * otherwise it's the same as family 15h/16h.
+ */
+
+static void ssbd_amd_ls_cfg_set_nonsmt(bool enable_ssbd)
+{
+uint64_t ls_cfg, new_ls_cfg;
+
+rdmsrl(MSR_AMD64_LS_CFG, ls_cfg);
+
+if ( enable_ssbd )
+new_ls_cfg = ls_cfg | ssbd_amd_ls_cfg_mask;
+else
+new_ls_cfg = ls_cfg & ~ssbd_amd_ls_cfg_mask;
+
+if ( new_ls_cfg != ls_cfg )
+wrmsrl(MSR_AMD64_LS_CFG, new_ls_cfg);
+}
+
+static void ssbd_amd_ls_cfg_set_smt(bool enable_ssbd)
+{
+unsigned socket, core, thread;
+uint64_t enable_mask;
+uint64_t ls_cfg;
+struct ssbd_amd_ls_cfg_smt_status *status;
+
[Xen-devel] [PATCH v3 0/2] SSBD via LS_CFG
This series of patches is for enabling SSBD via the LS_CFG MSR for family 15h-17h. The first patch make it so that the information is correctly displayed on boot. The last patch is for further enablement for Xen switching SSBD on or off internally, or for further control of SSBD for guests via the VIRT_SPEC_CTRL. Note: Patch 1 is standalone and just improves reporting of SSBD in the init print statements. Patch 2 is intended for a later date when there's a C ALTERNATIVE (which I've talked to Andy about some) and will most likely get rolled into series of patches for giving HVM guests control of SSBD when it's only available via LS_CFG. v1 -> v2: - changed some variable types - updated ssbd_amd_ls_cfg_set_nonsmt function - changed sbd_amd_ls_cfg_init to a presmp_initcall - deleted ssbd_amd_ls_cfg_set_init due to ^ and it happening later in the boot - various smaller cleanups v2 -> v3: - moved the SSBD_AMD_LS_CFG synthetic feature from patch 1 to 2 and added ssbd_amd_ls_cfg_av for it's use in patch 1 - in patch 2, only set SSBD_AMD_LS_CFG once everything is known to be ready/good - since it's later in the boot process due to a v1 change, use nr_sockets - various smaller changes/cleanups from Jan's feedback Brian Woods (2): x86/spec-ctrl: add AMD SSBD LS_CFG in init print x86/spec-ctrl: add support for modifying SSBD via LS_CFG MSR xen/arch/x86/cpu/amd.c| 15 ++-- xen/arch/x86/smpboot.c| 3 + xen/arch/x86/spec_ctrl.c | 180 +- xen/include/asm-x86/cpufeatures.h | 1 + xen/include/asm-x86/spec_ctrl.h | 5 ++ 5 files changed, 195 insertions(+), 9 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 1/2] x86/spec-ctrl: add AMD SSBD LS_CFG in init print
Edit the initialization code for AMD's SSBD via the LS_CFG MSR and
enable it to pass the status to the initial spec-ctrl print_details at
boot.
Signed-off-by: Brian Woods
---
xen/arch/x86/cpu/amd.c | 12 +---
xen/arch/x86/spec_ctrl.c| 10 --
xen/include/asm-x86/spec_ctrl.h | 3 +++
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index a7afa2fa7a..6e65ae7427 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -604,7 +604,7 @@ static void init_amd(struct cpuinfo_x86 *c)
* If the user has explicitly chosen to disable Memory Disambiguation
* to mitigiate Speculative Store Bypass, poke the appropriate MSR.
*/
- if (opt_ssbd) {
+ if (!ssbd_amd_ls_cfg_mask) {
int bit = -1;
switch (c->x86) {
@@ -613,8 +613,14 @@ static void init_amd(struct cpuinfo_x86 *c)
case 0x17: bit = 10; break;
}
- if (bit >= 0 && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
- value |= 1ull << bit;
+ if (bit >= 0)
+ ssbd_amd_ls_cfg_mask = 1ull << bit;
+ }
+
+ if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
+ ssbd_amd_ls_cfg_av = true;
+ if (opt_ssbd) {
+ value |= ssbd_amd_ls_cfg_mask;
wrmsr_safe(MSR_AMD64_LS_CFG, value);
}
}
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index c430b25b84..b32c12c6c0 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -58,6 +58,9 @@ paddr_t __read_mostly l1tf_addr_mask, __read_mostly
l1tf_safe_maddr;
static bool __initdata cpu_has_bug_l1tf;
static unsigned int __initdata l1d_maxphysaddr;
+bool ssbd_amd_ls_cfg_av;
+uint64_t __read_mostly ssbd_amd_ls_cfg_mask;
+
static int __init parse_bti(const char *s)
{
const char *ss;
@@ -281,11 +284,12 @@ static void __init print_details(enum ind_thunk thunk,
uint64_t caps)
printk("Speculative mitigation facilities:\n");
/* Hardware features which pertain to speculative mitigations. */
-printk(" Hardware features:%s%s%s%s%s%s%s%s%s%s\n",
+printk(" Hardware features:%s%s%s%s%s%s%s%s%s%s%s\n",
(_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_L1D_FLUSH)) ? " L1D_FLUSH" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_SSBD)) ? " SSBD" : "",
+ ssbd_amd_ls_cfg_av ? " LS_CFG_SSBD" : "",
(e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
(caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "",
(caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "",
@@ -305,7 +309,7 @@ static void __init print_details(enum ind_thunk thunk,
uint64_t caps)
"\n");
/* Settings for Xen's protection, irrespective of guests. */
-printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s\n",
+printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s%s, Other:%s%s\n",
thunk == THUNK_NONE ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE? "LFENCE" :
@@ -314,6 +318,8 @@ static void __init print_details(enum ind_thunk thunk,
uint64_t caps)
(default_xen_spec_ctrl & SPEC_CTRL_IBRS) ? "IBRS+" : "IBRS-",
!boot_cpu_has(X86_FEATURE_SSBD) ? "" :
(default_xen_spec_ctrl & SPEC_CTRL_SSBD) ? " SSBD+" : " SSBD-",
+ !ssbd_amd_ls_cfg_av ? "" :
+ opt_ssbd ? " LS_CFG_SSBD+" : "
LS_CFG_SSBD-",
opt_ibpb ? " IBPB" : "",
opt_l1d_flush ? " L1D_FLUSH" : "");
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 8f8aad40bb..1b9101a988 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -50,6 +50,9 @@ extern int8_t opt_pv_l1tf;
*/
extern paddr_t l1tf_addr_mask, l1tf_safe_maddr;
+extern bool ssbd_amd_ls_cfg_av;
+extern uint64_t ssbd_amd_ls_cfg_mask;
+
static inline void init_shadow_spec_ctrl_state(void)
{
struct cpu_info *info = get_cpu_info();
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention
Hi,
On 27.08.18 18:29, Julien Grall wrote:
On 27/08/2018 15:15, Volodymyr Babchuk wrote:
Hi Julien,
Hi,
On 24.08.18 19:58, Julien Grall wrote:
Signed-off-by: Julien Grall
---
xen/arch/arm/psci.c | 4
xen/include/asm-arm/cpufeature.h | 3 ++-
xen/include/asm-arm/smccc.h | 8
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 3cf5ecf0f3..941eec921b 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -118,6 +119,9 @@ static void __init psci_init_smccc(void)
smccc_ver = ret;
}
+ if ( smccc_ver >= SMCCC_VERSION(1, 1) )
+ cpus_set_cap(ARM_SMCCC_1_1);
+
printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
SMCCC_VERSION_MAJOR(smccc_ver),
SMCCC_VERSION_MINOR(smccc_ver));
}
diff --git a/xen/include/asm-arm/cpufeature.h
b/xen/include/asm-arm/cpufeature.h
index 9c297c521c..c9c4046f5f 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -44,8 +44,9 @@
#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
#define ARM_HARDEN_BRANCH_PREDICTOR 7
#define ARM_SSBD 8
+#define ARM_SMCCC_1_1 9
-#define ARM_NCAPS 9
+#define ARM_NCAPS 10
#ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 1ed6cbaa48..7c39c530e2 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -213,6 +213,7 @@ struct arm_smccc_res {
*/
#ifdef CONFIG_ARM_32
#define arm_smccc_1_0_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
#else
void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
@@ -254,6 +255,13 @@ void __arm_smccc_1_0_smc(register_t a0,
register_t a1, register_t a2,
#define
arm_smccc_1_0_smc(...) \
__arm_smccc_1_0_smc_count(__count_args(__VA_ARGS__),
__VA_ARGS__)
+#define arm_smccc_smc(...) \
+ do { \
+ if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) \
+ arm_smccc_1_0_smc(__VA_ARGS__); \
+ else \
+ arm_smccc_1_1_smc(__VA_ARGS__); \
+ } while ( 0 )
#endif /* CONFIG_ARM_64 */
This will generate lots of code for every arm_smccc_smc(). Can we have
function pointer arm_smccc_smc instead and assign it to either
arm_smccc_1_1_smc() or arm_asmccc_1_0_smc() at boot?
I know that currently we have no function arm_smccc_1_1_smc() because
it is being constructed inline every time. But we can write it
explicitly and then have one indirect call to (maybe cached) function
instead of
lots inlined code with conditional branches.
This is indeed increasing the size of the function, but with a better
performance when you perform an SMC with 1.1.
The main goal of SMCCC 1.1 is to limit the number of registers saved
when calling an SMC. So if you provide provide a wrapper using a
function, then you are better off sticking with SMCCC 1.0.
The idea of this code is to provide a faster call on the presence of
SMCCC 1.1 while providing a fallback for other case. The function
cpus_have_const_cap is implemented using an ALTERNATIVE (similar to
static key) that will make if close to a NOP. I am saying close because
this is not quite a static key as we don't have it in Xen. Instead, you
replace a memory load by a constant.
Ah, yes, true.
I checked how static keys are working. Seems, they use asm goto feature.
Now I think: can we optimize this more? Something like that:
#define arm_smccc_smc(...)
do {
asm goto (ALTERNATIVE(nop",
"b %l[smccc_1_1]",
ARM_SMCCC_1_1));
arm_smccc_1_0_smc(__VA_ARGS__);
break;
smccc_1_1:
arm_smccc_1_1_smc(__VA_ARGS__);
} while ( 0 )
This will save use additional conditional branch.
--
Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] tboot: Avoid recursive fault in early boot panic with tboot
On Thu, Jul 19, 2018 at 6:39 AM Jason Andryuk wrote: > > If panic is called before init_idle_domain on a tboot-launched system, > then Xen recursively faults in write_ptbase as seen below. > > (XEN)[] write_ptbase+0/0x10 > (XEN)[] tboot_shutdown+0x6b/0x260 > (XEN)[] machine_restart+0xac/0x2d0 > (XEN)[] write_ptbase+0/0x10 > (XEN)[] panic+0x111/0x120 > (XEN)[] do_general_protection+0x171/0x1f0 > (XEN)[] mm.c#virt_to_xen_l2e+0x12/0x1c0 > (XEN)[] x86_64/entry.S#handle_exception_saved+0x66/0xa4 > (XEN)[] write_ptbase+0/0x10 > (XEN)[] tboot_shutdown+0x6b/0x260 > (XEN)[] machine_restart+0xac/0x2d0 > (XEN)[] panic+0x111/0x120 > (XEN)[] setup.c#bootstrap_map+0/0x11a > (XEN)[] flask_op.c#parse_flask_param+0/0xb0 > (XEN)[] setup.c#bootstrap_map+0/0x11a > (XEN)[] xsm_multiboot_init+0x7c/0xb0 > (XEN)[] __start_xen+0x1d2b/0x2da0 > (XEN)[] __high_start+0x53/0x60 > > idle_vcpu[0] is still poisoned with INVALID_VCPU, so write_ptbase faults > dereferencing the pointer. This fault calls panic and recurses through > the same code path. > > If tboot_shutdown is called while idle_vcpu[0] == INVALID_VCPU, then we > are still operating with the initial page tables. Therefore changing > page tables with write_ptbase is unnecessary. > > An easy way to reproduce this is to use tboot to launch an XSM-enabled > Xen without an XSM policy. > > Signed-off-by: Jason Andryuk Ping, Shane and Gang? > --- > v2: Correct comment and brace style > --- > xen/arch/x86/tboot.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c > index fb4616ae83..d5a5292d7e 100644 > --- a/xen/arch/x86/tboot.c > +++ b/xen/arch/x86/tboot.c > @@ -391,7 +391,12 @@ void tboot_shutdown(uint32_t shutdown_type) > tboot_gen_xenheap_integrity(g_tboot_shared->s3_key, _mac); > } > > -write_ptbase(idle_vcpu[0]); > +/* > + * During early boot, we can be called by panic before idle_vcpu[0] is > + * setup, but in that case we don't need to change page tables. > + */ > +if ( idle_vcpu[0] != INVALID_VCPU ) > +write_ptbase(idle_vcpu[0]); > > ((void(*)(void))(unsigned long)g_tboot_shared->shutdown_entry)(); > > -- > 2.17.1 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/arm: smccc-1.1: Make return values unsigned long
Julien,
On 27.08.18 18:23, Julien Grall wrote:
On 27/08/2018 15:03, Volodymyr Babchuk wrote:
Hi Julien, Marc
Hi,
On 24.08.18 19:58, Julien Grall wrote:
From: Marc Zyngier
An unfortunate consequence of having a strong typing for the input
values to the SMC call is that it also affects the type of the
return values, limiting r0 to 32 bits and r{1,2,3} to whatever
was passed as an input. >
Let's turn everything into "unsigned long", which satisfies the
requirements of both architectures, and allows for the full
range of return values.
Maybe it better to use register_t then? By definition register_t has
the same size as a CPU register and SMC uses CPU registers to pass
parameters/return values.
The code is based on Linux series (posted last Friday). I don't much
want to diverge too much from it. So unsigned "unsigned long" here is ok.
Reported-by: Stefano Stabellini
Signed-off-by: Marc Zyngier
Signed-off-by: Julien Grall
---
xen/include/asm-arm/smccc.h | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 74c13f8419..a31d67a1de 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -119,35 +119,35 @@ struct arm_smccc_res {
#define __declare_arg_0(a0, res) \
struct arm_smccc_res *___res = res; \
- register uin32_t r0 asm("r0") = a0; \
+ register unsigned long r0 asm("r0") = (uint32_t)a0;\
Do you really want to silently drop upper 32 bits of the argument?
I know, that SMCCC states that function id is a 32-bit value,
but I don't think that it is a good place to enforce this
behavior.
I think it is better to allow user to shoot in his leg in this case.
I don't see any issue with casting here. This is what the SMCCC request
for x0 and after all you silently drop upper 32-bit when using a static
inline function...
Yes, you are right.
Reviewed-by: Volodymyr Babchuk
--
Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] build: remove tboot make targets
On Mon, Aug 20, 2018 at 02:33:10AM -0600, Jan Beulich wrote: > >>> On 19.08.18 at 04:22, wrote: > > The tboot targets are woefully out of date. These should really be > > retired because setting up tboot is more complex than the build process > > for it. > > > > Signed-off-by: Doug Goldstein > > Acked-by: Jan Beulich > > But I think you would better have Cc-ed the TXT/TBOOT maintainers > (now done), as it was presumably them having introduced these > targets. Even if the chance if high to not see any response, we > should at least give them a chance to chime in. Just a one week ping. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [GIT PULL] (xen) stable/for-jens-4.19 for 4.19..
Hey Jens, Would you be OK pulling the following branch: git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/for-jens-4.19 which has a fix for flushing out persistent pages at a deterministic rate. Thanks to the L1TF I did not manage to send this email until today - but hopefully it won't be an issue to push it to Linus after -rc1? It is based on your 'b86d865cb1ca (for-4.19/block) blkcg: Make blkg_root_lookup() work for queues in bypass mode' branch. Thank you! Documentation/ABI/testing/sysfs-driver-xen-blkback | 10 ++ drivers/block/xen-blkback/blkback.c| 99 ++- drivers/block/xen-blkback/common.h | 14 +-- drivers/block/xen-blkfront.c | 110 ++--- 4 files changed, 163 insertions(+), 70 deletions(-) Juergen Gross (5): xen/blkback: don't keep persistent grants too long xen/blkfront: cleanup stale persistent grants xen/blkfront: reorder tests in xlblk_init() xen/blkback: move persistent grants flags to bool xen/blkback: remove unused pers_gnts_lock from struct xen_blkif_ring ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 0/5] xen/blk: persistent grant rework
On Fri, Aug 24, 2018 at 03:52:23PM +0200, Juergen Gross wrote: > On 17/08/18 17:59, Roger Pau Monné wrote: > > On Mon, Aug 13, 2018 at 04:01:09PM +0200, Juergen Gross wrote: > >> Persistent grants are used in the Xen's blkfront/blkback drivers to > >> avoid mapping/unmapping of I/O buffers in the backend for each I/O. > >> > >> While this speeds up processing quite a bit there are problems related > >> to persistent grants in some configurations: domains with multiple > >> block devices making use of persistent grants might suffer from a lack > >> of grants if each of the block devices experienced a high I/O load at > >> some time. This is due to the number of persistent grants per device > >> only to be limited by a rather high maximum value, but never being > >> released even in case of longer times without any I/O. > >> > >> This series modifies xen-blkback to unmap any domU page mapped via a > >> persistent grant after a timeout (default: 60 seconds). The timeout > >> is set to its default value again when a persistent grant has been > >> used for an I/O. > >> > >> xen-blkfront is modified to scan every 10 seconds for persistent grants > >> not in use by blkback any more and to remove such grants. > >> > >> The last 3 patches are small cleanups of blkfront and blkback drivers. > >> > >> V3: > >> - patch 1: make timeout parameter static > > > > Konrad if you are OK with this series, could you please send a pull > > request to Jens? > > Ping? Yes, let me do that now. > > > Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86/xen: avoid 32-bit writes to PTEs in PV PAE guests
On Mon, Aug 27, 2018 at 12:03 PM Jason Andryuk wrote: > > On Tue, Aug 21, 2018 at 11:40 AM Juergen Gross wrote: > > > > While the hypervisor emulates plain writes to PTEs happily, this is > > much slower than issuing a hypercall for PTE modifcations. And writing > > a PTE via two 32-bit write instructions (especially when clearing the > > PTE) will result in an intermediate L1TF vulnerable PTE. > > > > Writes to PAE PTEs should always be done with 64-bit writes or via > > hypercalls. > > > > Juergen Gross (2): > > x86/xen: don't write ptes directly in 32-bit PV guests > > x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear > > > > I tested both patches on 4.14, changing patch 2 to atomic64_xchg since > arch_atomic64_xchg doesn't exist. > > I haven't seen https://bugzilla.kernel.org/show_bug.cgi?id=198497 > trigger since incorporating these patch. Without the patches, I would > have seen it trigger by now. Also, I've confirmed Xen does not enable > page table shadowing. For what it's worth, the PTEs that would > trigger Xen shadowing (0x8000'0002'') are the same as those > that triggered bug 198497. There was at least 1 non-Xen user affected > by 198497, but this at least seems to fix it for me. > > Tested-by: Jason Andryuk Also, can these patches be Cc: sta...@vger.kernel.org? Thanks, Jason ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86/xen: avoid 32-bit writes to PTEs in PV PAE guests
On Tue, Aug 21, 2018 at 11:40 AM Juergen Gross wrote: > > While the hypervisor emulates plain writes to PTEs happily, this is > much slower than issuing a hypercall for PTE modifcations. And writing > a PTE via two 32-bit write instructions (especially when clearing the > PTE) will result in an intermediate L1TF vulnerable PTE. > > Writes to PAE PTEs should always be done with 64-bit writes or via > hypercalls. > > Juergen Gross (2): > x86/xen: don't write ptes directly in 32-bit PV guests > x86/pae: use 64 bit atomic xchg function in native_ptep_get_and_clear > I tested both patches on 4.14, changing patch 2 to atomic64_xchg since arch_atomic64_xchg doesn't exist. I haven't seen https://bugzilla.kernel.org/show_bug.cgi?id=198497 trigger since incorporating these patch. Without the patches, I would have seen it trigger by now. Also, I've confirmed Xen does not enable page table shadowing. For what it's worth, the PTEs that would trigger Xen shadowing (0x8000'0002'') are the same as those that triggered bug 198497. There was at least 1 non-Xen user affected by 198497, but this at least seems to fix it for me. Tested-by: Jason Andryuk Thank you. Jason ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 15/23] x86/mm: put HVM only code under CONFIG_HVM
>>> On 26.08.18 at 14:19, wrote:
> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -1,9 +1,10 @@
> subdir-y += shadow
> -subdir-y += hap
> +subdir-$(CONFIG_HVM) += hap
>
> obj-y += paging.o
> -obj-y += p2m.o p2m-pt.o p2m-ept.o p2m-pod.o
> -obj-y += altp2m.o
> +obj-y += p2m.o p2m-pt.o
> +obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
> +obj-$(CONFIG_HVM) += altp2m.o
> obj-y += guest_walk_2.o
> obj-y += guest_walk_3.o
> obj-y += guest_walk_4.o
Would you mind once again sorting things at the same time?
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -535,7 +535,7 @@ struct page_info *p2m_get_page_from_gfn(
> int p2m_set_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
>unsigned int page_order, p2m_type_t p2mt, p2m_access_t
> p2ma)
> {
> -struct domain *d = p2m->domain;
> +struct domain * __maybe_unused d = p2m->domain;
Instead of this, why don't you simply latch hap_enabled()'s return
value into a local bool variable? (Otherwise I would ask for const
to be added here as you touch this anyway.) Adding
__maybe_unused should really be a last resort only.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 14/23] x86/mm: put nested p2m code under CONFIG_HVM
>>> On 26.08.18 at 14:19, wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1689,7 +1689,8 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
> {
> _update_runstate_area(prev);
> vpmu_switch_from(prev);
> -np2m_schedule(NP2M_SCHEDLE_OUT);
> +if ( nestedhvm_enabled(prevd) )
> +np2m_schedule(NP2M_SCHEDLE_OUT);
> }
>
> if ( is_hvm_domain(prevd) && !list_empty(>arch.hvm_vcpu.tm_list) )
> @@ -1756,7 +1757,8 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
>
> /* Must be done with interrupts enabled */
> vpmu_switch_to(next);
> -np2m_schedule(NP2M_SCHEDLE_IN);
> +if ( nestedhvm_enabled(nextd) )
> +np2m_schedule(NP2M_SCHEDLE_IN);
> }
How do these additions help? nestedhvm_enabled() is not an inline
function, and the entire series doesn't seem to touch hvm/nestedhvm.h
(i.e. there's no inline stub being added).
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -144,6 +144,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
>
> static void p2m_teardown_nestedp2m(struct domain *d)
> {
> +#ifdef CONFIG_HVM
> unsigned int i;
> struct p2m_domain *p2m;
>
> @@ -156,10 +157,12 @@ static void p2m_teardown_nestedp2m(struct domain *d)
> p2m_free_one(p2m);
> d->arch.nested_p2m[i] = NULL;
> }
> +#endif
> }
>
> static int p2m_init_nestedp2m(struct domain *d)
> {
> +#ifdef CONFIG_HVM
> unsigned int i;
> struct p2m_domain *p2m;
>
> @@ -176,6 +179,7 @@ static int p2m_init_nestedp2m(struct domain *d)
> p2m->write_p2m_entry = nestedp2m_write_p2m_entry;
> list_add(>np2m_list, _get_hostp2m(d)->np2m_list);
> }
> +#endif
>
> return 0;
> }
Hmm, I think this is too ad hoc for my taste: For one I'm puzzled
by the lack of any (existing) is_hvm_domain() here. And then the
fields initialization of which you skip should also disappear, to
eliminate the risk of some code somewhere using the fields
uninitialized. This might simply mean to move the fields from
struct arch_domain to struct hvm_domain. I understand this may
end up being a more involved task, but it looks pretty much
unavoidable to me.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 126779: tolerable all pass - PUSHED
flight 126779 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126779/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 36e29dd9e580cb0f847f5ac1e72afdb5febe3e99 baseline version: xen e5d6ddcd31a6113e4a3db7a235ca78770fe8f401 Last test of basis 126702 2018-08-26 13:00:31 Z1 days Failing since126763 2018-08-27 10:00:29 Z0 days3 attempts Testing same since 126779 2018-08-27 14:01:04 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Doug Goldstein Jan Beulich Kevin Tian Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git e5d6ddcd31..36e29dd9e5 36e29dd9e580cb0f847f5ac1e72afdb5febe3e99 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 13/23] x86: provide stubs, declarations and macros in hvm.h
>>> On 26.08.18 at 14:19, wrote:
> Make sure hvm_enabled evaluate to false then provide necessary stubs,
> declarations and macros to make Xen build.
>
> The is_viridian_domain macro can't be turned into an inline function
> easily, so instead its caller is modified to avoid unused variable
> warning.
I assume that's because struct domain hasn't been fully declared
yet at that point. Some preliminary looking around suggests that
it may be possible to address this by moving it plus a few more
nearby items into hvm/viridian.h, and avoiding other headers in
hvm/ to include hvm/viridian.h. This last item would in turn seem
to require to convert struct viridian_domain the instance in
struct hvm_domain to a pointer, which I personally think would
be desirable anyway. Paul?
Otoh I'm not really sure all of this is worth the effort.
> @@ -268,8 +271,8 @@ u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
> #define hvm_tsc_scaling_ratio(d) \
> ((d)->arch.hvm_domain.tsc_scaling_ratio)
>
> -u64 hvm_scale_tsc(const struct domain *d, u64 tsc);
> -u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz);
> +uint64_t hvm_scale_tsc(const struct domain *d, uint64_t tsc);
> +uint64_t hvm_get_tsc_scaling_ratio(uint32_t gtsc_khz);
I think this change doesn't really belong here. I don't mind if you
want to keep it, but then I question the need for uint32_t (rather
than unsigned int) for the second prototype's parameter.
> @@ -675,6 +678,146 @@ unsigned long hvm_cr4_guest_valid_bits(const struct
> domain *d, bool restore);
> d_->arch.hvm_domain.pi_ops.vcpu_block(v_); \
> })
>
> +#else /* CONFIG_HVM */
> +
> +#define hvm_enabled false
> +
> +static inline int hvm_guest_x86_mode(struct vcpu *v)
> +{
> +ASSERT_UNREACHABLE();
> +return -1;
> +}
Hmm, there's a whole lot of things down from here. I'd really like to
see this minimized quite a bit. A first thought: On top of my indirect
call patching series, instead of directly using the macros provided
by the alternatives.h additions, an extra layer of abstraction could
introduce a macro which evaluates to ASSERT_UNREACHABLE()
without HVM, and to alternative_{,v}call() otherwise. I think this
would eliminate quite a few of the newly added inline functions.
I'll therefore skip some of the additions here.
> +#define hvm_long_mode_active(v) (false)
> +#define hvm_pae_enabled(v) (false)
> +#define hvm_get_guest_time(v) (0)
> +#define is_viridian_domain(d) (false)
> +#define has_viridian_time_ref_count(d) (false)
> +#define hvm_tsc_scaling_supported (false)
> +#define hap_has_1gb (false)
> +#define hap_has_2mb (false)
> +
> +#define hvm_paging_enabled(v) (false)
> +#define hvm_nx_enabled(v) (false)
> +#define hvm_wp_enabled(v) (false)
> +#define hvm_smap_enabled(v) (false)
> +#define hvm_smep_enabled(v) (false)
> +#define hvm_pku_enabled(v) (false)
> +
> +#define arch_vcpu_block(v) ((void)v)
Parenthesization is odd here: Just like for hvm_enabled, false
and 0 don't need parentheses, yet arguments should (a) be
properly parenthesized when used and (b) be consistently
evaluated (or not).
> +int hvm_vcpu_initialise(struct vcpu *v);
> +void hvm_vcpu_destroy(struct vcpu *v);
> +int hvm_domain_initialise(struct domain *d);
> +void hvm_domain_destroy(struct domain *d);
> +void hvm_domain_soft_reset(struct domain *d);
> +void hvm_domain_relinquish_resources(struct domain *d);
> +uint64_t hvm_scale_tsc(const struct domain *d, uint64_t tsc);
> +uint64_t hvm_get_tsc_scaling_ratio(uint32_t gtsc_khz);
> +
> +void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg,
> + struct segment_register *reg);
> +
> +void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
> +void hvm_toggle_singlestep(struct vcpu *v);
> +void hvm_mapped_guest_frames_mark_dirty(struct domain *);
> +void hvm_hypercall_page_initialise(struct domain *d,
> + void *hypercall_page);
Many if not all of these already have declarations. They shouldn't
be repeated (and risk to go out of sync) in the #if and #else
sections of this conditional. All external declarations are fine to
remain visible regardless of CONFIG_HVM - the linker will complain
if any are referenced without there being a definition anywhere.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention
On 27/08/2018 15:15, Volodymyr Babchuk wrote:
Hi Julien,
Hi,
On 24.08.18 19:58, Julien Grall wrote:
Signed-off-by: Julien Grall
---
xen/arch/arm/psci.c | 4
xen/include/asm-arm/cpufeature.h | 3 ++-
xen/include/asm-arm/smccc.h | 8
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 3cf5ecf0f3..941eec921b 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -118,6 +119,9 @@ static void __init psci_init_smccc(void)
smccc_ver = ret;
}
+ if ( smccc_ver >= SMCCC_VERSION(1, 1) )
+ cpus_set_cap(ARM_SMCCC_1_1);
+
printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
SMCCC_VERSION_MAJOR(smccc_ver),
SMCCC_VERSION_MINOR(smccc_ver));
}
diff --git a/xen/include/asm-arm/cpufeature.h
b/xen/include/asm-arm/cpufeature.h
index 9c297c521c..c9c4046f5f 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -44,8 +44,9 @@
#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
#define ARM_HARDEN_BRANCH_PREDICTOR 7
#define ARM_SSBD 8
+#define ARM_SMCCC_1_1 9
-#define ARM_NCAPS 9
+#define ARM_NCAPS 10
#ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 1ed6cbaa48..7c39c530e2 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -213,6 +213,7 @@ struct arm_smccc_res {
*/
#ifdef CONFIG_ARM_32
#define arm_smccc_1_0_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
#else
void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
@@ -254,6 +255,13 @@ void __arm_smccc_1_0_smc(register_t a0,
register_t a1, register_t a2,
#define
arm_smccc_1_0_smc(...) \
__arm_smccc_1_0_smc_count(__count_args(__VA_ARGS__),
__VA_ARGS__)
+#define arm_smccc_smc(...) \
+ do { \
+ if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) \
+ arm_smccc_1_0_smc(__VA_ARGS__); \
+ else \
+ arm_smccc_1_1_smc(__VA_ARGS__); \
+ } while ( 0 )
#endif /* CONFIG_ARM_64 */
This will generate lots of code for every arm_smccc_smc(). Can we have
function pointer arm_smccc_smc instead and assign it to either
arm_smccc_1_1_smc() or arm_asmccc_1_0_smc() at boot?
I know that currently we have no function arm_smccc_1_1_smc() because it
is being constructed inline every time. But we can write it explicitly
and then have one indirect call to (maybe cached) function instead of
lots inlined code with conditional branches.
This is indeed increasing the size of the function, but with a better
performance when you perform an SMC with 1.1.
The main goal of SMCCC 1.1 is to limit the number of registers saved
when calling an SMC. So if you provide provide a wrapper using a
function, then you are better off sticking with SMCCC 1.0.
The idea of this code is to provide a faster call on the presence of
SMCCC 1.1 while providing a fallback for other case. The function
cpus_have_const_cap is implemented using an ALTERNATIVE (similar to
static key) that will make if close to a NOP. I am saying close because
this is not quite a static key as we don't have it in Xen. Instead, you
replace a memory load by a constant.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 12/23] x86: monitor.o is currently HVM only
On 8/27/18 6:18 PM, Jan Beulich wrote:
On 26.08.18 at 14:19, wrote:
>> There has been plan to make PV work, but it is not yet there. Provide
>> stubs to make it build with !CONFIG_HVM.
>>
>> Signed-off-by: Wei Liu
>> ---
>> xen/arch/x86/Makefile | 2 +-
>> xen/include/asm-x86/monitor.h | 14 ++
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> index 9b9b63a..43f9189 100644
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -45,7 +45,7 @@ obj-y += microcode_amd.o
>> obj-y += microcode_intel.o
>> obj-y += microcode.o
>> obj-y += mm.o x86_64/mm.o
>> -obj-y += monitor.o
>> +obj-$(CONFIG_HVM) += monitor.o
>> obj-y += mpparse.o
>> obj-y += nmi.o
>> obj-y += numa.o
>> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
>> index 4988903..09f7f8a 100644
>> --- a/xen/include/asm-x86/monitor.h
>> +++ b/xen/include/asm-x86/monitor.h
>> @@ -99,10 +99,24 @@ static inline uint32_t
>> arch_monitor_get_capabilities(struct domain *d)
>> int arch_monitor_domctl_event(struct domain *d,
>>struct xen_domctl_monitor_op *mop);
>>
>> +#ifdef CONFIG_HVM
>> +
>> int arch_monitor_init_domain(struct domain *d);
>>
>> void arch_monitor_cleanup_domain(struct domain *d);
>>
>> +#else
>> +
>> +static inline int arch_monitor_init_domain(struct domain *d)
>> +{
>> +return 0;
>> +}
>> +
>> +static inline void arch_monitor_cleanup_domain(struct domain *d)
>> +{}
>> +
>> +#endif
>
> Wouldn't the entire XEN_DOMCTL_VM_EVENT_OP_MONITOR case
> in vm_event_domctl() then better be put in an #ifdef instead?
That should be fine as well (perhaps even better than returning an error
here, since the switch would then just default with an error without the
duplicate function).
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/arm: smccc-1.1: Make return values unsigned long
On 27/08/2018 15:03, Volodymyr Babchuk wrote:
Hi Julien, Marc
Hi,
On 24.08.18 19:58, Julien Grall wrote:
From: Marc Zyngier
An unfortunate consequence of having a strong typing for the input
values to the SMC call is that it also affects the type of the
return values, limiting r0 to 32 bits and r{1,2,3} to whatever
was passed as an input. >
Let's turn everything into "unsigned long", which satisfies the
requirements of both architectures, and allows for the full
range of return values.
Maybe it better to use register_t then? By definition register_t has the
same size as a CPU register and SMC uses CPU registers to pass
parameters/return values.
The code is based on Linux series (posted last Friday). I don't much
want to diverge too much from it. So unsigned "unsigned long" here is ok.
Reported-by: Stefano Stabellini
Signed-off-by: Marc Zyngier
Signed-off-by: Julien Grall
---
xen/include/asm-arm/smccc.h | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 74c13f8419..a31d67a1de 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -119,35 +119,35 @@ struct arm_smccc_res {
#define __declare_arg_0(a0, res) \
struct arm_smccc_res *___res = res; \
- register uin32_t r0 asm("r0") = a0; \
+ register unsigned long r0 asm("r0") = (uint32_t)a0;\
Do you really want to silently drop upper 32 bits of the argument?
I know, that SMCCC states that function id is a 32-bit value,
but I don't think that it is a good place to enforce this
behavior.
I think it is better to allow user to shoot in his leg in this case.
I don't see any issue with casting here. This is what the SMCCC request
for x0 and after all you silently drop upper 32-bit when using a static
inline function...
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 12/23] x86: monitor.o is currently HVM only
>>> On 26.08.18 at 14:19, wrote:
> There has been plan to make PV work, but it is not yet there. Provide
> stubs to make it build with !CONFIG_HVM.
>
> Signed-off-by: Wei Liu
> ---
> xen/arch/x86/Makefile | 2 +-
> xen/include/asm-x86/monitor.h | 14 ++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 9b9b63a..43f9189 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -45,7 +45,7 @@ obj-y += microcode_amd.o
> obj-y += microcode_intel.o
> obj-y += microcode.o
> obj-y += mm.o x86_64/mm.o
> -obj-y += monitor.o
> +obj-$(CONFIG_HVM) += monitor.o
> obj-y += mpparse.o
> obj-y += nmi.o
> obj-y += numa.o
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 4988903..09f7f8a 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -99,10 +99,24 @@ static inline uint32_t
> arch_monitor_get_capabilities(struct domain *d)
> int arch_monitor_domctl_event(struct domain *d,
>struct xen_domctl_monitor_op *mop);
>
> +#ifdef CONFIG_HVM
> +
> int arch_monitor_init_domain(struct domain *d);
>
> void arch_monitor_cleanup_domain(struct domain *d);
>
> +#else
> +
> +static inline int arch_monitor_init_domain(struct domain *d)
> +{
> +return 0;
> +}
> +
> +static inline void arch_monitor_cleanup_domain(struct domain *d)
> +{}
> +
> +#endif
Wouldn't the entire XEN_DOMCTL_VM_EVENT_OP_MONITOR case
in vm_event_domctl() then better be put in an #ifdef instead?
Otherwise, style wise I think I sort of agree with Razvan: We don't
seem to have any examples of {} together on their own line. We
do have example though with them together at the end of the
preceding line, which I think is better than having { and } separately
on their own lines.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 11/23] x86: XENMEM_resource_ioreq_server is HVM only
>>> On 26.08.18 at 14:19, wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4376,12 +4376,17 @@ int arch_acquire_resource(struct domain *d, unsigned
> int type,
>
> switch ( type )
> {
> +#ifdef CONFIG_HVM
> case XENMEM_resource_ioreq_server:
> {
> ioservid_t ioservid = id;
> unsigned int i;
>
> rc = -EINVAL;
> +if ( !is_hvm_domain(d) )
> +break;
> +
> +rc = -EINVAL;
> if ( id != (unsigned int)ioservid )
> break;
Since this is the only caller of hvm_get_ioreq_server_frame(),
adding an is_hvm_domain() check here means the one inside
the function becomes redundant - it should be dropped or
converted to an ASSERT() imo. Furthermore if the check is to
remain here, please drop the redundant assignment of rc.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 09/23] x86/pt: make it build with !CONFIG_HVM
>>> On 26.08.18 at 14:19, wrote: > This requires providing stubs for a few functions which are part of > HVM code. > > Signed-off-by: Wei Liu Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 08/23] xen/pt: io.c contains HVM only code
>>> On 26.08.18 at 14:19, wrote: > We also need to make it x86 only because ARM also defines CONFIG_HVM. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/23] x86/vpmu: put HVM only code under CONFIG_HVM
>>> On 26.08.18 at 14:19, wrote: > Change u32 to uint32_t while at it. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 126682: regressions - FAIL
flight 126682 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/126682/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-rumprun-i386 12 guest-start fail REGR. vs. 125898 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 125898 test-amd64-amd64-xl-shadow 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-rumprun-amd64 12 guest-startfail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-xl 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-shadow12 guest-start fail REGR. vs. 125898 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-i386-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl 12 guest-start fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-amd64-xl-credit2 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-amd64-xl-xsm 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-pair21 guest-start/debian fail REGR. vs. 125898 test-amd64-amd64-xl-pvshim 12 guest-start fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail REGR. vs. 125898 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 125898 test-amd64-amd64-amd64-pvgrub 10 debian-di-install fail REGR. vs. 125898 test-amd64-i386-xl-raw 10 debian-di-installfail REGR. vs. 125898 test-amd64-amd64-i386-pvgrub 10 debian-di-installfail REGR. vs. 125898 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win7-amd64 10 windows-install fail REGR. vs.
Re: [Xen-devel] [PATCH v2 06/23] x86: don't call vpci function in physdev_op when !CONFIG_HAS_VPCI
>>> On 26.08.18 at 14:19, wrote:
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -557,6 +557,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
>
> ret = pci_mmcfg_reserved(info.address, info.segment,
> info.start_bus, info.end_bus, info.flags);
> +#ifdef CONFIG_HAS_VPCI
> if ( !ret && has_vpci(currd) )
> {
> /*
> @@ -567,6 +568,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
>info.start_bus, info.end_bus,
>info.segment);
> }
> +#endif
Perhaps better to make has_vpci() evaluate to false in that case,
and once again rely on DCE?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/23] x86: provide stub for memory_type_changed
>>> On 26.08.18 at 14:19, wrote: > Jan indicated that for PV guests the memory type is not changed, for > HVM guests memory_type_changed is needed for EPT's effective memory > type calculation. This means memory_type_changed is HVM only. > > Provide a stub to minimise code churn. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 04/23] x86/hvm: provide hvm_hap_supported
>>> On 26.08.18 at 14:19, wrote: > And replace direct accesses in non-HVM subsystems to > hvm_funcs.hap_supported with the new function, to avoid accessing an > internal data structure of another subsystem directly. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 02/23] xen: is_hvm_{domain, vcpu} should evaluate to false when !CONFIG_HVM
>>> On 26.08.18 at 14:19, wrote:
> Turn them into static inline functions which evaluate to false when
> CONFIG_HVM is not set. ARM won't be broken because ARM guests are set
> to PV type in the hypervisor.
>
> But ARM has plan to switch to HVM guest type inside the hypervisor, so
> preemptively introduce CONFIG_HVM for ARM here.
But is setting this to Y then correct at this point?
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -879,8 +879,17 @@ void watchdog_domain_destroy(struct domain *d);
>
> #define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
> #define is_pv_vcpu(v) (is_pv_domain((v)->domain))
> -#define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
> -#define is_hvm_vcpu(v) (is_hvm_domain(v->domain))
> +
> +static inline bool is_hvm_domain(const struct domain *d)
> +{
> +return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
Ultimately I think the guest_type_{hvm,pv} enumerators should only
be enabled when CONFIG_{HVM,PV}, so I think #ifdef CONFIG_HVM
would be the better choice here. But of course we can do this further
conversion at a later point, so
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/23] x86: enclose hvm_op and dm_op in CONFIG_HVM in relevant tables
>>> On 26.08.18 at 14:19, wrote: > PV guest (Dom0) needs to able to use these two hypercalls in order to > serve HVM guests. But if xen doesn't support HVM at all there is no > point in exposing them to PV guests. > > Signed-off-by: Wei Liu Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 01/23] x86: change name of parameter for various invlpg functions
>>> On 26.08.18 at 14:19, wrote: > They all incorrectly named a parameter virtual address while it should > have been linear address. > > Requested-by: Andrew Cooper > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention
Hi Julien,
On 24.08.18 19:58, Julien Grall wrote:
Signed-off-by: Julien Grall
---
xen/arch/arm/psci.c | 4
xen/include/asm-arm/cpufeature.h | 3 ++-
xen/include/asm-arm/smccc.h | 8
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 3cf5ecf0f3..941eec921b 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -118,6 +119,9 @@ static void __init psci_init_smccc(void)
smccc_ver = ret;
}
+if ( smccc_ver >= SMCCC_VERSION(1, 1) )
+cpus_set_cap(ARM_SMCCC_1_1);
+
printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
}
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 9c297c521c..c9c4046f5f 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -44,8 +44,9 @@
#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
#define ARM_HARDEN_BRANCH_PREDICTOR 7
#define ARM_SSBD 8
+#define ARM_SMCCC_1_1 9
-#define ARM_NCAPS 9
+#define ARM_NCAPS 10
#ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 1ed6cbaa48..7c39c530e2 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -213,6 +213,7 @@ struct arm_smccc_res {
*/
#ifdef CONFIG_ARM_32
#define arm_smccc_1_0_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
#else
void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
@@ -254,6 +255,13 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1,
register_t a2,
#define arm_smccc_1_0_smc(...) \
__arm_smccc_1_0_smc_count(__count_args(__VA_ARGS__), __VA_ARGS__)
+#define arm_smccc_smc(...) \
+do {\
+if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) \
+arm_smccc_1_0_smc(__VA_ARGS__); \
+else\
+arm_smccc_1_1_smc(__VA_ARGS__); \
+} while ( 0 )
#endif /* CONFIG_ARM_64 */
This will generate lots of code for every arm_smccc_smc(). Can we have
function pointer arm_smccc_smc instead and assign it to either
arm_smccc_1_1_smc() or arm_asmccc_1_0_smc() at boot?
I know that currently we have no function arm_smccc_1_1_smc() because it
is being constructed inline every time. But we can write it explicitly
and then have one indirect call to (maybe cached) function instead of
lots inlined code with conditional branches.
#endif /* __ASSEMBLY__ */
--
Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
>>> On 27.08.18 at 15:47, wrote: > On 8/27/18 4:17 PM, Jan Beulich wrote: > On 27.08.18 at 15:02, wrote: >>> This should be architecturally correct as it is exclusively derived from >>> information provided by the VMExit, and won't cause dirty bits to be >>> written in cases where the hardware wouldn't have written them >>> (speculative or otherwise). It does mean that an instruction which >>> would need to set A and D bits in the walk will take two EPT violations >>> to achieve the end result, but it probably is still quicker than sending >>> the vm_event out. >> >> I'm afraid this is going to be only mostly correct: Atomicity of the page >> table write is going to be lost. This could become an actual problem if >> the guest used racing PTE accesses. Such racing accesses might not >> be a bug - simply consider the OS scanning for set A and/or D bits >> (and clearing them when they're set). Or an entity temporarily clearing >> (parts of) PTEs, with recovery logic in place to restore them when >> needed for a synchronous access. At the very least there's then the >> risk of a live lock within the guest. > > But it's not clear to me why that can't already happen when just > emulating the current instruction (as we do now), if emulating said > instruction would set A or D? Yes, good point - this is a problem not just to the new handling you propose. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
>>> On 27.08.18 at 15:24, wrote: > On 8/27/18 4:02 PM, Andrew Cooper wrote: >> On 27/08/18 13:53, Razvan Cojocaru wrote: >>> On 8/27/18 3:37 PM, Andrew Cooper wrote: On 27/08/18 13:12, Jan Beulich wrote: >>> For NPT, isn't there an error code bit telling you whether the >>> request was a user or "system" one? If not, some cheating >>> would be needed (derive from CPL, accepting that e.g. >>> descriptor table accesses would get mis-attributed), but >>> that's still not going to involve looking at the PTE flags. >> The alternative would be to simply walk (without enforcing any flags, >> and so making the pfec walk parameter unnecessary) to the respective >> address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. >> >> If _PAGE_ACCESSED is not set, set it and exit. >> If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. > Since it's ambiguous in the NPT case - are you talking about > setting the flags in the guest or host page tables? The > former, I'm afraid, might not be acceptable (as not always > being architecturally correct). In anyway feels as if we'd > been here before, in that this reminds me of you meaning > to imply from a second walk (with A already set) that it must > be a write access. I thought we had settled on such an > implication not being generally correct. The problem that is trying to be solved is that when operating in non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a write-protected EPT page, takes an EPT violation for a write to a read-only page. Manually setting the A/D bits (as appropriate) and restarting the instruction is sufficient for it to complete correctly. At the moment, every time this happens, a request is sent to the introspection agent, and the agent calculates that it was due to pagetable protection, and instructs Xen to emulate the instruction. This accounts for 97% (?) of the VMExits, and is unrelated to any of the real protections which introspection is trying to achieve. What Razvan is looking to do is to have Xen skip the "send to the introspection agent" part as an optimisation, because hardware tells Xen (as part of the VMExit) when this condition has occurred, and the vm_event logic has already asked Xen to try and fix up this condition automatically. What can actually be done depends on how A/D bits behave in real hardware. Setting access bits for non-leaf entries is definitely fine, and speculatively setting the access bit is also explicitly permitted by the spec. However, I can't find any comment on speculative dirty bits (from either Intel or AMD), and I've not encountered such a behaviour with the pagetable work I've been doing. >>> Thanks for the reply! >>> >>> I've forgotten a piece of information that I really should have written >>> here: we would only set the D bit if A is already set and either the >>> page is writable (has _PAGE_RW set) or CR0.WP is 0 (the latter case is >>> admittedly more tricky). >> >> How about a new function which works similarly to guest-walk-tables, but >> only ever sets A/D bits. >> >> Given information from hardware, we know the linear address, and that it >> was a problem with the guest pagetables, from which we explicitly know >> that it was from writing an A/D bit to a guest PTE. >> >> While walking down the levels, set any missing A bits and remember if we >> set any. If we set A bits, consider ourselves complete and exit back to >> the guest. If no A bits were set, and the access was a write (which we >> know from the EPT violation information), then set the leaf D bit. >> >> This should be architecturally correct as it is exclusively derived from >> information provided by the VMExit, and won't cause dirty bits to be >> written in cases where the hardware wouldn't have written them >> (speculative or otherwise). It does mean that an instruction which >> would need to set A and D bits in the walk will take two EPT violations >> to achieve the end result, but it probably is still quicker than sending >> the vm_event out. > > Right, that's pretty much what we were proposing, a basic algoritm of: > > if ((pte & A) && (pte & RW)) pte |= D; > pte |= A; > > where the if probably becomes: > > if ((pte & A) && ((pte & RW) || CR0.WP == 0)) pte |= D; > pte |= A > > for the CR0.WP case. Except that it's not quite this simple, as Andrew has already explained: You need to check all levels' A bits, and only if they've all been set you'd want to set the leaf level D bit. Then again, looking at the A bits as all levels should not be necessary - you know the guest physical address for which the exit occurred, and hence you can tell at least which page the bit of interest lives in. For A bits this may be any page table touched during the walk, while for D bits this should only
Re: [Xen-devel] [PATCH 2/6] xen/arm: smccc-1.1: Handle function result as parameters
Hi,
On 24.08.18 19:58, Julien Grall wrote:
From: Marc Zyngier
If someone has the silly idea to write something along those lines:
extern u64 foo(void);
void bar(struct arm_smccc_res *res)
{
arm_smccc_1_1_smc(0xbad, foo(), res);
}
they are in for a surprise, as this gets compiled as:
0588 :
588: a9be7bfdstp x29, x30, [sp, #-32]!
58c: 910003fdmov x29, sp
590: f9000bf3str x19, [sp, #16]
594: aa0003f3mov x19, x0
598: aa1e03e0mov x0, x30
59c: 9400bl 0 <_mcount>
5a0: 9400bl 0
5a4: aa0003e1mov x1, x0
5a8: d403smc #0x0
5ac: b473cbz x19, 5b8
5b0: a9000660stp x0, x1, [x19]
5b4: a9010e62stp x2, x3, [x19, #16]
5b8: f9400bf3ldr x19, [sp, #16]
5bc: a8c27bfdldp x29, x30, [sp], #32
5c0: d65f03c0ret
5c4: d503201fnop
The call to foo "overwrites" the x0 register for the return value,
and we end up calling the wrong secure service.
A solution is to evaluate all the parameters before assigning
anything to specific registers, leading to the expected result:
0588 :
588: a9be7bfdstp x29, x30, [sp, #-32]!
58c: 910003fdmov x29, sp
590: f9000bf3str x19, [sp, #16]
594: aa0003f3mov x19, x0
598: aa1e03e0mov x0, x30
59c: 9400bl 0 <_mcount>
5a0: 9400bl 0
5a4: aa0003e1mov x1, x0
5a8: d28175a0mov x0, #0xbad
5ac: d403smc #0x0
5b0: b473cbz x19, 5bc
5b4: a9000660stp x0, x1, [x19]
5b8: a9010e62stp x2, x3, [x19, #16]
5bc: f9400bf3ldr x19, [sp, #16]
5c0: a8c27bfdldp x29, x30, [sp], #32
5c4: d65f03c0ret
Reported-by: Stefano Stabellini
Signed-off-by: Marc Zyngier
Reviewed-by: Volodymyr Babchuk
---
xen/include/asm-arm/smccc.h | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index a31d67a1de..648bef28bd 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -125,41 +125,51 @@ struct arm_smccc_res {
register unsigned long r3 asm("r3")
#define __declare_arg_1(a0, a1, res)\
+typeof(a1) __a1 = a1; \
struct arm_smccc_res*___res = res; \
register unsigned long r0 asm("r0") = (uint32_t)a0;\
-register unsigned long r1 asm("r1") = a1; \
+register unsigned long r1 asm("r1") = __a1;\
register unsigned long r2 asm("r2"); \
register unsigned long r3 asm("r3")
#define __declare_arg_2(a0, a1, a2, res)\
+typeof(a1) __a1 = a1; \
+typeof(a2) __a2 = a2; \
struct arm_smccc_res*___res = res;\
register unsigned long r0 asm("r0") = (uint32_t)a0;\
-register unsigned long r1 asm("r1") = a1; \
-register unsigned long r2 asm("r2") = a2; \
+register unsigned long r1 asm("r1") = __a1;\
+register unsigned long r2 asm("r2") = __a2;\
register unsigned long r3 asm("r3")
#define __declare_arg_3(a0, a1, a2, a3, res)\
+typeof(a1) __a1 = a1; \
+typeof(a2) __a2 = a2; \
+typeof(a3) __a3 = a3; \
struct arm_smccc_res*___res = res; \
register unsigned long r0 asm("r0") = (uint32_t)a0;\
-register unsigned long r1 asm("r1") = a1; \
-register unsigned long r2 asm("r2") = a2; \
-register unsigned long r3 asm("r3") = a3
+register unsigned long r1 asm("r1") = __a1;\
+register unsigned long r2 asm("r2") = __a2;\
+register unsigned long r3 asm("r3") = __a3
#define __declare_arg_4(a0, a1, a2, a3, a4, res)\
+typeof(a4) __a4 = a4; \
__declare_arg_3(a0, a1, a2, a3, res); \
-register unsigned long r4 asm("r4") = a4
+register unsigned long r4 asm("r4") = __a4
#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)\
+typeof(a5) __a5 = a5; \
__declare_arg_4(a0, a1, a2, a3, a4, res); \
-register typeof(a5) r5 asm("r5") = a5
+
Re: [Xen-devel] [PATCH 1/6] xen/arm: smccc-1.1: Make return values unsigned long
Hi Julien, Marc
On 24.08.18 19:58, Julien Grall wrote:
From: Marc Zyngier
An unfortunate consequence of having a strong typing for the input
values to the SMC call is that it also affects the type of the
return values, limiting r0 to 32 bits and r{1,2,3} to whatever
was passed as an input. >
Let's turn everything into "unsigned long", which satisfies the
requirements of both architectures, and allows for the full
range of return values.
Maybe it better to use register_t then? By definition register_t has the
same size as a CPU register and SMC uses CPU registers to pass
parameters/return values.
Reported-by: Stefano Stabellini
Signed-off-by: Marc Zyngier
Signed-off-by: Julien Grall
---
xen/include/asm-arm/smccc.h | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 74c13f8419..a31d67a1de 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -119,35 +119,35 @@ struct arm_smccc_res {
#define __declare_arg_0(a0, res)\
struct arm_smccc_res*___res = res; \
-register uin32_tr0 asm("r0") = a0; \
+register unsigned long r0 asm("r0") = (uint32_t)a0;\
Do you really want to silently drop upper 32 bits of the argument?
I know, that SMCCC states that function id is a 32-bit value,
but I don't think that it is a good place to enforce this
behavior.
I think it is better to allow user to shoot in his leg in this case.
register unsigned long r1 asm("r1"); \
register unsigned long r2 asm("r2"); \
register unsigned long r3 asm("r3")
#define __declare_arg_1(a0, a1, res)\
struct arm_smccc_res*___res = res; \
-register uint32_t r0 asm("r0") = a0; \
-register typeof(a1) r1 asm("r1") = a1; \
+register unsigned long r0 asm("r0") = (uint32_t)a0;\
+register unsigned long r1 asm("r1") = a1; \
register unsigned long r2 asm("r2"); \
register unsigned long r3 asm("r3")
#define __declare_arg_2(a0, a1, a2, res)\
struct arm_smccc_res*___res = res;\
-register u32r0 asm("r0") = a0; \
-register typeof(a1) r1 asm("r1") = a1; \
-register typeof(a2) r2 asm("r2") = a2; \
+register unsigned long r0 asm("r0") = (uint32_t)a0;\
+register unsigned long r1 asm("r1") = a1; \
+register unsigned long r2 asm("r2") = a2; \
register unsigned long r3 asm("r3")
#define __declare_arg_3(a0, a1, a2, a3, res)\
struct arm_smccc_res*___res = res; \
-register u32r0 asm("r0") = a0; \
-register typeof(a1) r1 asm("r1") = a1; \
-register typeof(a2) r2 asm("r2") = a2; \
-register typeof(a3) r3 asm("r3") = a3
+register unsigned long r0 asm("r0") = (uint32_t)a0;\
+register unsigned long r1 asm("r1") = a1; \
+register unsigned long r2 asm("r2") = a2; \
+register unsigned long r3 asm("r3") = a3
#define __declare_arg_4(a0, a1, a2, a3, a4, res)\
__declare_arg_3(a0, a1, a2, a3, res); \
-register typeof(a4) r4 asm("r4") = a4
+register unsigned long r4 asm("r4") = a4
#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)\
__declare_arg_4(a0, a1, a2, a3, a4, res); \
--
Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 01/23] x86: change name of parameter for various invlpg functions
>>> On 27.08.18 at 15:49, wrote: > On 08/26/2018 08:19 AM, Wei Liu wrote: >> They all incorrectly named a parameter virtual address while it should >> have been linear address. >> >> Requested-by: Andrew Cooper > > AMD SDM doesn't make distinction between linear and virtual addresses so > I wonder why this was requested. Because it is incorrect to consider them the same: A virtual address is a segment:offset pair, while a linear address is a plain number (typically but not always resulting from adding segment base and offset). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] xen/arm: Replace call_smc with arm_smccc_smc
Hi Julien,
On 24.08.18 19:58, Julien Grall wrote:
call_smc is a subset of arm_smccc_smc. Rather than having 2 methods to
do SMCCC call, replace all call to the former by the later.
Signed-off-by: Julien Grall
---
xen/arch/arm/Makefile| 1 -
xen/arch/arm/platforms/exynos5.c | 3 ++-
xen/arch/arm/platforms/seattle.c | 4 ++--
xen/arch/arm/psci.c | 37 +
xen/arch/arm/smc.S | 21 -
xen/include/asm-arm/processor.h | 3 ---
6 files changed, 29 insertions(+), 40 deletions(-)
delete mode 100644 xen/arch/arm/smc.S
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b9b141dc84..37fa8268b3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -39,7 +39,6 @@ obj-y += processor.o
obj-y += psci.o
obj-y += setup.o
obj-y += shutdown.o
-obj-y += smc.o
obj-y += smp.o
obj-y += smpboot.o
obj-y += sysctl.o
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index c15ecf80f5..e2c0b7b878 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
static bool secure_firmware;
@@ -249,7 +250,7 @@ static int exynos5_cpu_up(int cpu)
iounmap(power);
if ( secure_firmware )
-call_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
+arm_smccc_smc(SMC_CMD_CPU1BOOT, cpu, NULL);
return cpu_up_send_sgi(cpu);
}
diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
index 893cc17972..64cc1868c2 100644
--- a/xen/arch/arm/platforms/seattle.c
+++ b/xen/arch/arm/platforms/seattle.c
@@ -33,12 +33,12 @@ static const char * const seattle_dt_compat[] __initconst =
*/
static void seattle_system_reset(void)
{
-call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
+arm_smccc_smc(PSCI_0_2_FN32_SYSTEM_RESET, NULL);
}
static void seattle_system_off(void)
{
-call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
+arm_smccc_smc(PSCI_0_2_FN32_SYSTEM_OFF, NULL);
}
PLATFORM_START(seattle, "SEATTLE")
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 941eec921b..02737e6caa 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -42,42 +42,53 @@ uint32_t smccc_ver;
static uint32_t psci_cpu_on_nr;
+#define PSCI_RET(res) ((int32_t)(res).a0)
Do we really need this macro?
+
int call_psci_cpu_on(int cpu)
{
-return call_smc(psci_cpu_on_nr, cpu_logical_map(cpu),
__pa(init_secondary), 0);
+struct arm_smccc_res res;
+
+arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary),
+ );
+
+return (int32_t)res.a0;
If you still want to introduce it, then there should be
+return PSCI_RET(res);
}
void call_psci_cpu_off(void)
{
if ( psci_ver > PSCI_VERSION(0, 1) )
{
-int errno;
+struct arm_smccc_res res;
/* If successfull the PSCI cpu_off call doesn't return */
-errno = call_smc(PSCI_0_2_FN32_CPU_OFF, 0, 0, 0);
+arm_smccc_smc(PSCI_0_2_FN32_CPU_OFF, );
panic("PSCI cpu off failed for CPU%d err=%d\n", smp_processor_id(),
- errno);
+ PSCI_RET(res));
}
}
void call_psci_system_off(void)
{
if ( psci_ver > PSCI_VERSION(0, 1) )
-call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
+arm_smccc_smc(PSCI_0_2_FN32_SYSTEM_OFF, NULL);
}
void call_psci_system_reset(void)
{
if ( psci_ver > PSCI_VERSION(0, 1) )
-call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
+arm_smccc_smc(PSCI_0_2_FN32_SYSTEM_RESET, NULL);
}
static int __init psci_features(uint32_t psci_func_id)
{
+struct arm_smccc_res res;
+
if ( psci_ver < PSCI_VERSION(1, 0) )
return PSCI_NOT_SUPPORTED;
-return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
+arm_smccc_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, NULL);
+
+return PSCI_RET(res);
}
static int __init psci_is_smc_method(const struct dt_device_node *psci)
@@ -112,11 +123,11 @@ static void __init psci_init_smccc(void)
if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED )
{
-uint32_t ret;
+struct arm_smccc_res res;
-ret = call_smc(ARM_SMCCC_VERSION_FID, 0, 0, 0);
-if ( ret != ARM_SMCCC_NOT_SUPPORTED )
-smccc_ver = ret;
+arm_smccc_smc(ARM_SMCCC_VERSION_FID, );
+if ( PSCI_RET(res) != ARM_SMCCC_NOT_SUPPORTED )
+smccc_ver = PSCI_RET(res);
}
if ( smccc_ver >= SMCCC_VERSION(1, 1) )
@@ -165,6 +176,7 @@ static int __init psci_init_0_2(void)
{ /* sentinel */ },
};
int ret;
+struct arm_smccc_res res;
if ( acpi_disabled )
{
@@ -186,7 +198,8 @@ static int __init psci_init_0_2(void)
}
}
-psci_ver = call_smc(PSCI_0_2_FN32_PSCI_VERSION, 0, 0, 0);
+
[Xen-devel] [xen-unstable test] 126683: tolerable FAIL
flight 126683 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/126683/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-rumprun-i386 17 rumprun-demo-xenstorels/xenstorels.repeat fail in 126564 pass in 126683 test-amd64-amd64-examine 4 memdisk-try-append fail pass in 126564 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 126564 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 126564 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 126564 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 126564 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 126564 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 126564 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 126564 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 126564 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 126564 build-amd64-xen-xsm-freebsd 7 xen-build-freebsdfail never pass build-amd64-xen-freebsd 7 xen-build-freebsdfail never pass test-amd64-i386-xl-pvshim12 guest-start fail 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-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-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-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 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-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-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-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-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen a923919797c39d51ea0b808ea691bed20fe8e072 baseline version: xen a923919797c39d51ea0b808ea691bed20fe8e072 Last test of basis 126683 2018-08-26 06:47:14 Z1 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf pass build-amd64-xen-freebsd fail build-amd64-xen-xsm-freebsd fail build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-prev pass build-i386-prev pass build-amd64-pvops
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 8/27/18 4:17 PM, Jan Beulich wrote: On 27.08.18 at 15:02, wrote: >> This should be architecturally correct as it is exclusively derived from >> information provided by the VMExit, and won't cause dirty bits to be >> written in cases where the hardware wouldn't have written them >> (speculative or otherwise). It does mean that an instruction which >> would need to set A and D bits in the walk will take two EPT violations >> to achieve the end result, but it probably is still quicker than sending >> the vm_event out. > > I'm afraid this is going to be only mostly correct: Atomicity of the page > table write is going to be lost. This could become an actual problem if > the guest used racing PTE accesses. Such racing accesses might not > be a bug - simply consider the OS scanning for set A and/or D bits > (and clearing them when they're set). Or an entity temporarily clearing > (parts of) PTEs, with recovery logic in place to restore them when > needed for a synchronous access. At the very least there's then the > risk of a live lock within the guest. But it's not clear to me why that can't already happen when just emulating the current instruction (as we do now), if emulating said instruction would set A or D? Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 01/23] x86: change name of parameter for various invlpg functions
On 08/26/2018 08:19 AM, Wei Liu wrote: > They all incorrectly named a parameter virtual address while it should > have been linear address. > > Requested-by: Andrew Cooper AMD SDM doesn't make distinction between linear and virtual addresses so I wonder why this was requested. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 8/27/18 4:02 PM, Andrew Cooper wrote: > On 27/08/18 13:53, Razvan Cojocaru wrote: >> On 8/27/18 3:37 PM, Andrew Cooper wrote: >>> On 27/08/18 13:12, Jan Beulich wrote: >> For NPT, isn't there an error code bit telling you whether the >> request was a user or "system" one? If not, some cheating >> would be needed (derive from CPL, accepting that e.g. >> descriptor table accesses would get mis-attributed), but >> that's still not going to involve looking at the PTE flags. > The alternative would be to simply walk (without enforcing any flags, > and so making the pfec walk parameter unnecessary) to the respective > address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. > > If _PAGE_ACCESSED is not set, set it and exit. > If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. Since it's ambiguous in the NPT case - are you talking about setting the flags in the guest or host page tables? The former, I'm afraid, might not be acceptable (as not always being architecturally correct). In anyway feels as if we'd been here before, in that this reminds me of you meaning to imply from a second walk (with A already set) that it must be a write access. I thought we had settled on such an implication not being generally correct. >>> The problem that is trying to be solved is that when operating in >>> non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a >>> write-protected EPT page, takes an EPT violation for a write to a >>> read-only page. >>> >>> Manually setting the A/D bits (as appropriate) and restarting the >>> instruction is sufficient for it to complete correctly. >>> >>> At the moment, every time this happens, a request is sent to the >>> introspection agent, and the agent calculates that it was due to >>> pagetable protection, and instructs Xen to emulate the instruction. >>> This accounts for 97% (?) of the VMExits, and is unrelated to any of the >>> real protections which introspection is trying to achieve. >>> >>> What Razvan is looking to do is to have Xen skip the "send to the >>> introspection agent" part as an optimisation, because hardware tells Xen >>> (as part of the VMExit) when this condition has occurred, and the >>> vm_event logic has already asked Xen to try and fix up this condition >>> automatically. >>> >>> What can actually be done depends on how A/D bits behave in real hardware. >>> >>> Setting access bits for non-leaf entries is definitely fine, and >>> speculatively setting the access bit is also explicitly permitted by the >>> spec. However, I can't find any comment on speculative dirty bits (from >>> either Intel or AMD), and I've not encountered such a behaviour with the >>> pagetable work I've been doing. >> Thanks for the reply! >> >> I've forgotten a piece of information that I really should have written >> here: we would only set the D bit if A is already set and either the >> page is writable (has _PAGE_RW set) or CR0.WP is 0 (the latter case is >> admittedly more tricky). > > How about a new function which works similarly to guest-walk-tables, but > only ever sets A/D bits. > > Given information from hardware, we know the linear address, and that it > was a problem with the guest pagetables, from which we explicitly know > that it was from writing an A/D bit to a guest PTE. > > While walking down the levels, set any missing A bits and remember if we > set any. If we set A bits, consider ourselves complete and exit back to > the guest. If no A bits were set, and the access was a write (which we > know from the EPT violation information), then set the leaf D bit. > > This should be architecturally correct as it is exclusively derived from > information provided by the VMExit, and won't cause dirty bits to be > written in cases where the hardware wouldn't have written them > (speculative or otherwise). It does mean that an instruction which > would need to set A and D bits in the walk will take two EPT violations > to achieve the end result, but it probably is still quicker than sending > the vm_event out. Right, that's pretty much what we were proposing, a basic algoritm of: if ((pte & A) && (pte & RW)) pte |= D; pte |= A; where the if probably becomes: if ((pte & A) && ((pte & RW) || CR0.WP == 0)) pte |= D; pte |= A for the CR0.WP case. As discussed privately, there's also the case where two VCPUs may try to set A concurrently, which is what I assume is the case Jan has hinted at. Another small issue is that we do need to ignore the EPT violation information as it pertains to reads or writes: that will always be the page-walk access type, rw. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/hvm: remove default ioreq server
>>> On 07.08.18 at 17:42, wrote: > My recent patch [1] to qemu-xen-traditional removes the last use of the > 'default' ioreq server in Xen. (This is a catch-all ioreq server that is > used if no explicitly registered I/O range is targetted). > > This patch can be applied once that patch is committed, to remove the > (>100 lines of) redundant code in Xen. > > NOTE: The removal of the special case for HVM_PARAM_DM_DOMAIN in > hvm_allow_set_param() is not directly related to removal of > default ioreq servers. It could have been cleaned up at any time > after commit 9a422c03 "x86/hvm: stop passing explicit domid to > hvm_create_ioreq_server()". It is now added to the new > deprecated sets introduced by this patch. > > [1] > https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html > > Signed-off-by: Paul Durrant > Acked-by: Andrew Cooper I'm afraid this change is responsible for osstest finding In file included from /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/include/hw/xen/xen_backend.h:4:0, from /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/hw/block/xen_disk.c:28: /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/include/hw/xen/xen_common.h: In function 'xen_get_default_ioreq_server_info': /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/include/hw/xen/xen_common.h:412:40: error: 'HVM_PARAM_BUFIOREQ_EVTCHN' undeclared (first use in this function) rc = xc_get_hvm_param(xen_xc, dom, HVM_PARAM_BUFIOREQ_EVTCHN, ^ /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/include/hw/xen/xen_common.h:412:40: note: each undeclared identifier is reported only once for each function it appears in /home/osstest/build.126771.build-amd64/xen/tools/qemu-xen-dir/rules.mak:69: recipe for target 'hw/block/xen_disk.o' failed make: *** [hw/block/xen_disk.o] Error 1 I guess I'm going to revert it for now. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
>>> On 27.08.18 at 15:02, wrote: > On 27/08/18 13:53, Razvan Cojocaru wrote: >> On 8/27/18 3:37 PM, Andrew Cooper wrote: >>> On 27/08/18 13:12, Jan Beulich wrote: >> For NPT, isn't there an error code bit telling you whether the >> request was a user or "system" one? If not, some cheating >> would be needed (derive from CPL, accepting that e.g. >> descriptor table accesses would get mis-attributed), but >> that's still not going to involve looking at the PTE flags. > The alternative would be to simply walk (without enforcing any flags, > and so making the pfec walk parameter unnecessary) to the respective > address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. > > If _PAGE_ACCESSED is not set, set it and exit. > If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. Since it's ambiguous in the NPT case - are you talking about setting the flags in the guest or host page tables? The former, I'm afraid, might not be acceptable (as not always being architecturally correct). In anyway feels as if we'd been here before, in that this reminds me of you meaning to imply from a second walk (with A already set) that it must be a write access. I thought we had settled on such an implication not being generally correct. >>> The problem that is trying to be solved is that when operating in >>> non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a >>> write-protected EPT page, takes an EPT violation for a write to a >>> read-only page. >>> >>> Manually setting the A/D bits (as appropriate) and restarting the >>> instruction is sufficient for it to complete correctly. >>> >>> At the moment, every time this happens, a request is sent to the >>> introspection agent, and the agent calculates that it was due to >>> pagetable protection, and instructs Xen to emulate the instruction. >>> This accounts for 97% (?) of the VMExits, and is unrelated to any of the >>> real protections which introspection is trying to achieve. >>> >>> What Razvan is looking to do is to have Xen skip the "send to the >>> introspection agent" part as an optimisation, because hardware tells Xen >>> (as part of the VMExit) when this condition has occurred, and the >>> vm_event logic has already asked Xen to try and fix up this condition >>> automatically. >>> >>> What can actually be done depends on how A/D bits behave in real hardware. >>> >>> Setting access bits for non-leaf entries is definitely fine, and >>> speculatively setting the access bit is also explicitly permitted by the >>> spec. However, I can't find any comment on speculative dirty bits (from >>> either Intel or AMD), and I've not encountered such a behaviour with the >>> pagetable work I've been doing. Yeah, a description of the problem to solve definitely helps. >> I've forgotten a piece of information that I really should have written >> here: we would only set the D bit if A is already set and either the >> page is writable (has _PAGE_RW set) or CR0.WP is 0 (the latter case is >> admittedly more tricky). > > How about a new function which works similarly to guest-walk-tables, but > only ever sets A/D bits. > > Given information from hardware, we know the linear address, and that it > was a problem with the guest pagetables, from which we explicitly know > that it was from writing an A/D bit to a guest PTE. > > While walking down the levels, set any missing A bits and remember if we > set any. If we set A bits, consider ourselves complete and exit back to > the guest. If no A bits were set, and the access was a write (which we > know from the EPT violation information), then set the leaf D bit. Plus taking into consideration CR0.WP and the entry's W bit, as Razvan has said. > This should be architecturally correct as it is exclusively derived from > information provided by the VMExit, and won't cause dirty bits to be > written in cases where the hardware wouldn't have written them > (speculative or otherwise). It does mean that an instruction which > would need to set A and D bits in the walk will take two EPT violations > to achieve the end result, but it probably is still quicker than sending > the vm_event out. I'm afraid this is going to be only mostly correct: Atomicity of the page table write is going to be lost. This could become an actual problem if the guest used racing PTE accesses. Such racing accesses might not be a bug - simply consider the OS scanning for set A and/or D bits (and clearing them when they're set). Or an entity temporarily clearing (parts of) PTEs, with recovery logic in place to restore them when needed for a synchronous access. At the very least there's then the risk of a live lock within the guest. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 126771: regressions - trouble: blocked/fail
flight 126771 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126771/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 126702 build-armhf 6 xen-buildfail REGR. vs. 126702 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a version targeted for testing: xen fd07b6648c4c8891dca5bd0f7ef174b6831f80b2 baseline version: xen e5d6ddcd31a6113e4a3db7a235ca78770fe8f401 Last test of basis 126702 2018-08-26 13:00:31 Z1 days Testing same since 126763 2018-08-27 10:00:29 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Doug Goldstein Jan Beulich Kevin Tian Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 fail build-armhf fail build-amd64-libvirt blocked test-armhf-armhf-xl blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked 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 Not pushing. commit fd07b6648c4c8891dca5bd0f7ef174b6831f80b2 Author: Zhenzhong Duan Date: Mon Aug 27 11:37:24 2018 +0200 VT-d/dmar: iommu mem leak fix Release memory allocated for drhd iommu in error path. Signed-off-by: Zhenzhong Duan Acked-by: Kevin Tian commit c3f0dccc9668b14949c91b76f97dbaccb17d2477 Author: Doug Goldstein Date: Mon Aug 27 11:37:01 2018 +0200 build: remove tboot make targets The tboot targets are woefully out of date. These should really be retired because setting up tboot is more complex than the build process for it. Signed-off-by: Doug Goldstein Acked-by: Jan Beulich Acked-by: Wei Liu Reviewed-by: Christopher Clark commit 629856eae2a7f766f1f024a06ad3abf1fd4b9d37 Author: Paul Durrant Date: Mon Aug 27 11:30:18 2018 +0200 x86/hvm: remove default ioreq server My recent patch [1] to qemu-xen-traditional removes the last use of the 'default' ioreq server in Xen. (This is a catch-all ioreq server that is used if no explicitly registered I/O range is targetted). This patch can be applied once that patch is committed, to remove the (>100 lines of) redundant code in Xen. NOTE: The removal of the special case for HVM_PARAM_DM_DOMAIN in hvm_allow_set_param() is not directly related to removal of default ioreq servers. It could have been cleaned up at any time after commit 9a422c03 "x86/hvm: stop passing explicit domid to hvm_create_ioreq_server()". It is now added to the new deprecated sets introduced by this patch. [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html Signed-off-by: Paul Durrant Acked-by: Andrew Cooper Reviewed-by: Jan Beulich (qemu changes not included) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 27/08/18 13:53, Razvan Cojocaru wrote: > On 8/27/18 3:37 PM, Andrew Cooper wrote: >> On 27/08/18 13:12, Jan Beulich wrote: > For NPT, isn't there an error code bit telling you whether the > request was a user or "system" one? If not, some cheating > would be needed (derive from CPL, accepting that e.g. > descriptor table accesses would get mis-attributed), but > that's still not going to involve looking at the PTE flags. The alternative would be to simply walk (without enforcing any flags, and so making the pfec walk parameter unnecessary) to the respective address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. If _PAGE_ACCESSED is not set, set it and exit. If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. >>> Since it's ambiguous in the NPT case - are you talking about >>> setting the flags in the guest or host page tables? The >>> former, I'm afraid, might not be acceptable (as not always >>> being architecturally correct). In anyway feels as if we'd >>> been here before, in that this reminds me of you meaning >>> to imply from a second walk (with A already set) that it must >>> be a write access. I thought we had settled on such an >>> implication not being generally correct. >> The problem that is trying to be solved is that when operating in >> non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a >> write-protected EPT page, takes an EPT violation for a write to a >> read-only page. >> >> Manually setting the A/D bits (as appropriate) and restarting the >> instruction is sufficient for it to complete correctly. >> >> At the moment, every time this happens, a request is sent to the >> introspection agent, and the agent calculates that it was due to >> pagetable protection, and instructs Xen to emulate the instruction. >> This accounts for 97% (?) of the VMExits, and is unrelated to any of the >> real protections which introspection is trying to achieve. >> >> What Razvan is looking to do is to have Xen skip the "send to the >> introspection agent" part as an optimisation, because hardware tells Xen >> (as part of the VMExit) when this condition has occurred, and the >> vm_event logic has already asked Xen to try and fix up this condition >> automatically. >> >> What can actually be done depends on how A/D bits behave in real hardware. >> >> Setting access bits for non-leaf entries is definitely fine, and >> speculatively setting the access bit is also explicitly permitted by the >> spec. However, I can't find any comment on speculative dirty bits (from >> either Intel or AMD), and I've not encountered such a behaviour with the >> pagetable work I've been doing. > Thanks for the reply! > > I've forgotten a piece of information that I really should have written > here: we would only set the D bit if A is already set and either the > page is writable (has _PAGE_RW set) or CR0.WP is 0 (the latter case is > admittedly more tricky). How about a new function which works similarly to guest-walk-tables, but only ever sets A/D bits. Given information from hardware, we know the linear address, and that it was a problem with the guest pagetables, from which we explicitly know that it was from writing an A/D bit to a guest PTE. While walking down the levels, set any missing A bits and remember if we set any. If we set A bits, consider ourselves complete and exit back to the guest. If no A bits were set, and the access was a write (which we know from the EPT violation information), then set the leaf D bit. This should be architecturally correct as it is exclusively derived from information provided by the VMExit, and won't cause dirty bits to be written in cases where the hardware wouldn't have written them (speculative or otherwise). It does mean that an instruction which would need to set A and D bits in the walk will take two EPT violations to achieve the end result, but it probably is still quicker than sending the vm_event out. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 8/27/18 3:37 PM, Andrew Cooper wrote: > On 27/08/18 13:12, Jan Beulich wrote: For NPT, isn't there an error code bit telling you whether the request was a user or "system" one? If not, some cheating would be needed (derive from CPL, accepting that e.g. descriptor table accesses would get mis-attributed), but that's still not going to involve looking at the PTE flags. >>> The alternative would be to simply walk (without enforcing any flags, >>> and so making the pfec walk parameter unnecessary) to the respective >>> address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. >>> >>> If _PAGE_ACCESSED is not set, set it and exit. >>> If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. >> Since it's ambiguous in the NPT case - are you talking about >> setting the flags in the guest or host page tables? The >> former, I'm afraid, might not be acceptable (as not always >> being architecturally correct). In anyway feels as if we'd >> been here before, in that this reminds me of you meaning >> to imply from a second walk (with A already set) that it must >> be a write access. I thought we had settled on such an >> implication not being generally correct. > > The problem that is trying to be solved is that when operating in > non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a > write-protected EPT page, takes an EPT violation for a write to a > read-only page. > > Manually setting the A/D bits (as appropriate) and restarting the > instruction is sufficient for it to complete correctly. > > At the moment, every time this happens, a request is sent to the > introspection agent, and the agent calculates that it was due to > pagetable protection, and instructs Xen to emulate the instruction. > This accounts for 97% (?) of the VMExits, and is unrelated to any of the > real protections which introspection is trying to achieve. > > What Razvan is looking to do is to have Xen skip the "send to the > introspection agent" part as an optimisation, because hardware tells Xen > (as part of the VMExit) when this condition has occurred, and the > vm_event logic has already asked Xen to try and fix up this condition > automatically. > > What can actually be done depends on how A/D bits behave in real hardware. > > Setting access bits for non-leaf entries is definitely fine, and > speculatively setting the access bit is also explicitly permitted by the > spec. However, I can't find any comment on speculative dirty bits (from > either Intel or AMD), and I've not encountered such a behaviour with the > pagetable work I've been doing. Thanks for the reply! I've forgotten a piece of information that I really should have written here: we would only set the D bit if A is already set and either the page is writable (has _PAGE_RW set) or CR0.WP is 0 (the latter case is admittedly more tricky). Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 126742: all pass - PUSHED
flight 126742 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/126742/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 983f5abb9a0d6cf9cfb5e16d671f15e5dc7510d8 baseline version: ovmf 6861765935d5b69803321ba6e43240845c7ab0e5 Last test of basis 126588 2018-08-25 02:15:40 Z2 days Testing same since 126742 2018-08-27 01:41:42 Z0 days1 attempts People who touched revisions under test: Ruiyu Ni 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 6861765935..983f5abb9a 983f5abb9a0d6cf9cfb5e16d671f15e5dc7510d8 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 27/08/18 13:12, Jan Beulich wrote: >>> For NPT, isn't there an error code bit telling you whether the >>> request was a user or "system" one? If not, some cheating >>> would be needed (derive from CPL, accepting that e.g. >>> descriptor table accesses would get mis-attributed), but >>> that's still not going to involve looking at the PTE flags. >> The alternative would be to simply walk (without enforcing any flags, >> and so making the pfec walk parameter unnecessary) to the respective >> address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. >> >> If _PAGE_ACCESSED is not set, set it and exit. >> If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. > Since it's ambiguous in the NPT case - are you talking about > setting the flags in the guest or host page tables? The > former, I'm afraid, might not be acceptable (as not always > being architecturally correct). In anyway feels as if we'd > been here before, in that this reminds me of you meaning > to imply from a second walk (with A already set) that it must > be a write access. I thought we had settled on such an > implication not being generally correct. The problem that is trying to be solved is that when operating in non-root mode, the cpu pagewalk, when trying to set a guest A/D bit in a write-protected EPT page, takes an EPT violation for a write to a read-only page. Manually setting the A/D bits (as appropriate) and restarting the instruction is sufficient for it to complete correctly. At the moment, every time this happens, a request is sent to the introspection agent, and the agent calculates that it was due to pagetable protection, and instructs Xen to emulate the instruction. This accounts for 97% (?) of the VMExits, and is unrelated to any of the real protections which introspection is trying to achieve. What Razvan is looking to do is to have Xen skip the "send to the introspection agent" part as an optimisation, because hardware tells Xen (as part of the VMExit) when this condition has occurred, and the vm_event logic has already asked Xen to try and fix up this condition automatically. What can actually be done depends on how A/D bits behave in real hardware. Setting access bits for non-leaf entries is definitely fine, and speculatively setting the access bit is also explicitly permitted by the spec. However, I can't find any comment on speculative dirty bits (from either Intel or AMD), and I've not encountered such a behaviour with the pagetable work I've been doing. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 8/27/18 3:12 PM, Jan Beulich wrote: >>> For NPT, isn't there an error code bit telling you whether the >>> request was a user or "system" one? If not, some cheating >>> would be needed (derive from CPL, accepting that e.g. >>> descriptor table accesses would get mis-attributed), but >>> that's still not going to involve looking at the PTE flags. >> >> The alternative would be to simply walk (without enforcing any flags, >> and so making the pfec walk parameter unnecessary) to the respective >> address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only. >> >> If _PAGE_ACCESSED is not set, set it and exit. >> If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit. > > Since it's ambiguous in the NPT case - are you talking about > setting the flags in the guest or host page tables? The > former, I'm afraid, might not be acceptable (as not always > being architecturally correct). In anyway feels as if we'd The former (i.e. the guest) - and I agree that it might not be 100% architecturally correct, however we can't think of a better solution that will really work on most actual hardware. > been here before, in that this reminds me of you meaning > to imply from a second walk (with A already set) that it must > be a write access. I thought we had settled on such an > implication not being generally correct. Right, but the previous attempt had a different problem: for each new [RIP:GLA] pairs we thought A was being set, and every time we had a violation where the current [RIP:GLA] matched the previous we thought D was being set. The main problem there was that an interrupt could have broken RIP constancy (for lack of a better term). This new proposal was to check if A is already set on the current violation - and if it is, set D. While not 100% architecturally correct, it should work with no ill-effects AFAIK. Our use case for this is to simply clear that violation, which is not interesting for the introspection agent - so primarily an optimization. But also, we'd like to be able to get the violation caused by the actual guest instruction (if it will cause one). The current patch solves the first problem, but: 1. It doesn't solve the second problem (we lose a potentially interesting violation). 2. It's slower to emulate the whole instruction. 3. Emulation can fail, and we end up needing to single-step the current instruction. I think what you are saying is that there are scenarios where A is set already, and the fault is caused by _again_ trying to set A, in which case we'd set D. I suppose that could happen, but I'm not sure what it would break. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Unable to build staging-4.9 on RHEL6
>>> On 27.08.18 at 13:30, wrote:
> On Monday, 27 August 2018 8:36:50 PM AEST Jan Beulich wrote:
>> >>> On 27.08.18 at 12:03, wrote:
>> > On Monday, 27 August 2018 6:32:17 PM AEST Jan Beulich wrote:
>> >> >>> On 24.08.18 at 04:56, wrote:
>> >> > When trying to build both xen and qemu-xen from the staging-4.9
>> >> > branches, I'm running into issues compiling.
>> >> >
>> >> > Errors start with:
>> >> >
>> >> > BUILDSTDERR: sse.c: In function 'simd_test':
>> >> > BUILDSTDERR: sse.c:319: error: subscripted value is neither array nor
>> >> > pointer
>> >>
>> >> That's the x86 insn emulator test harness afaict, which doesn't get
>> >> built unless you explicitly ask for it. Why are you building it? It's
>> >> well known that it requires a new enough compiler (and the bar will
>> >> raise with AVX512 support, which is in the works).
>> >
>> > Hi Jan,
>> >
>> > I don't specifically enable any testing (that I'm aware of).
>> >
>> > The current SPEC file that I'm using is at:
>> > https://git.crc.id.au/netwiz/xen49/src/devel/SPECS/xen49.spec
>> >
>> > Essentially it boils down to:
>> > ./configure --enable-systemd --prefix=/usr --enable-xsmpolicy
>> > %{?enable_ocaml}
>> > \
>> >
>> > --libdir=%{_libdir} --enable-efi --disable-qemu-traditional \
>> > --with-extra-qemuU-configure-args="--enable-spice --enable-usb-redir"
>> >
>> > (cd xen; make defconfig; sed -i 's/# CONFIG_LIVEPATCH is not set/
>> > CONFIG_LIVEPATCH=y/g' .config; make oldconfig)
>> >
>> > export XEN_DOMAIN=xen.crc.id.au
>> > make %{?_smp_mflags} %{?efi_flags} dist
>>
>> It's this last step which is of interest afaict; how you configure
>> things does not seem to matter. The dist target implies building
>> dist-tools, which in turn has install-tools as a dependency. Most
>> of the sub-directories under tools/tests/ have an empty
>> install target in their makefiles though, which means nothing is
>> going to be done when entering the respective directories.
>
> This is actually interesting.
>
> I tried originally doing a 'make build', then 'make DESTDIR=%{buildroot}
> install' - however I found that this didn't produce any EFI binaries in /boot/
> efi/efi/$EFI_VENDOR/ like the 'make dist' option does.
>
> This seemed to persist across all versions (4.6 - 4.10) hence the path that I
> had taken.
>
> Should this then be a part of fixing why no EFI binaries exist in the make
> build / make install method, and not trying to fix a test as a side effect of
> having to use 'make dist'?
Hmm - "make build" surely doesn't put anything anywhere outside the
build tree. At least I very much hope it doesn't. "make dist" and
"make install", otoh, ought to match in this respect though: Both
should install files into their dedicated places. After all
install: $(TARGS_INSTALL)
and
dist: $(TARGS_DIST) dist-misc
dist-%: install-%
But for me all of this is pure theory only anyway, as I never use any
of these targets.
Considering that xen.gz and xen.efi installation are don by the same
rule, quite a bit depends on whether you observe the two variants of
make invocation to differ in more that what lands in
/boot/efi/efi/$EFI_VENDOR/, in particular whether within the build
tree xen/xen.efi consistently appears. If both produce it, the
difference must be contained in xen/Makefile's _install rule. And don't
forget that putting _anything_ in /boot/efi/efi/$EFI_VENDOR/ is meant
as a courtesy to developers only anyway. Distributions should not
depend on this part; their boot loader setup should instead copy
whatever is necessary into /boot/efi/... (from, in the xen.efi case,
whatever EFI_DIR resolves to).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
>>> On 27.08.18 at 13:24, wrote:
> On 8/27/18 12:11 PM, Jan Beulich wrote:
> On 24.08.18 at 20:47, wrote:
>>> 619 /* EPT violation qualifications definitions */
>>> 620 typedef union ept_qual {
>>> 621 unsigned long raw;
>>> 622 struct {
>>> 623 bool read:1, write:1, fetch:1,
>>> 624 eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
>>> 625 gla_valid:1,
>>> 626 gla_fault:1; /* Valid iff gla_valid. */
>>> 627 unsigned long /* pad */:55;
>>> 628 };
>>> 629 } __transparent__ ept_qual_t;
>>>
>>> Unfortunately, I've been told that that's not the way to go since those
>>> fields will be relevant only on newer architectures, and we'd like to be
>>> able to support even older ones. And, of course, the other thing is that
>>> they're VMX / EPT specific, and we were hoping to be able to get SVM
>>> support for free like that.
>>
>> If this was the case (I didn't check, and you didn't provide a
>> pointer along with the "I've been told"), at the very least on
>> newer hardware correct behavior should be implemented.
>
> Sorry for being unclear. If the question is who told me, it's been
> decided internally that the product should support older processors. If
> the question is what does "older" mean, we've checked a random Skylake
> CPU with the "if bit 22 is read as 1, the processor reports advanced
> VM-exit information for EPT violations (see Section 27.2.1). This
> reporting is done only if this bit is read as 1" method, and the
> respective bit was not set.
Thanks. If this information was indeed supplied by Intel only
after the original introduction of EPT, then - as said - when this
is available it should be used, but an alternative approach is
needed when the hardware doesn't supply the bits.
>> For NPT, isn't there an error code bit telling you whether the
>> request was a user or "system" one? If not, some cheating
>> would be needed (derive from CPL, accepting that e.g.
>> descriptor table accesses would get mis-attributed), but
>> that's still not going to involve looking at the PTE flags.
>
> The alternative would be to simply walk (without enforcing any flags,
> and so making the pfec walk parameter unnecessary) to the respective
> address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only.
>
> If _PAGE_ACCESSED is not set, set it and exit.
> If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit.
Since it's ambiguous in the NPT case - are you talking about
setting the flags in the guest or host page tables? The
former, I'm afraid, might not be acceptable (as not always
being architecturally correct). In anyway feels as if we'd
been here before, in that this reminds me of you meaning
to imply from a second walk (with A already set) that it must
be a write access. I thought we had settled on such an
implication not being generally correct.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] L1TF, and future work
On Sat, Aug 25, 2018 at 1:21 AM Juergen Gross wrote:
>
> On 24/08/18 20:43, Jason Andryuk wrote:
> > On Wed, Aug 15, 2018 at 10:39 AM Juergen Gross wrote:
> >>
> >> On 15/08/18 16:10, Jan Beulich wrote:
> >> On 15.08.18 at 15:17, wrote:
> 2) 32bit PV guests which use writeable pagetable support will
> automatically get shadowed when the clear the lower half.
> >>>
> >>> ... of a page table entry.
> >>>
> Ideally, such
> guests should be modified to use hypercalls rather than the ptwr
> infrastructure (as its more efficient to begin with), but we can
> probably work around this in Xen by emulating the next few instructions
> until we have a complete PTE (same as the shadow code).
> >>>
> >>> Provided the intervening insns are simple enough. I've looked into
> >>> current Linux pv-ops code the other day, and afaict it's already
> >>> using mmu-op or cmpxchg8b, but not two separate mov-s. But
> >>> of course I've looked at the general routines only, not at things
> >>> perhaps hidden in special cases, or in init-only code.
> >>
> >> Look at xen_pte_clear(). Inside irq handling it will use (PAE case):
> >>
> >> static inline void native_pte_clear(struct mm_struct *mm, unsigned long
> >> addr,
> >> pte_t *ptep)
> >> {
> >> ptep->pte_low = 0;
> >> smp_wmb();
> >> ptep->pte_high = 0;
> >> }
> >
> > I've been testing out set_64bit for PTE operations on 32bit PAE. I
> > haven't found all the spots, but shadowing is now enabled a few
> > seconds into boot instead of immediately.
> >
> > And yes, I think https://bugzilla.kernel.org/show_bug.cgi?id=198497 is
> > related as you presumed a while back.
>
> I have a patch series (two patches) avoiding shadowing completely:
>
> https://lists.xen.org/archives/html/xen-devel/2018-08/msg01785.html
Great! Thank you. I'm building now.
Looks like I missed native_ptep_get_and_clear which led to the delay
enabling shadowing.
Regards,
Jason
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Unable to build staging-4.9 on RHEL6
On Monday, 27 August 2018 8:36:50 PM AEST Jan Beulich wrote:
> >>> On 27.08.18 at 12:03, wrote:
> > On Monday, 27 August 2018 6:32:17 PM AEST Jan Beulich wrote:
> >> >>> On 24.08.18 at 04:56, wrote:
> >> > When trying to build both xen and qemu-xen from the staging-4.9
> >> > branches, I'm running into issues compiling.
> >> >
> >> > Errors start with:
> >> >
> >> > BUILDSTDERR: sse.c: In function 'simd_test':
> >> > BUILDSTDERR: sse.c:319: error: subscripted value is neither array nor
> >> > pointer
> >>
> >> That's the x86 insn emulator test harness afaict, which doesn't get
> >> built unless you explicitly ask for it. Why are you building it? It's
> >> well known that it requires a new enough compiler (and the bar will
> >> raise with AVX512 support, which is in the works).
> >
> > Hi Jan,
> >
> > I don't specifically enable any testing (that I'm aware of).
> >
> > The current SPEC file that I'm using is at:
> > https://git.crc.id.au/netwiz/xen49/src/devel/SPECS/xen49.spec
> >
> > Essentially it boils down to:
> > ./configure --enable-systemd --prefix=/usr --enable-xsmpolicy
> > %{?enable_ocaml}
> > \
> >
> > --libdir=%{_libdir} --enable-efi --disable-qemu-traditional \
> > --with-extra-qemuU-configure-args="--enable-spice --enable-usb-redir"
> >
> > (cd xen; make defconfig; sed -i 's/# CONFIG_LIVEPATCH is not set/
> > CONFIG_LIVEPATCH=y/g' .config; make oldconfig)
> >
> > export XEN_DOMAIN=xen.crc.id.au
> > make %{?_smp_mflags} %{?efi_flags} dist
>
> It's this last step which is of interest afaict; how you configure
> things does not seem to matter. The dist target implies building
> dist-tools, which in turn has install-tools as a dependency. Most
> of the sub-directories under tools/tests/ have an empty
> install target in their makefiles though, which means nothing is
> going to be done when entering the respective directories.
This is actually interesting.
I tried originally doing a 'make build', then 'make DESTDIR=%{buildroot}
install' - however I found that this didn't produce any EFI binaries in /boot/
efi/efi/$EFI_VENDOR/ like the 'make dist' option does.
This seemed to persist across all versions (4.6 - 4.10) hence the path that I
had taken.
Should this then be a part of fixing why no EFI binaries exist in the make
build / make install method, and not trying to fix a test as a side effect of
having to use 'make dist'?
> > I'll note that 4.6, 4.7, and 4.10 do not fail in this fashion using almost
> > the same command set to build.
>
> I can't spot any relevant difference between the versions you
> mention (or master). I'm afraid you'll need to dig a little deeper
> yourself (unless someone else has an idea).
>
> Jan
--
Steven Haigh
net...@crc.id.au https://www.crc.id.au
+61 (3) 9001 6090 0412 935 897
signature.asc
Description: This is a digitally signed message part.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 126763: regressions - trouble: blocked/fail
flight 126763 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126763/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 126702 build-armhf 6 xen-buildfail REGR. vs. 126702 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a version targeted for testing: xen fd07b6648c4c8891dca5bd0f7ef174b6831f80b2 baseline version: xen e5d6ddcd31a6113e4a3db7a235ca78770fe8f401 Last test of basis 126702 2018-08-26 13:00:31 Z0 days Testing same since 126763 2018-08-27 10:00:29 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Doug Goldstein Jan Beulich Kevin Tian Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 fail build-armhf fail build-amd64-libvirt blocked test-armhf-armhf-xl blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked 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 Not pushing. commit fd07b6648c4c8891dca5bd0f7ef174b6831f80b2 Author: Zhenzhong Duan Date: Mon Aug 27 11:37:24 2018 +0200 VT-d/dmar: iommu mem leak fix Release memory allocated for drhd iommu in error path. Signed-off-by: Zhenzhong Duan Acked-by: Kevin Tian commit c3f0dccc9668b14949c91b76f97dbaccb17d2477 Author: Doug Goldstein Date: Mon Aug 27 11:37:01 2018 +0200 build: remove tboot make targets The tboot targets are woefully out of date. These should really be retired because setting up tboot is more complex than the build process for it. Signed-off-by: Doug Goldstein Acked-by: Jan Beulich Acked-by: Wei Liu Reviewed-by: Christopher Clark commit 629856eae2a7f766f1f024a06ad3abf1fd4b9d37 Author: Paul Durrant Date: Mon Aug 27 11:30:18 2018 +0200 x86/hvm: remove default ioreq server My recent patch [1] to qemu-xen-traditional removes the last use of the 'default' ioreq server in Xen. (This is a catch-all ioreq server that is used if no explicitly registered I/O range is targetted). This patch can be applied once that patch is committed, to remove the (>100 lines of) redundant code in Xen. NOTE: The removal of the special case for HVM_PARAM_DM_DOMAIN in hvm_allow_set_param() is not directly related to removal of default ioreq servers. It could have been cleaned up at any time after commit 9a422c03 "x86/hvm: stop passing explicit domid to hvm_create_ioreq_server()". It is now added to the new deprecated sets introduced by this patch. [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html Signed-off-by: Paul Durrant Acked-by: Andrew Cooper Reviewed-by: Jan Beulich (qemu changes not included) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
On 8/27/18 12:11 PM, Jan Beulich wrote:
On 24.08.18 at 20:47, wrote:
>> 619 /* EPT violation qualifications definitions */
>> 620 typedef union ept_qual {
>> 621 unsigned long raw;
>> 622 struct {
>> 623 bool read:1, write:1, fetch:1,
>> 624 eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
>> 625 gla_valid:1,
>> 626 gla_fault:1; /* Valid iff gla_valid. */
>> 627 unsigned long /* pad */:55;
>> 628 };
>> 629 } __transparent__ ept_qual_t;
>>
>> Unfortunately, I've been told that that's not the way to go since those
>> fields will be relevant only on newer architectures, and we'd like to be
>> able to support even older ones. And, of course, the other thing is that
>> they're VMX / EPT specific, and we were hoping to be able to get SVM
>> support for free like that.
>
> If this was the case (I didn't check, and you didn't provide a
> pointer along with the "I've been told"), at the very least on
> newer hardware correct behavior should be implemented.
Sorry for being unclear. If the question is who told me, it's been
decided internally that the product should support older processors. If
the question is what does "older" mean, we've checked a random Skylake
CPU with the "if bit 22 is read as 1, the processor reports advanced
VM-exit information for EPT violations (see Section 27.2.1). This
reporting is done only if this bit is read as 1" method, and the
respective bit was not set.
The information is in Table 27-6 of the Intel SDM.
> For NPT, isn't there an error code bit telling you whether the
> request was a user or "system" one? If not, some cheating
> would be needed (derive from CPL, accepting that e.g.
> descriptor table accesses would get mis-attributed), but
> that's still not going to involve looking at the PTE flags.
The alternative would be to simply walk (without enforcing any flags,
and so making the pfec walk parameter unnecessary) to the respective
address, and query for _PAGE_ACCESSED and _PAGE_DIRTY only.
If _PAGE_ACCESSED is not set, set it and exit.
If _PAGE_ACCESSED is set, set _PAGE_DIRTY also and exit.
The idea here being that (1) "no TLB entry or paging-structure cache
entry is created with information from a paging-structure entry in which
the accessed flag is 0" and (2) "a page-fault exception resulting from
an attempt to use a linear address will invalidate any TLB entries that
are for a page number corresponding to that linear address and that are
associated with the current PCID" mean that the only way we would end up
in the codepath is if we need to set the A/D bits - which we can, if we
use modified version of the code walker that simply does not care about
the pfec parameter.
This would presumably also work well for NPT, and also on CPUs that do
not support the newer EPT qualification bits.
Is this upstreamable?
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
On Mon, Aug 27, 2018 at 03:59:16AM -0600, Jan Beulich wrote:
> >>> On 27.08.18 at 11:38, wrote:
> > On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
> >> >>> On 25.07.18 at 13:16, wrote:
> >> > --- a/xen/include/public/hvm/hvm_op.h
> >> > +++ b/xen/include/public/hvm/hvm_op.h
> >> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
> >> > typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> >> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >> >
> >> > -struct xen_hvm_altp2m_set_mem_access {
> >> > +struct xen_hvm_altp2m_mem_access {
> >> > /* view */
> >> > uint16_t view;
> >> > /* Memory type */
> >> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
> >> > /* gfn */
> >> > uint64_t gfn;
> >> > };
> >> > -typedef struct xen_hvm_altp2m_set_mem_access
> >> > xen_hvm_altp2m_set_mem_access_t;
> >> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> >> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
> >> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
> >> >
> >> > struct xen_hvm_altp2m_set_mem_access_multi {
> >> > /* view */
> >> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
> >> > #define HVMOP_altp2m_change_gfn 8
> >> > /* Set access for an array of pages */
> >> > #define HVMOP_altp2m_set_mem_access_multi 9
> >> > +/* Get the access of a page of memory from a certain view */
> >> > +#define HVMOP_altp2m_get_mem_access 10
> >> > domid_t domain;
> >> > uint16_t pad1;
> >> > uint32_t pad2;
> >> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
> >> > struct xen_hvm_altp2m_domain_state domain_state;
> >> > struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
> >> > struct xen_hvm_altp2m_view view;
> >> > -struct xen_hvm_altp2m_set_mem_access set_mem_access;
> >> > +struct xen_hvm_altp2m_mem_access mem_access;
> >> > struct xen_hvm_altp2m_change_gfn change_gfn;
> >> > struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
> >> > uint8_t pad[64];
> >>
> >> This being exposed to guests, the interface has to be considered
> >> stable imo, in which case you can't rename things like this. You'd
> >> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
> >> case further up in the file).
> >
> > Right. Sorry about that. Maybe just having separate structs for
> > get/set would be cleaner in this case, even though they would be
> > similar.
>
> Personally I'd prefer to avoid having two structures with identical
> layout but different tags. But if others think differently, I'm not
> meaning to stand in the way.
Ok then. I have no strong preference either way.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 126677: regressions - FAIL
flight 126677 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/126677/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-rumprun-amd64 12 guest-startfail REGR. vs. 125183 test-amd64-amd64-pair21 guest-start/debian fail REGR. vs. 125183 test-amd64-amd64-xl-xsm 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-xl 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-pygrub 10 debian-di-installfail REGR. vs. 125183 test-amd64-amd64-xl-credit2 12 guest-start fail REGR. vs. 125183 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 125183 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-xl-shadow 12 guest-start fail REGR. vs. 125183 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 125183 test-amd64-i386-pair 21 guest-start/debian fail REGR. vs. 125183 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 125183 test-amd64-amd64-xl-pvshim 12 guest-start fail REGR. vs. 125183 test-amd64-i386-xl-xsm 12 guest-start fail REGR. vs. 125183 test-amd64-i386-xl-shadow12 guest-start fail REGR. vs. 125183 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125183 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-qemut-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125183 test-amd64-i386-xl 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-xl-multivcpu 12 guest-start fail REGR. vs. 125183 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125183 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 125183 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125183 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-i386-pvgrub 10 debian-di-installfail REGR. vs. 125183 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemut-win7-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 125183 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-i386-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125183 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125183 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125183 test-armhf-armhf-xl-credit2 12 guest-start fail REGR. vs. 125183 test-armhf-armhf-xl-cubietruck 12 guest-startfail REGR. vs. 125183 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 125183 test-armhf-armhf-xl-multivcpu 12 guest-start fail REGR. vs. 125183 test-amd64-i386-rumprun-i386 12 guest-start fail REGR. vs. 125183 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 125183 test-amd64-amd64-amd64-pvgrub 10 debian-di-install fail REGR. vs.
Re: [Xen-devel] Unable to build staging-4.9 on RHEL6
>>> On 27.08.18 at 12:03, wrote:
> On Monday, 27 August 2018 6:32:17 PM AEST Jan Beulich wrote:
>> >>> On 24.08.18 at 04:56, wrote:
>> > When trying to build both xen and qemu-xen from the staging-4.9
>> > branches, I'm running into issues compiling.
>> >
>> > Errors start with:
>> >
>> > BUILDSTDERR: sse.c: In function 'simd_test':
>> > BUILDSTDERR: sse.c:319: error: subscripted value is neither array nor
>> > pointer
>>
>> That's the x86 insn emulator test harness afaict, which doesn't get
>> built unless you explicitly ask for it. Why are you building it? It's
>> well known that it requires a new enough compiler (and the bar will
>> raise with AVX512 support, which is in the works).
>
> Hi Jan,
>
> I don't specifically enable any testing (that I'm aware of).
>
> The current SPEC file that I'm using is at:
> https://git.crc.id.au/netwiz/xen49/src/devel/SPECS/xen49.spec
>
> Essentially it boils down to:
> ./configure --enable-systemd --prefix=/usr --enable-xsmpolicy
> %{?enable_ocaml}
> \
> --libdir=%{_libdir} --enable-efi --disable-qemu-traditional \
> --with-extra-qemuU-configure-args="--enable-spice --enable-usb-redir"
>
> (cd xen; make defconfig; sed -i 's/# CONFIG_LIVEPATCH is not set/
> CONFIG_LIVEPATCH=y/g' .config; make oldconfig)
>
> export XEN_DOMAIN=xen.crc.id.au
> make %{?_smp_mflags} %{?efi_flags} dist
It's this last step which is of interest afaict; how you configure
things does not seem to matter. The dist target implies building
dist-tools, which in turn has install-tools as a dependency. Most
of the sub-directories under tools/tests/ have an empty
install target in their makefiles though, which means nothing is
going to be done when entering the respective directories.
> I'll note that 4.6, 4.7, and 4.10 do not fail in this fashion using almost
> the
> same command set to build.
I can't spot any relevant difference between the versions you
mention (or master). I'm afraid you'll need to dig a little deeper
yourself (unless someone else has an idea).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Unable to build staging-4.9 on RHEL6
On Monday, 27 August 2018 6:32:17 PM AEST Jan Beulich wrote:
> >>> On 24.08.18 at 04:56, wrote:
> > When trying to build both xen and qemu-xen from the staging-4.9
> > branches, I'm running into issues compiling.
> >
> > Errors start with:
> >
> > BUILDSTDERR: sse.c: In function 'simd_test':
> > BUILDSTDERR: sse.c:319: error: subscripted value is neither array nor
> > pointer
>
> That's the x86 insn emulator test harness afaict, which doesn't get
> built unless you explicitly ask for it. Why are you building it? It's
> well known that it requires a new enough compiler (and the bar will
> raise with AVX512 support, which is in the works).
Hi Jan,
I don't specifically enable any testing (that I'm aware of).
The current SPEC file that I'm using is at:
https://git.crc.id.au/netwiz/xen49/src/devel/SPECS/xen49.spec
Essentially it boils down to:
./configure --enable-systemd --prefix=/usr --enable-xsmpolicy %{?enable_ocaml}
\
--libdir=%{_libdir} --enable-efi --disable-qemu-traditional \
--with-extra-qemuU-configure-args="--enable-spice --enable-usb-redir"
(cd xen; make defconfig; sed -i 's/# CONFIG_LIVEPATCH is not set/
CONFIG_LIVEPATCH=y/g' .config; make oldconfig)
export XEN_DOMAIN=xen.crc.id.au
make %{?_smp_mflags} %{?efi_flags} dist
I'll note that 4.6, 4.7, and 4.10 do not fail in this fashion using almost the
same command set to build.
--
Steven Haigh
net...@crc.id.au https://www.crc.id.au
+61 (3) 9001 6090 0412 935 897
signature.asc
Description: This is a digitally signed message part.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
>>> On 27.08.18 at 11:38, wrote:
> On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
>> >>> On 25.07.18 at 13:16, wrote:
>> > --- a/xen/include/public/hvm/hvm_op.h
>> > +++ b/xen/include/public/hvm/hvm_op.h
>> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
>> > typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
>> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
>> >
>> > -struct xen_hvm_altp2m_set_mem_access {
>> > +struct xen_hvm_altp2m_mem_access {
>> > /* view */
>> > uint16_t view;
>> > /* Memory type */
>> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
>> > /* gfn */
>> > uint64_t gfn;
>> > };
>> > -typedef struct xen_hvm_altp2m_set_mem_access
>> > xen_hvm_altp2m_set_mem_access_t;
>> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
>> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
>> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
>> >
>> > struct xen_hvm_altp2m_set_mem_access_multi {
>> > /* view */
>> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
>> > #define HVMOP_altp2m_change_gfn 8
>> > /* Set access for an array of pages */
>> > #define HVMOP_altp2m_set_mem_access_multi 9
>> > +/* Get the access of a page of memory from a certain view */
>> > +#define HVMOP_altp2m_get_mem_access 10
>> > domid_t domain;
>> > uint16_t pad1;
>> > uint32_t pad2;
>> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
>> > struct xen_hvm_altp2m_domain_state domain_state;
>> > struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
>> > struct xen_hvm_altp2m_view view;
>> > -struct xen_hvm_altp2m_set_mem_access set_mem_access;
>> > +struct xen_hvm_altp2m_mem_access mem_access;
>> > struct xen_hvm_altp2m_change_gfn change_gfn;
>> > struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
>> > uint8_t pad[64];
>>
>> This being exposed to guests, the interface has to be considered
>> stable imo, in which case you can't rename things like this. You'd
>> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
>> case further up in the file).
>
> Right. Sorry about that. Maybe just having separate structs for
> get/set would be cleaner in this case, even though they would be
> similar.
Personally I'd prefer to avoid having two structures with identical
layout but different tags. But if others think differently, I'm not
meaning to stand in the way.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] rombios: work around clang's -Waddress-of-packed-member
>>> On 24.08.18 at 17:22, wrote:
> Building rombios with clang XXX fails with:
>
> tcgbios.c:1519:34: error: taking address of packed member 'u' of class or
> structure 'pushad_regs_t' may result in an unaligned pointer value
> [-Werror,-Waddress-of-packed-member]
> ®s->u.r32.edx);
>^~~
>
> Work around that by using an intermediate variable.
>
> Signed-off-by: Wei Liu
> ---
> tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
> b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
> index fa22c4460a..581340da8e 100644
> --- a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
> +++ b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
> @@ -1507,7 +1507,8 @@ uint32_t TCGInterruptHandler(pushad_regs_t *regs,
> uint32_t esds,
> regs->u.r32.edx);
> CLEAR_CF();
> break;
> - case 0x07:
> + case 0x07: {
> + uint32_t edx = regs->u.r32.edx;
> regs->u.r32.eax =
> CompactHashLogExtendEvent32((unsigned char *)
> ADDR_FROM_SEG_OFF(ES,
> @@ -1516,9 +1517,11 @@ uint32_t TCGInterruptHandler(pushad_regs_t *regs,
> uint32_t esds,
> regs->u.r32.ebx,
> regs->u.r32.ecx,
> regs->u.r32.edx,
> - >u.r32.edx);
> + );
> + regs->u.r32.edx = edx;
> CLEAR_CF();
> break;
> + }
Hmm, this addresses a specific instance of the problem. In this (I think)
never changing code base doing so might be okay, but is there a reason
not to fix the root of the problem - remove the bogus packed attribute?
The structure definition in 32bit/rombios_compat.h spells out all padding
fields, and I can't spot any other (side) effect the attribute would have,
the more that it's only the outermost structure which gets the attribute
applied.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] hvmloader: fix build with LLVM Linker
>>> On 24.08.18 at 11:58, wrote:
> --- /dev/null
> +++ b/tools/firmware/hvmloader/hvmloader.lds
> @@ -0,0 +1,13 @@
> +SECTIONS
> +{
> + . = 0x10;
> + /*
> + * NB: there's no need to use the AT keyword in order to set the LMA, by
> + * default the linker will use VMA = LMA unless specified otherwise.
> + */
> + .text : { *(.text) }
> + .rodata : { *(.rodata) }
> + .data : { *(.data) }
> + .bss : { *(.bss) }
> + _end = .;
> +}
Is this really sufficient? Iirc the compiler could create quite a few
more variants of the sections named above, like .rodata.str* or
.text.cold. Hence at the very least I'd expect . on the
right sides above to be accompanied by ..* .
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] pvshim: introduce a PV shim defconfig
>>> On 22.08.18 at 12:36, wrote: > --- /dev/null > +++ b/xen/arch/x86/configs/pvshim_defconfig > @@ -0,0 +1,23 @@ > +# Enable PV shim mode > +CONFIG_PV=y > +CONFIG_XEN_GUEST=y > +CONFIG_PVH_GUEST=y > +CONFIG_PV_SHIM=y > +CONFIG_PV_SHIM_EXCLUSIVE=y > +# Disable features not used by the PV shim > +CONFIG_NR_CPUS=32 > +CONFIG_SHADOW_PAGING=n > +CONFIG_BIGMEM=n > +CONFIG_HVM_FEP=n > +CONFIG_TBOOT=n > +CONFIG_KEXEC=n > +CONFIG_TMEM=n > +CONFIG_XENOPROF=n > +CONFIG_XSM=n > +CONFIG_SCHED_CREDIT2=n > +CONFIG_SCHED_RTDS=n > +CONFIG_SCHED_ARINC653=n > +CONFIG_SCHED_NULL=n > +CONFIG_LIVEPATCH=n > +CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS=n > +CONFIG_DEBUG=n Since the *defconfig-s we have so far are all empty, and since the Linux x86 ones aren't written this way I wonder: Is there a reason you use "=n" instead of the "# CONFIG_... is not set" form? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/altp2m: Allow setting the #VE info page for an arbitrary VCPU
On Tue, Jul 31, 2018 at 05:53:00AM -0600, Jan Beulich wrote:
> > +struct vcpu *v;
> > +
> > +dom = rcu_lock_domain_by_id(domain_id);
> > +
> > +for_each_vcpu( dom, v )
> > +{
> > +if ( vcpu_id == v->vcpu_id )
> > +{
> > +rcu_unlock_domain(dom);
> > +return v;
> > +}
> > +}
>
> for_each_vcpu() looks excessive here - all you need is a bounds
> check and an access into d->vcpus[]. Together with the fact
> that your caller has already identified and locked d I wonder
> whether this helper is needed in the first place.
All right. I'll remove it altogether.
> > @@ -4576,26 +4599,32 @@ static int do_altp2m_op(
> >
> > case HVMOP_altp2m_vcpu_enable_notify:
> > {
> > -struct vcpu *curr = current;
> > +struct vcpu *v;
> > p2m_type_t p2mt;
> >
> > -if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
> > - a.u.enable_notify.vcpu_id != curr->vcpu_id )
> > +if ( a.u.enable_notify.pad )
> > {
> > rc = -EINVAL;
> > break;
> > }
> >
> > -if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
> > - mfn_eq(get_gfn_query_unlocked(curr->domain,
> > +v = __get_vcpu(a.domain, a.u.enable_notify.vcpu_id);
> > +if ( !v )
> > +{
> > +rc = -EFAULT;
>
> Hardly an appropriate error indicator for the condition.
I'll change it to -EINVAL.
> > +break;
> > +}
> > +
> > +if ( !gfn_eq(vcpu_altp2m(v).veinfo_gfn, INVALID_GFN) ||
> > + mfn_eq(get_gfn_query_unlocked(v->domain,
> > a.u.enable_notify.gfn, ), INVALID_MFN) )
> > {
> > rc = -EINVAL;
> > break;
> > }
> >
> > -vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
> > -altp2m_vcpu_update_vmfunc_ve(curr);
> > +vcpu_altp2m(v).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
> > +altp2m_vcpu_update_vmfunc_ve(v);
>
> I'd like you to outline in the description how you mean an external
> agent to coordinate the use of this GFN with the guest (and in
> particular without in-guest agent).
I'll try to clarify this.
Thank you!
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit
On Tue, Jul 31, 2018 at 05:44:03AM -0600, Jan Beulich wrote:
> >>> On 25.07.18 at 13:18, wrote:
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -38,7 +38,7 @@ struct xen_hvm_param {
> > typedef struct xen_hvm_param xen_hvm_param_t;
> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_param_t);
> >
> > -struct xen_hvm_altp2m_set_suppress_ve {
> > +struct xen_hvm_altp2m_suppress_ve {
>
> Please add this without the "set_" right away in patch 2.
Sure. Thanks!
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
> >>> On 25.07.18 at 13:16, wrote:
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
> > typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >
> > -struct xen_hvm_altp2m_set_mem_access {
> > +struct xen_hvm_altp2m_mem_access {
> > /* view */
> > uint16_t view;
> > /* Memory type */
> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
> > /* gfn */
> > uint64_t gfn;
> > };
> > -typedef struct xen_hvm_altp2m_set_mem_access
> > xen_hvm_altp2m_set_mem_access_t;
> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
> >
> > struct xen_hvm_altp2m_set_mem_access_multi {
> > /* view */
> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
> > #define HVMOP_altp2m_change_gfn 8
> > /* Set access for an array of pages */
> > #define HVMOP_altp2m_set_mem_access_multi 9
> > +/* Get the access of a page of memory from a certain view */
> > +#define HVMOP_altp2m_get_mem_access 10
> > domid_t domain;
> > uint16_t pad1;
> > uint32_t pad2;
> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
> > struct xen_hvm_altp2m_domain_state domain_state;
> > struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
> > struct xen_hvm_altp2m_view view;
> > -struct xen_hvm_altp2m_set_mem_access set_mem_access;
> > +struct xen_hvm_altp2m_mem_access mem_access;
> > struct xen_hvm_altp2m_change_gfn change_gfn;
> > struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
> > uint8_t pad[64];
>
> This being exposed to guests, the interface has to be considered
> stable imo, in which case you can't rename things like this. You'd
> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
> case further up in the file).
Right. Sorry about that. Maybe just having separate structs for
get/set would be cleaner in this case, even though they would be
similar.
> Also, to you, George, and whoever else advocates for this, another
> remark regarding the guest accessibility here (at the risk of getting
> flamed once again): The less capable (afaict)
> XENMEM_access_op_{g,s}et_access (previously
> HVMOP_{g,s}et_mem_access) are tools accessible only. I find such
> an inconsistency rather odd.
I don't really know what to reply. While there probably isn't, as far
as I'm aware, any use-case for the non-altp2m OPs to be accessed from
the guest via HVMOPs directly, if my understanding is correct, the
scenario involving #VE and an in-guest agent would require this
functionality.
Thanks.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 0/6] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
>>> On 22.08.18 at 09:51, wrote: > Hello, > > The following series implement a workaround for missing RMRR > entries for a PVH Dom0. It's based on the iommu_inclusive_mapping VTd > option. > > The PVH workaround identity maps all regions marked as reserved in the > memory map. > > Note that this workaround is enabled by default on Intel hardware. It's > also available to AMD hardware, although it's disabled by default in > that case. > > The series can be found at: > > git://xenbits.xen.org/people/royger/xen.git iommu_inclusive_v7 > > Thanks. > > Roger Pau Monne (6): > iommu: rename iommu_dom0_strict and iommu_passthrough > iommu: introduce dom0-iommu option > iommu: make iommu_inclusive_mapping a suboption of dom0-iommu Brian, Suravee, the first three patches of this series could go in if only we had an ack from one of you on the first and third... Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 25/34] x86/mm/shadow: make it build with !CONFIG_HVM
On Mon, Aug 27, 2018 at 01:46:10AM -0600, Jan Beulich wrote:
> >>> On 26.08.18 at 13:04, wrote:
> > On Tue, Aug 21, 2018 at 02:27:40AM -0600, Jan Beulich wrote:
> >>
> >> > --- a/xen/arch/x86/mm/shadow/multi.c
> >> > +++ b/xen/arch/x86/mm/shadow/multi.c
> >> > @@ -2926,18 +2926,25 @@ static int sh_page_fault(struct vcpu *v,
> >> > }
> >> > else
> >> > {
> >> > +#if CONFIG_HVM
> >> > /* Magic MMIO marker: extract gfn for MMIO address */
> >> > ASSERT(sh_l1e_is_mmio(sl1e));
> >> > +ASSERT(is_hvm_vcpu(v));
> >> > gpa = (((paddr_t)(gfn_x(sh_l1e_mmio_get_gfn(sl1e
> >> > << PAGE_SHIFT)
> >> > | (va & ~PAGE_MASK);
> >> > +perfc_incr(shadow_fault_fast_mmio);
> >> > +SHADOW_PRINTK("fast path mmio %#"PRIpaddr"\n", gpa);
> >> > +sh_reset_early_unshadow(v);
> >> > +trace_shadow_gen(TRC_SHADOW_FAST_MMIO, va);
> >> > +return handle_mmio_with_translation(va, gpa >>
> >> > PAGE_SHIFT,
> >> > +access)
> >> > +? EXCRET_fault_fixed : 0;
> >> > +#else
> >> > +/* When HVM is not enabled, there shouldn't be MMIO
> >> > marker */
> >> > +BUG();
> >>
> >> At the example of this, while I agree we shouldn't reach here for PV,
> >> can this nevertheless be the less impactful domain_crash() please?
> >>
> >
> > Do you only want this BUG() to be replaced?
> >
> > I think the two in shadonw_*_emulation should stay because you will only
> > ever get NULL pointer deref if you allow the code to continue.
>
> Did you perhaps remove too much context? From what's left I can't
> judge which others you refer to, or what NULL deref you talk about.
The BUG()s in shadow_*_emulation, like I mentioned in my reply.
What I meant was if I make shadown_init_emulation like:
domain_crash(d);
return NULL;
Nothing good is going to happen.
> Looking back at the full patch - I think I had already suggested that
> the two shadow_*_emulation() should altogether go inside #ifdef
> CONFIG_HVM, not just their bodies.
I will see about doing that later this week.
(Today is public holiday in UK)
Wei.
>
> Jan
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xenforeignmemory: work around bug in older privcmd
On Mon, Aug 27, 2018 at 03:04:55AM -0600, Jan Beulich wrote:
> >>> On 24.08.18 at 14:16, wrote:
> > Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> > for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> > -ENOTTY for unimplemented codes. This breaks the error path in
> > libxenforeignmemory resource mapping, which only translates ENOTTY into
> > EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> > mechanism.
> >
> > This patch adds a new 'unimplemented' [1] ioctl code into the local
> > privcmd header which is then used to probe for the appropriate errno to
> > translate in the resource mapping error path
> >
> > [1] this is a code that has, so far, never been used in any version of
> > privcmd and will be added to future versions of the header in the
> > linux source, to make sure it stays unimplemented.
>
> Shouldn't this addition happen before the Xen tools side change goes
> in?
It doesn't really make any difference from a practical PoV. Applying
this patch right away helps unblocking various Linux branches.
Wei.
>
> Jan
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/mm: Suppresses vm_events caused by page-walks
>>> On 24.08.18 at 20:47, wrote:
> On 8/24/18 8:49 PM, Andrew Cooper wrote:
>> On 24/08/18 15:11, Alexandru Isaila wrote:
>>> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
>>> index 03a864156..b01194d 100644
>>> --- a/xen/arch/x86/mm/mem_access.c
>>> +++ b/xen/arch/x86/mm/mem_access.c
>>> @@ -212,7 +212,20 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long
>>> gla,
>>> d->arch.monitor.inguest_pagefault_disabled &&
>>> npfec.kind != npfec_kind_with_gla ) /* don't send a mem_event */
>>> {
>>> -hvm_emulate_one_vm_event(EMUL_KIND_NORMAL, TRAP_invalid_op,
>>> X86_EVENT_NO_EC);
>>> +struct hvm_hw_cpu ctxt;
>>> +uint32_t pfec = PFEC_page_present;
>>> +unsigned long gfn;
>>> +uint32_t gflags;
>>> +
>>> +hvm_funcs.save_cpu_ctxt(v, );
>>> +paging_get_hostmode(v)->pte_flags(v, p2m, gla, 0, ctxt.cr3,
>>> );
>>> +if ( gflags & _PAGE_RW )
>>> +pfec |= PFEC_write_access;
>>> +
>>> +if ( gflags & _PAGE_USER )
>>> +pfec |= PFEC_user_mode;
>>
>> As I've tried to explain before, this is architecturally incorrect.
>> Both need to be derived from the EPT violation, because they are
>> properties of instruction which caused the fault, not the mapping which
>> faulted.
>
> Right, I assume you mean starting to use eff_read, eff_write, eff_exec,
> and eff_user_exec from ept_qual_t:
>
> 619 /* EPT violation qualifications definitions */
> 620 typedef union ept_qual {
> 621 unsigned long raw;
> 622 struct {
> 623 bool read:1, write:1, fetch:1,
> 624 eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
> 625 gla_valid:1,
> 626 gla_fault:1; /* Valid iff gla_valid. */
> 627 unsigned long /* pad */:55;
> 628 };
> 629 } __transparent__ ept_qual_t;
>
> Unfortunately, I've been told that that's not the way to go since those
> fields will be relevant only on newer architectures, and we'd like to be
> able to support even older ones. And, of course, the other thing is that
> they're VMX / EPT specific, and we were hoping to be able to get SVM
> support for free like that.
If this was the case (I didn't check, and you didn't provide a
pointer along with the "I've been told"), at the very least on
newer hardware correct behavior should be implemented.
For NPT, isn't there an error code bit telling you whether the
request was a user or "system" one? If not, some cheating
would be needed (derive from CPL, accepting that e.g.
descriptor table accesses would get mis-attributed), but
that's still not going to involve looking at the PTE flags.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xenforeignmemory: work around bug in older privcmd
>>> On 24.08.18 at 14:16, wrote:
> Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> -ENOTTY for unimplemented codes. This breaks the error path in
> libxenforeignmemory resource mapping, which only translates ENOTTY into
> EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> mechanism.
>
> This patch adds a new 'unimplemented' [1] ioctl code into the local
> privcmd header which is then used to probe for the appropriate errno to
> translate in the resource mapping error path
>
> [1] this is a code that has, so far, never been used in any version of
> privcmd and will be added to future versions of the header in the
> linux source, to make sure it stays unimplemented.
Shouldn't this addition happen before the Xen tools side change goes
in?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 15/23] x86/mm: put HVM only code under CONFIG_HVM
On Sun, Aug 26, 2018 at 01:19:48PM +0100, Wei Liu wrote:
> Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
> Put these components under CONFIG_HVM. This further requires putting
> one of the vm event under CONFIG_HVM.
>
> Also make hap_enabled evaluate to false when !CONFIG_HVM. This in turn
> requires marking a variable in p2m_set_entry as __maybe_unused.
>
> Signed-off-by: Wei Liu
> ---
> xen/arch/x86/mm/Makefile | 7 ---
> xen/arch/x86/mm/p2m.c| 17 ++---
> xen/common/vm_event.c| 2 ++
> xen/include/asm-x86/hvm/domain.h | 4
> 4 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
> index 3017119..e3b7be8 100644
> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -1,9 +1,10 @@
> subdir-y += shadow
> -subdir-y += hap
> +subdir-$(CONFIG_HVM) += hap
>
> obj-y += paging.o
> -obj-y += p2m.o p2m-pt.o p2m-ept.o p2m-pod.o
> -obj-y += altp2m.o
> +obj-y += p2m.o p2m-pt.o
> +obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
> +obj-$(CONFIG_HVM) += altp2m.o
> obj-y += guest_walk_2.o
> obj-y += guest_walk_3.o
> obj-y += guest_walk_4.o
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index c82db32..e41d6bf 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -535,7 +535,7 @@ struct page_info *p2m_get_page_from_gfn(
> int p2m_set_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
>unsigned int page_order, p2m_type_t p2mt, p2m_access_t
> p2ma)
> {
> -struct domain *d = p2m->domain;
> +struct domain * __maybe_unused d = p2m->domain;
> unsigned long todo = 1ul << page_order;
> unsigned int order;
> int set_rc, rc = 0;
> @@ -1712,12 +1712,6 @@ void p2m_mem_paging_resume(struct domain *d,
> vm_event_response_t *rsp)
> }
> }
>
> -void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> -{
> -if ( altp2m_active(v->domain) )
> -p2m_switch_vcpu_altp2m_by_id(v, idx);
> -}
> -
> #ifdef CONFIG_HVM
> static struct p2m_domain *
> p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
> @@ -2165,6 +2159,14 @@ int unmap_mmio_regions(struct domain *d,
> return i == nr ? 0 : i ?: ret;
> }
>
> +#ifdef CONFIG_HVM
> +
> +void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> +if ( altp2m_active(v->domain) )
> +p2m_switch_vcpu_altp2m_by_id(v, idx);
> +}
> +
> bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
> {
> struct domain *d = v->domain;
> @@ -2542,6 +2544,7 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t
> gfn,
>
> return ret;
> }
> +#endif /* CONFIG_HVM */
>
> /*** Audit ***/
>
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 144ab81..4d06c8f 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -429,9 +429,11 @@ void vm_event_resume(struct domain *d, struct
> vm_event_domain *ved)
> */
> vm_event_toggle_singlestep(d, v, );
>
> +#ifdef CONFIG_HVM
> /* Check for altp2m switch */
> if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
> p2m_altp2m_check(v, rsp.altp2m_idx);
> +#endif
>
> if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
> vm_event_set_registers(v, );
> diff --git a/xen/include/asm-x86/hvm/domain.h
> b/xen/include/asm-x86/hvm/domain.h
> index 5885950..4517f89 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -202,7 +202,11 @@ struct hvm_domain {
> };
> };
>
> +#ifdef CONFIG_HVM
> #define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled)
> +#else
> +#define hap_enabled(d) false
> +#endif
Hmm... I thought I had rewritten this to be a static inline function.
I will try to make it a static inline function.
Wei.
>
> #endif /* __ASM_X86_HVM_DOMAIN_H__ */
>
> --
> git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] incompatible changes in staging break qemu
>>> On 27.08.18 at 08:43, wrote: > Since about two weeks, no released qemu can be built against xen.git#staging. > The error looks like that: > > qemu-20180825T130857.235c82acca/include/hw/xen/xen_common.h:677:5: error: too > many arguments to function 'xc_domain_create' > > It looks like staging lacks proper compat wrappers for released qemu > versions. I think it's the other way around - qemu needs such wrappers added (which of course for released versions of qemu means they'd need backporting of such an addition). After all the libxc interface, other than the libxl one, is not stable iirc. However - is the code controlled by CONFIG_XEN_PV_DOMAIN_BUILD actually used by anyone? So far I was under the impression that it's the core tool stack's job to create domains, no qemu's. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RESEND v3 1/2] x86/mmcfg: Rename pt_pci_init() and call it in acpi_mmcfg_init()
在 2018/8/27 16:16, Jan Beulich 写道: On 22.08.18 at 11:16, wrote: Given what pt_pci_init() actually does, rename it properly and move its declaration to pci.h, move the only call in acpi_mmcfg_init(). No functional change. Signed-off-by: Zhenzhong Duan Tested-by: Gopalasetty, Manoj Acked-by: Jan Beulich Please avoid needless re-sending of patches that have already gone in. Roger's remark regarding the questionable solitary v3 2/2 was correct, just that you should have dropped the 2/2 instead of re-sending an already applied patch. Ok. To avoid endless resending, I'll follow that way next time. Thanks Zhenzhong ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v17 11/13] x86/domctl: Use hvm_save_vcpu_handler
>>> On 22.08.18 at 16:28, wrote:
> On Wed, Aug 22, 2018 at 05:02:41PM +0300, Alexandru Isaila wrote:
>> @@ -223,17 +222,37 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
>> /* Save all available kinds of state */
>> for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ )
>> {
>> -handler = hvm_sr_handlers[i].save;
>> -if ( handler != NULL )
>> +struct vcpu *v;
>> +hvm_save_vcpu_handler save_one_handler =
>> hvm_sr_handlers[i].save_one;
>> +hvm_save_handler handler = hvm_sr_handlers[i].save;;
>
> Double ';'.
>
> Also, I would expect that a device either has a global save handler
> (handler) or a per-vcpu save handler (save_one_handler), but not both
> at the same time?
>
> In which case, I would add something like:
>
> ASSERT(save_one_handler == NULL || handler == NULL);
>
> In order to make sure both are not set at the same time.
I'm not sure this is worthwhile - the next patch would remove it
again right away.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Unable to build staging-4.9 on RHEL6
>>> On 24.08.18 at 04:56, wrote: > When trying to build both xen and qemu-xen from the staging-4.9 > branches, I'm running into issues compiling. > > Errors start with: > > BUILDSTDERR: sse.c: In function 'simd_test': > BUILDSTDERR: sse.c:319: error: subscripted value is neither array nor > pointer That's the x86 insn emulator test harness afaict, which doesn't get built unless you explicitly ask for it. Why are you building it? It's well known that it requires a new enough compiler (and the bar will raise with AVX512 support, which is in the works). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RESEND v3 1/2] x86/mmcfg: Rename pt_pci_init() and call it in acpi_mmcfg_init()
>>> On 22.08.18 at 11:16, wrote: > Given what pt_pci_init() actually does, rename it properly and move its > declaration to pci.h, move the only call in acpi_mmcfg_init(). > > No functional change. > > Signed-off-by: Zhenzhong Duan > Tested-by: Gopalasetty, Manoj > Acked-by: Jan Beulich Please avoid needless re-sending of patches that have already gone in. Roger's remark regarding the questionable solitary v3 2/2 was correct, just that you should have dropped the 2/2 instead of re-sending an already applied patch. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
Marc-André Lureau writes:
> Hi
> On Fri, Aug 24, 2018 at 9:37 AM Markus Armbruster wrote:
>>
>> Marc-André Lureau writes:
>>
>> > This is mostly for readability of the code. Let's make it clear which
>> > callers can create an implicit monitor when the chardev is muxed.
>> >
>> > This will also enforce a safer behaviour, as we don't really support
>> > creating monitor anywhere/anytime at the moment.
>> >
>> > There are documented cases, such as: -serial/-parallel/-virtioconsole
>> > and to less extent -debugcon.
>> >
>> > Less obvious and questionnable ones are -gdb and Xen console. Add a
>> > FIXME note for those, but keep the support for now.
>> >
>> > Other qemu_chr_new() callers either have a fixed parameter/filename
>> > string, or do preliminary checks on the string (such as slirp), or do
>> > not need it, such as -qtest.
>> >
>> > On a related note, the list of monitor creation places:
>> >
>> > - the chardev creators listed above: all from command line (except
>> > perhaps Xen console?)
>> >
>> > - -gdb & hmp gdbserver will create a "GDB monitor command" chardev
>> > that is wired to an HMP monitor.
>> >
>> > - -mon command line option
>>
>> Aside: the question "what does it mean to connect a monitor to a chardev
>> that has an implicit monitor" crosses my mind. Next thought is "the
>> day's too short to go there".
>>
>> > From this short study, I would like to think that a monitor may only
>> > be created in the main thread today, though I remain skeptical :)
>>
>> Hah!
>>
>> > Signed-off-by: Marc-André Lureau
>> > ---
>> > include/chardev/char.h | 18 +-
>> > chardev/char.c | 21 +
>> > gdbstub.c | 6 +-
>> > hw/char/xen_console.c | 5 -
>> > vl.c | 8
>> > 5 files changed, 47 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/include/chardev/char.h b/include/chardev/char.h
>> > index 6f0576e214..be2b9b817e 100644
>> > --- a/
>> > +++ b/include/chardev/char.h
>> > @@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
>> > * @qemu_chr_new:
>> > *
>> > * Create a new character backend from a URI.
>> > + * Do not implicitely initialize a monitor if the chardev is muxed.
>> > *
>> > * @label the name of the backend
>> > * @filename the URI
>> > @@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
>> > */
>> > Chardev *qemu_chr_new(const char *label, const char *filename);
>> >
>> > +/**
>> > + * @qemu_chr_new_mux_mon:
>> > + *
>> > + * Create a new character backend from a URI.
>> > + * Implicitely initialize a monitor if the chardev is muxed.
>> > + *
>> > + * @label the name of the backend
>> > + * @filename the URI
>>
>> I'm no friend of GTK-Doc style, but where we use it, we should use it
>> correctly: put a colon after @param.
>
> I copied existing comment... Should I fixed all others in the headers?
Do we expect to actually *use* doc comments for anything? We haven't in
a decade or so, but if we still expect to all the same, then not fixing
them up now merely delays the inevitable.
Do we think adding the colons makes the comments easier to read? For
what it's worth, I do.
Decision's up to you.
>> > + *
>> > + * Returns: a new character backend
>> > + */
>> > +Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename);
>> > +
>> > /**
>> > * @qemu_chr_change:
>> > *
>> > @@ -138,10 +152,12 @@ void qemu_chr_cleanup(void);
>> > *
>> > * @label the name of the backend
>> > * @filename the URI
>> > + * @with_mux_mon if chardev is muxed, initialize a monitor
>> > *
>> > * Returns: a new character backend
>> > */
>> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
>> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
>> > + bool with_mux_mon);
>> >
>> > /**
>> > * @qemu_chr_be_can_write:
>> > diff --git a/chardev/char.c b/chardev/char.c
>> > index 76d866e6fe..2c295ad676 100644
>> > --- a/chardev/char.c
>> > +++ b/chardev/char.c
>> > @@ -683,7 +683,8 @@ out:
>> > return chr;
>> > }
>> >
>> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename)
>> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
>> > + bool with_mux_mon)
>> > {
>> > const char *p;
>> > Chardev *chr;
>> > @@ -702,17 +703,19 @@ Chardev *qemu_chr_new_noreplay(const char *label,
>> > const char *filename)
>> > if (err) {
>> > error_report_err(err);
>> > }
>> > -if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
>> > +if (with_mux_mon && chr && qemu_opt_get_bool(opts, "mux", 0)) {
>> > monitor_init(chr, MONITOR_USE_READLINE);
>> > }
>>
>> When !chr, the function already failed, so that part of the condition is
>> uninteresting here[*].
>
> yeah, hopefully err is always set if the return value is NULL.
>
>> That leaves
Re: [Xen-devel] [xen-4.9-testing test] 126201: regressions - FAIL
>>> On 24.08.18 at 10:58, wrote: > On Wed, Aug 22, 2018 at 04:52:27PM -0600, Jim Fehlig wrote: >> What could be causing the long startup time of qemu on these hosts? Does >> dom0 have enough cpu/memory? As you noticed, the libvirt commit used for >> this test has not changed in a long time, well before the failures appeared. >> Perhaps a subtle change in libxl is exposing the bug? > > There have only been two changes to libxl in the range of changesets > being tested. > > c257e35a libxl: qemu_disk_scsi_drive_string: Break out common parts of disk > config > 5d92007c libxl: restore passing "readonly=" to qemu for SCSI disks > > They wouldn't change how libxl interact with libvirt. QEMU tag didn't > change. I'm afraid this is an unhelpful perspective to take: The issue apparently being host-specific, a possible commit having exposed the bad behavior may have passed the push gate long ago, due to the test having been performed on another host. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 25/34] x86/mm/shadow: make it build with !CONFIG_HVM
>>> On 26.08.18 at 13:04, wrote:
> On Tue, Aug 21, 2018 at 02:27:40AM -0600, Jan Beulich wrote:
>>
>> > --- a/xen/arch/x86/mm/shadow/multi.c
>> > +++ b/xen/arch/x86/mm/shadow/multi.c
>> > @@ -2926,18 +2926,25 @@ static int sh_page_fault(struct vcpu *v,
>> > }
>> > else
>> > {
>> > +#if CONFIG_HVM
>> > /* Magic MMIO marker: extract gfn for MMIO address */
>> > ASSERT(sh_l1e_is_mmio(sl1e));
>> > +ASSERT(is_hvm_vcpu(v));
>> > gpa = (((paddr_t)(gfn_x(sh_l1e_mmio_get_gfn(sl1e
>> > << PAGE_SHIFT)
>> > | (va & ~PAGE_MASK);
>> > +perfc_incr(shadow_fault_fast_mmio);
>> > +SHADOW_PRINTK("fast path mmio %#"PRIpaddr"\n", gpa);
>> > +sh_reset_early_unshadow(v);
>> > +trace_shadow_gen(TRC_SHADOW_FAST_MMIO, va);
>> > +return handle_mmio_with_translation(va, gpa >> PAGE_SHIFT,
>> > +access)
>> > +? EXCRET_fault_fixed : 0;
>> > +#else
>> > +/* When HVM is not enabled, there shouldn't be MMIO
>> > marker */
>> > +BUG();
>>
>> At the example of this, while I agree we shouldn't reach here for PV,
>> can this nevertheless be the less impactful domain_crash() please?
>>
>
> Do you only want this BUG() to be replaced?
>
> I think the two in shadonw_*_emulation should stay because you will only
> ever get NULL pointer deref if you allow the code to continue.
Did you perhaps remove too much context? From what's left I can't
judge which others you refer to, or what NULL deref you talk about.
Looking back at the full patch - I think I had already suggested that
the two shadow_*_emulation() should altogether go inside #ifdef
CONFIG_HVM, not just their bodies.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
Am 26.08.2018 um 10:40 schrieb Tetsuo Handa:
On 2018/08/24 22:52, Michal Hocko wrote:
@@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
*/
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
- if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
- return -EAGAIN;
-
+ /*
+* We can take sleepable lock even on !blockable mode because
+* read_lock is only ever take from this path and the notifier
+* lock never really sleeps. In fact the only reason why the
+* later is sleepable is because the notifier itself might sleep
+* in amdgpu_mn_invalidate_node but blockable mode is handled
+* before calling into that path.
+*/
+ mutex_lock(>read_lock);
if (atomic_inc_return(>recursion) == 1)
down_read_non_owner(>lock);
mutex_unlock(>read_lock);
I'm not following. Why don't we need to do like below (given that
nobody except amdgpu_mn_read_lock() holds ->read_lock) because e.g.
drm_sched_fence_create() from drm_sched_job_init() from amdgpu_cs_submit()
is doing GFP_KERNEL memory allocation with ->lock held for write?
That's a bug which needs to be fixed separately.
Allocating memory with GFP_KERNEL while holding a lock which is also
taken in the reclaim code path is illegal not matter what you do.
Patches to fix this are already on the appropriate mailing list and will
be pushed upstream today.
Regards,
Christian.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index e55508b..e1cb344 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -64,8 +64,6 @@
* @node: hash table node to find structure by adev and mn
* @lock: rw semaphore protecting the notifier nodes
* @objects: interval tree containing amdgpu_mn_nodes
- * @read_lock: mutex for recursive locking of @lock
- * @recursion: depth of recursion
*
* Data for each amdgpu device and process address space.
*/
@@ -85,8 +83,6 @@ struct amdgpu_mn {
/* objects protected by lock */
struct rw_semaphore lock;
struct rb_root_cached objects;
- struct mutexread_lock;
- atomic_trecursion;
};
/**
@@ -181,14 +177,9 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
{
if (blockable)
- mutex_lock(>read_lock);
- else if (!mutex_trylock(>read_lock))
+ down_read(>lock);
+ else if (!down_read_trylock(>lock))
return -EAGAIN;
-
- if (atomic_inc_return(>recursion) == 1)
- down_read_non_owner(>lock);
- mutex_unlock(>read_lock);
-
return 0;
}
@@ -199,8 +190,7 @@ static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable)
*/
static void amdgpu_mn_read_unlock(struct amdgpu_mn *amn)
{
- if (atomic_dec_return(>recursion) == 0)
- up_read_non_owner(>lock);
+ up_read(>lock);
}
/**
@@ -410,8 +400,6 @@ struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev,
amn->type = type;
amn->mn.ops = _mn_ops[type];
amn->objects = RB_ROOT_CACHED;
- mutex_init(>read_lock);
- atomic_set(>recursion, 0);
r = __mmu_notifier_register(>mn, mm);
if (r)
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
