Re: [Xen-devel] Design session report: Live-Updating Xen
On 7/15/19 8:48 PM, Juergen Gross wrote: On 15.07.19 21:31, Sarah Newman wrote: On 7/15/19 11:57 AM, Foerster, Leonard wrote: ... A key cornerstone for Live-update is guest transparent live migration ... -> for live migration: domid is a problem in this case -> randomize and pray does not work on smaller fleets -> this is not a problem for live-update -> BUT: as a community we shoudl make this restriction go away Andrew Cooper pointed out to me that manually assigning domain IDs is supported in much of the code already. If guest transparent live migration gets merged, we'll look at passing in a domain ID to xl, which would be good enough for us. I don't know about the other toolstacks. The main problem is the case where on the target host the domid of the migrated domain is already in use by another domain. So you either need a domid allocator spanning all hosts or the change of domid during migration must be hidden from the guest for guest transparent migration. Yes. There are some cluster management systems which use xl rather than xapi. They could be extended to manage domain IDs if it's too difficult to allow the domain ID to change during migration. --Sarah ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On 15.07.19 19:39, Andrew Cooper wrote: On 15/07/2019 18:28, Andy Lutomirski wrote: On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen wrote: Juergen Gross writes: The long term plan has been to replace Xen PV guests by PVH. The first victim of that plan are now 32-bit PV guests, as those are used only rather seldom these days. Xen on x86 requires 64-bit support and with Grub2 now supporting PVH officially since version 2.04 there is no need to keep 32-bit PV guest support alive in the Linux kernel. Additionally Meltdown mitigation is not available in the kernel running as 32-bit PV guest, so dropping this mode makes sense from security point of view, too. Normally we have a deprecation period for feature removals like this. You would make the kernel print a warning for some releases, and when no user complains you can then remove. If a user complains you can't. As I understand it, the kernel rules do allow changes like this even if there's a complaint: this is a patch that removes what is effectively hardware support. If the maintenance cost exceeds the value, then removal is fair game. (Obviously we weight the value to preserving compatibility quite highly, but in this case, Xen dropped 32-bit hardware support a long time ago. If the Xen hypervisor says that 32-bit PV guest support is deprecated, it's deprecated.) That being said, a warning might not be a bad idea. What's the current status of this in upstream Xen? So personally, I'd prefer to see support stay, but at the end of the day it is Juergen's choice as the maintainer of the code. Especially on the security front we are unsafe with 32-bit PV Linux. And making it safe will make it so slow that the needed effort is not spent very well. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support
On 15.07.19 17:44, Boris Ostrovsky wrote: diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile index 084de77a109e..d42737f31304 100644 --- a/arch/x86/xen/Makefile +++ b/arch/x86/xen/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y +OBJECT_FILES_NON_STANDARD_xen-asm_64.o := y ifdef CONFIG_FUNCTION_TRACER # Do not profile debug and lowlevel utilities @@ -34,7 +34,7 @@ obj-$(CONFIG_XEN_PV) += mmu_pv.o obj-$(CONFIG_XEN_PV) += irq.o obj-$(CONFIG_XEN_PV) += multicalls.o obj-$(CONFIG_XEN_PV) += xen-asm.o -obj-$(CONFIG_XEN_PV) += xen-asm_$(BITS).o +obj-$(CONFIG_XEN_PV) += xen-asm_64.o We should be able to merge xen-asm_64.S into xen-asm.S, shouldn't we? Yes, probably a good idea to add that. @@ -1312,15 +1290,7 @@ asmlinkage __visible void __init xen_start_kernel(void) /* keep using Xen gdt for now; no urgent need to change it */ -#ifdef CONFIG_X86_32 - pv_info.kernel_rpl = 1; - if (xen_feature(XENFEAT_supervisor_mode_kernel)) - pv_info.kernel_rpl = 0; -#else pv_info.kernel_rpl = 0; Is kernel_rpl needed anymore? Yes, this can be dropped, together with get_kernel_rpl(). Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Live-Updating Xen
On 15.07.19 21:31, Sarah Newman wrote: On 7/15/19 11:57 AM, Foerster, Leonard wrote: ... A key cornerstone for Live-update is guest transparent live migration ... -> for live migration: domid is a problem in this case -> randomize and pray does not work on smaller fleets -> this is not a problem for live-update -> BUT: as a community we shoudl make this restriction go away Andrew Cooper pointed out to me that manually assigning domain IDs is supported in much of the code already. If guest transparent live migration gets merged, we'll look at passing in a domain ID to xl, which would be good enough for us. I don't know about the other toolstacks. The main problem is the case where on the target host the domid of the migrated domain is already in use by another domain. So you either need a domid allocator spanning all hosts or the change of domid during migration must be hidden from the guest for guest transparent migration. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 139014: regressions - FAIL
flight 139014 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/139014/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 138977 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 138977 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 138977 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 138977 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 138977 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 138977 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 138977 test-amd64-i386-xl-pvshim12 guest-start fail 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-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-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-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 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-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-qemuu-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 version targeted for testing: qemuu46cd24e7ed38191b5ab5c40a836d6c5b6b604f8a baseline version: qemuu1316b1ddc8a05e418c8134243f8bff8cccbbccb1 Last test of basis 138977 2019-07-14 03:43:52 Z1 days Testing same since 139014 2019-07-15 09:06:23 Z0 days1 attempts People who touched revisions under test: Dr. David Alan Gilbert Igor Mammedov Michael S. Tsirkin Pankaj Gupta Peter Maydell Stefan Hajnoczi Wolfgang Bumiller jobs: build-amd64-xsm pass
[Xen-devel] [xen-unstable test] 139010: tolerable FAIL
flight 139010 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/139010/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-examine 11 examine-serial/bootloaderfail like 138915 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 138986 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 138986 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 138986 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 138986 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 138986 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 138986 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 138986 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 138986 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 138986 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-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt 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-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail 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-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 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 38eeb3864de40aa568c48f9f26271c141c62b50b baseline version: xen 38eeb3864de40aa568c48f9f26271c141c62b50b Last test of basis 139010 2019-07-15 07:01:24 Z0 days Testing same since (not found) 0
[Xen-devel] [ovmf test] 139011: all pass - PUSHED
flight 139011 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/139011/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf eebc135ffb210c6da7133145ba9e5423cafc13d4 baseline version: ovmf 70565e64227dfa254d8a0703dd60dc74bd8b5e6e Last test of basis 138998 2019-07-14 17:21:25 Z1 days Testing same since 139011 2019-07-15 07:47:26 Z0 days1 attempts People who touched revisions under test: Cole Robinson 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 70565e6422..eebc135ffb eebc135ffb210c6da7133145ba9e5423cafc13d4 -> 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] 139016: all pass - PUSHED
flight 139016 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/139016/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd 163feb2e45cd088e65da1ff395dc3293065a4603 baseline version: freebsd 5c4a9b0e32c1f9c47d5b687d6036bb03c3cc071c Last test of basis 138886 2019-07-10 09:19:38 Z5 days Failing since138921 2019-07-12 09:19:32 Z3 days2 attempts Testing same since 139016 2019-07-15 09:19:57 Z0 days1 attempts People who touched revisions under test: ae alc avg chuck cy dim dougm ian imp jhibbits kib luporl markj np philip phk rrs scottl seanc sjg tijl tuexen vmaffione jobs: build-amd64-freebsd-againpass build-amd64-freebsd pass build-amd64-xen-freebsd 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/freebsd.git 5c4a9b0e32c..163feb2e45c 163feb2e45cd088e65da1ff395dc3293065a4603 -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 139003: regressions - FAIL
flight 139003 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/139003/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-i386-pvgrub 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-libvirt 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-qemuu-nested-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-examine 8 reboot fail REGR. vs. 133580 test-amd64-amd64-examine 8 reboot fail REGR. vs. 133580 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 133580 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 133580 test-amd64-amd64-xl-qemut-debianhvm-amd64 7 xen-bootfail REGR. vs. 133580 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-credit2 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-pvhv2-amd 7 xen-bootfail REGR. vs. 133580 test-amd64-amd64-qemuu-nested-amd 7 xen-bootfail REGR. vs. 133580 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-qemuu-debianhvm-amd64 7 xen-bootfail REGR. vs. 133580 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-credit1 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 133580 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 133580
[Xen-devel] [linux-4.19 test] 139004: regressions - FAIL
flight 139004 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/139004/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 129313 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail REGR. vs. 129313 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-examine 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail 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-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-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-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail 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 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: linux3bd837bfe431839a378e9d421af05b2e22a6d329 baseline version: linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d Last test of basis 129313 2018-11-02 05:39:08 Z 255 days Failing since129412 2018-11-04 14:10:15 Z 253 days 159 attempts Testing same since 139004 2019-07-14 23:35:54 Z0 days1 attempts 2271 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64
Re: [Xen-devel] Design session report: Live-Updating Xen
On 7/15/19 11:57 AM, Foerster, Leonard wrote: ... A key cornerstone for Live-update is guest transparent live migration ... -> for live migration: domid is a problem in this case -> randomize and pray does not work on smaller fleets -> this is not a problem for live-update -> BUT: as a community we shoudl make this restriction go away Andrew Cooper pointed out to me that manually assigning domain IDs is supported in much of the code already. If guest transparent live migration gets merged, we'll look at passing in a domain ID to xl, which would be good enough for us. I don't know about the other toolstacks. --Sarah ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg
Hi Julien, Julien Grall writes: > At the moment, the user should save x30/lr if it cares about it. > > Follow-up patches will introduce more use of putn in place where lr > should be preserved. > > Furthermore, any user of putn should also move the value to register x0 > if it was stored in a different register. > > For convenience, a new macro is introduced to print a given register. > The macro will take care for us to move the value to x0 and also > preserve lr. > > Lastly the new macro is used to replace all the callsite of putn. This > will simplify rework/review later on. > > Note that CurrentEL is now stored in x5 instead of x4 because the latter > will be clobbered by the macro print_reg. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/arm64/head.S | 29 ++--- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 84e26582c4..9142b4a774 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -90,8 +90,25 @@ > blputs; \ > mov lr, x3 ; \ > RODATA_STR(98, _s) > + > +/* > + * Macro to print the value of register \xb > + * > + * Clobbers x0 - x4 > + */ Despite its name, this macro can't print x4. I would recommend adding at least comment about this. Static assertion would be even better, but looks like we don't have them for asm code. > +.macro print_reg xb > +mov x4, lr > +mov x0, \xb > +blputn > +mov lr, x4 > +.endm > + > #else /* CONFIG_EARLY_PRINTK */ > #define PRINT(s) > + > +.macro print_reg xb > +.endm > + > #endif /* !CONFIG_EARLY_PRINTK */ > > /* Load the physical address of a symbol into xb */ > @@ -304,22 +321,20 @@ GLOBAL(init_secondary) > #ifdef CONFIG_EARLY_PRINTK > ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ > PRINT("- CPU ") > -mov x0, x24 > -blputn > +print_reg x24 > PRINT(" booting -\r\n") > #endif > > common_start: > > PRINT("- Current EL ") > -mrs x4, CurrentEL > -mov x0, x4 > -blputn > +mrs x5, CurrentEL > +print_reg x5 > PRINT(" -\r\n") > > /* Are we in EL2 */ > -cmp x4, #PSR_MODE_EL2t > -ccmp x4, #PSR_MODE_EL2h, #0x4, ne > +cmp x5, #PSR_MODE_EL2t > +ccmp x5, #PSR_MODE_EL2h, #0x4, ne > b.eq el2 /* Yes */ > > /* OK, we're boned. */ -- Best regards, Volodymyr Babchuk ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Design session report: Further defences for speculative sidechannels
Some numpty (me) forgot to organise a scribe for the session, so these notes are from memory. As a result, all aspects are up for argumen^W debate. We started by discussing Core-aware scheduling, and covering the point that while in principle it makes an attackers life easier, it doesn't make a difference in practice. A determined attacker won't be put off by the extra stats required to determine co-residency, and there are plenty of research papers published with various techniques in this area. We also discussed synchronised scheduling. Part of this was an explanation of why it is believed to be safe alternative to disabling HT. Part was a discussion of the performance aspects and why the perf hit is substantially worse when virtualised than on native, due to a lack of virtualised IPI support on affected hardware. Some discussion went on MSR_ARCH_CAPS virtualisation support, and support for eIBRS. This work is already on a TODO list, and no concerns were raised. The area which took up a large part of the discussion was the current state of "l1tf_barrier". In current staging, we are compiling code which takes all of the performance hit, and gains none of the safety. This is because we fought the optimiser, and the optimiser won. The use of inline assembly is causing the optimiser to out-of-line the protected predicates, which causes the LFENCE to be emitted in the instruction stream *before* the Jcc trying to be protected, which renders the protections useless. Annotating all predicates with always_inline is believed to resolve the issue, but this is almost impossible to spot during review, as all it takes is one intermediate non-always_inline predicate to break the security all over again. An alternative to the current approach was raised, which involves using a compiler extension. Linux has used compiler extensions for several releases now, and forms the basis of several KSPP features/mitigations. It is expected that we would be able to express the required protections using a compiler plugin rather than with inline assembly, which provides a much cleaner argument concerning correctness, and makes it less likely that errors will occur due to fighting with the optimiser. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: merge make_timer_node and make_timer_domU_node
Hi Viktor, On 20/06/2019 11:38, Viktor Mitin wrote: Functions make_timer_node and make_timer_domU_node are quite similar. The only difference between Dom0 and DomU timer DT node is the timer interrupts used. All the rest code should be the same. This is a bit confusing to read. First you say there are only "one difference" but then the second part leads to think there are more difference. The commit message should describe what are the differences and which version you are keeping. So it is better to merge them to avoid discrepancy. Tested dom0 boot with rcar h3 sk board. How about domU support? Also, do you have the clock-frequency property in your DT timer node? Suggested-by: Julien Grall Signed-off-by: Viktor Mitin --- xen/arch/arm/domain_build.c | 66 - 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 7fb828cae2..610dd3e8e7 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -976,6 +976,8 @@ static int __init make_timer_node(const struct domain *d, void *fdt) gic_interrupt_t intrs[3]; u32 clock_frequency; bool clock_valid; +bool d0 = is_hardware_domain(d); Please avoid to use the term "d0"/"dom0" whenever it is possible. While in practice the hardware domain is always dom0 on Arm, I want to avoid the mixing. +uint32_t ip_val; dt_dprintk("Create timer node\n"); I am not sure where to comment, but the compatible will be different for DomU now. I would actually prefer if we keep the domU version for the compatible as it is simpler. @@ -1004,22 +1006,36 @@ static int __init make_timer_node(const struct domain *d, void *fdt) /* The timer IRQ is emulated by Xen. It always exposes an active-low * level-sensitive interrupt */ -irq = timer_get_irq(TIMER_PHYS_SECURE_PPI); +irq = d0 +? timer_get_irq(TIMER_PHYS_SECURE_PPI) +: GUEST_TIMER_PHYS_S_PPI; Rather than using ternary everywhere, how about introducing an array where the value will be stored. So the code would look like: if ( is_hardware_domain(...) ) { timer_irq[...] = ...; timer_irq[...] = ...; } else { } [] set_interrupt(timer_irq[...]); Have a look at time.c as we define handy value (enum timer_ppi and MAX_TIMER_PPI). dt_dprintk(" Secure interrupt %u\n", irq); set_interrupt(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW); -irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI); +irq = d0 +? timer_get_irq(TIMER_PHYS_NONSECURE_PPI) +: GUEST_TIMER_PHYS_NS_PPI; dt_dprintk(" Non secure interrupt %u\n", irq); set_interrupt(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW); -irq = timer_get_irq(TIMER_VIRT_PPI); +irq = d0 +? timer_get_irq(TIMER_VIRT_PPI) +: GUEST_TIMER_VIRT_PPI; dt_dprintk(" Virt interrupt %u\n", irq); set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW); -res = fdt_property_interrupts(fdt, intrs, 3); +res = fdt_property(fdt, "interrupts", intrs, sizeof (intrs[0]) * 3); if ( res ) return res; +ip_val = d0 + ? dt_interrupt_controller->phandle + : GUEST_PHANDLE_GIC; + +res = fdt_property_cell(fdt, "interrupt-parent", ip_val); I would actually prefer if we extend fdt_property_interrupts to deal with other domain than the hwdom. This would avoid the function. +if (res) NIT: Coding style if ( ... ) Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design Session report: Toolstacks
On Mon, 2019-07-15 at 14:59 +0100, George Dunlap wrote: > Looking through the notes, it seems like the first part of this > discussion, re hypervisor upgrade / downgrade & libraries, was partly > covered in the distro session, in which Debian's Xen version co- > install > was discussed and found useful even if we had a hypervisor , Ian > Jackson > agreed to post Debian's co-install patches. Yea. That's also useful for the live-update project, where we want to (in the future) use the userspace tools and libraries depending on which Xen version is in use at a time. It doesn't need to be much smarter than symlinks for binaries at the first go, and the symlinks updated each time a live-update operation succeeds. Beyond that, for libraries, we'll have to do much smarter things eventually. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Xen on Distros
On Mon, 2019-07-15 at 14:52 +, Jan Beulich wrote: > On 15.07.2019 16:42, George Dunlap wrote: > > On 7/15/19 3:23 PM, Jan Beulich wrote: > > > On 15.07.2019 16:11, George Dunlap wrote: > > > > There was a long discussion about security patches, with the > > > > general > > > > proposal being that we should cut a point release for every > > > > security issue. > > > > > > Interesting. Looks like in politics that until a decision fits > > > people > > > they keep re-raising the point. Iirc on a prior meeting > > > (Budapest?) > > > we had settled on continuing with the current scheme. Were there > > > any > > > new arguments towards this alternative model? > > > > Well I don't know if there were any new arguments because I don't > > immediately remember the old discussion. Do we have a summary of > > the > > discussion in Budapest, with its conclusions, anywhere? > > I don't recall if suitable notes were taken back then; as indicated > I'm not even sure which meeting it was at. > > > The basic idea was that: > > > > 1. Most distros / packagers are going to want to do an immediate > > release > > anyway. > > > > 2. Distros generally seemed to be rebasing on top of staging as > > soon as > > the XSA went out anyway (and ISTR this being the recommeneded > > course of > > action) > > > > So for all intents and purposes, we have something which is, in > > fact, a > > release; all it's missing is a signed tag and a tarball. > > > > Obviously there are testing implications that would need to be > > sorted > > out before this could become a reality. > > > > In any case, the ball is in the court of "VOLUNTEER" to write up a > > concrete proposal which could be discussed. You'll be able to > > raise all > > your concerns at that point if you want (although having a sketch > > would > > of course be helpful for whoever is writing such a proposal). > > Sure - I realized soon after having sent the initial reply that > perhaps > this was the wrong context in the first place to raise my question. In any case, I'd like to know why it doesn't make sense for Xen to have a point release frequently, and not have a point release after an XSA above some severity level (pick one - high/critical/important). As George mentioned, distros have to do it anyway, and the upstream project not doing it only makes it more difficult for all distros involved. Not sure of the politics involved though, and what can of worms this opens. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On 15.07.19 19:28, Andy Lutomirski wrote: On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen wrote: Juergen Gross writes: The long term plan has been to replace Xen PV guests by PVH. The first victim of that plan are now 32-bit PV guests, as those are used only rather seldom these days. Xen on x86 requires 64-bit support and with Grub2 now supporting PVH officially since version 2.04 there is no need to keep 32-bit PV guest support alive in the Linux kernel. Additionally Meltdown mitigation is not available in the kernel running as 32-bit PV guest, so dropping this mode makes sense from security point of view, too. Normally we have a deprecation period for feature removals like this. You would make the kernel print a warning for some releases, and when no user complains you can then remove. If a user complains you can't. As I understand it, the kernel rules do allow changes like this even if there's a complaint: this is a patch that removes what is effectively hardware support. If the maintenance cost exceeds the value, then removal is fair game. (Obviously we weight the value to preserving compatibility quite highly, but in this case, Xen dropped 32-bit hardware support a long time ago. If the Xen hypervisor says that 32-bit PV guest support is deprecated, it's deprecated.) That being said, a warning might not be a bad idea. What's the current status of this in upstream Xen? Xen still supports that. We have asked downstream for their opinion about dropping 32-bit PV guest support in the kernel about 1 year ago and the common answer was: no problem, but for users still wanting 32 bit guests we should wait until PVH support is available in all related products. Grub2 was the last one missing and as grub2 has released a version with PVH support I posted this small series now. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On 15/07/2019 18:28, Andy Lutomirski wrote: > On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen wrote: >> Juergen Gross writes: >> >>> The long term plan has been to replace Xen PV guests by PVH. The first >>> victim of that plan are now 32-bit PV guests, as those are used only >>> rather seldom these days. Xen on x86 requires 64-bit support and with >>> Grub2 now supporting PVH officially since version 2.04 there is no >>> need to keep 32-bit PV guest support alive in the Linux kernel. >>> Additionally Meltdown mitigation is not available in the kernel running >>> as 32-bit PV guest, so dropping this mode makes sense from security >>> point of view, too. >> Normally we have a deprecation period for feature removals like this. >> You would make the kernel print a warning for some releases, and when >> no user complains you can then remove. If a user complains you can't. >> > As I understand it, the kernel rules do allow changes like this even > if there's a complaint: this is a patch that removes what is > effectively hardware support. If the maintenance cost exceeds the > value, then removal is fair game. (Obviously we weight the value to > preserving compatibility quite highly, but in this case, Xen dropped > 32-bit hardware support a long time ago. If the Xen hypervisor says > that 32-bit PV guest support is deprecated, it's deprecated.) > > That being said, a warning might not be a bad idea. What's the > current status of this in upstream Xen? So personally, I'd prefer to see support stay, but at the end of the day it is Juergen's choice as the maintainer of the code. Xen itself has been exclusively 64-bit since Xen 4.3 (released in 2013). Over time, various features like SMEP/SMAP have been making 32bit PV guests progressively slower, because ring 1 is supervisor rather than user. Things have got even worse with IBRS, to the point at which 32bit PV guests are starting to run like treacle. There are no current plans to remove support for 32bit PV guests from Xen, but it is very much in the category of "you shouldn't be using this mode any more". ~Andrew P.S. I don't see 64bit PV guest support going anywhere, because there are still a number of open performance questions due to the inherent differences between syscall and vmexit, and the difference EPT/NPT tables make on cross-domain mappings. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen wrote: > > Juergen Gross writes: > > > The long term plan has been to replace Xen PV guests by PVH. The first > > victim of that plan are now 32-bit PV guests, as those are used only > > rather seldom these days. Xen on x86 requires 64-bit support and with > > Grub2 now supporting PVH officially since version 2.04 there is no > > need to keep 32-bit PV guest support alive in the Linux kernel. > > Additionally Meltdown mitigation is not available in the kernel running > > as 32-bit PV guest, so dropping this mode makes sense from security > > point of view, too. > > Normally we have a deprecation period for feature removals like this. > You would make the kernel print a warning for some releases, and when > no user complains you can then remove. If a user complains you can't. > As I understand it, the kernel rules do allow changes like this even if there's a complaint: this is a patch that removes what is effectively hardware support. If the maintenance cost exceeds the value, then removal is fair game. (Obviously we weight the value to preserving compatibility quite highly, but in this case, Xen dropped 32-bit hardware support a long time ago. If the Xen hypervisor says that 32-bit PV guest support is deprecated, it's deprecated.) That being said, a warning might not be a bad idea. What's the current status of this in upstream Xen? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/doc: update ARM warning about testing gcov on arm
On 15/07/2019 18:26, Julien Grall wrote: > Hi, > > On 10/07/2019 05:57, Viktor Mitin wrote: >> Update ARM code coverage warning about testing gcov on arm >> >> Signed-off-by: Viktor Mitin > > Acked-by: Julien Grall Docs parts Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/doc: update ARM warning about testing gcov on arm
Hi, On 10/07/2019 05:57, Viktor Mitin wrote: Update ARM code coverage warning about testing gcov on arm Signed-off-by: Viktor Mitin Acked-by: Julien Grall 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] xen/mm.h: add helper function to test-and-clear _PGC_allocated
Hi, On 15/07/2019 10:17, Paul Durrant wrote: The _PGC_allocated flag is set on a page when it is assigned to a domain along with an initial reference count of at least 1. To clear this 'allocation' reference it is necessary to test-and-clear _PGC_allocated and then only drop the reference if the test-and-clear succeeds. This is open- coded in many places. It is also unsafe to test-and-clear _PGC_allocated unless the caller holds an additional reference. This patch adds a helper function, put_page_alloc_ref(), to replace all the open-coded test-and-clear/put_page occurrences and incorporates in that a BUG_ON() an additional page reference not being held. Signed-off-by: Paul Durrant For the Arm bits: Acked-by: Julien Grall Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/suspend: Don't save/restore %cr8
%cr8 is an alias of APIC_TASKPRI, which is handled by lapic_{suspend,resume}() with the rest of the Local APIC state. Saving and restoring the TPR state in isolation is not a clever idea. Drop it all. While editing wakeup_prot.S, trim its include list to just the headers which are used, which is precicely none of them. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Juergen Gross This is a Xen mirror to my Linux patch of the same effect: https://lore.kernel.org/lkml/20190715151641.29210-1-andrew.coop...@citrix.com/T/#u With a bit of care, I'm pretty sure the whole of wakeup_prot.S can disappear, but -ETIME right now. I've confirmed that after resume TPR retains its value of 0x10. However, all attempts to debug the internals of lapic_suspend/resume have eluded me, including manually poking the UART. Again, -ETIME to investigate further. --- xen/arch/x86/acpi/wakeup_prot.S | 15 --- 1 file changed, 15 deletions(-) diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S index 361751d290..4a92627436 100644 --- a/xen/arch/x86/acpi/wakeup_prot.S +++ b/xen/arch/x86/acpi/wakeup_prot.S @@ -1,13 +1,5 @@ .file __FILE__ .text - -#include -#include -#include -#include -#include -#include - .code64 #define GREG(x) %r##x @@ -40,9 +32,6 @@ ENTRY(do_suspend_lowlevel) pushfq; popqSAVED_GREG(flags) -mov %cr8, GREG(ax) -mov GREG(ax), REF(saved_cr8) - mov %ss, REF(saved_ss) sgdtREF(saved_gdt) @@ -90,9 +79,6 @@ ENTRY(__ret_point) pushq %rax lretq 1: -mov REF(saved_cr8), %rax -mov %rax, %cr8 - pushq SAVED_GREG(flags) popfq @@ -149,4 +135,3 @@ saved_ldt: .quad 0,0 saved_cr0: .quad 0 saved_cr3: .quad 0 -saved_cr8: .quad 0 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
Juergen Gross writes: > The long term plan has been to replace Xen PV guests by PVH. The first > victim of that plan are now 32-bit PV guests, as those are used only > rather seldom these days. Xen on x86 requires 64-bit support and with > Grub2 now supporting PVH officially since version 2.04 there is no > need to keep 32-bit PV guest support alive in the Linux kernel. > Additionally Meltdown mitigation is not available in the kernel running > as 32-bit PV guest, so dropping this mode makes sense from security > point of view, too. Normally we have a deprecation period for feature removals like this. You would make the kernel print a warning for some releases, and when no user complains you can then remove. If a user complains you can't. -Andi ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 02/33] drm/xen: drop use of drmP.h
On Mon, Jul 01, 2019 at 08:05:24AM +0200, Sam Ravnborg wrote: > Hi Oleksandr > > > > --- a/drivers/gpu/drm/xen/xen_drm_front.h > > > +++ b/drivers/gpu/drm/xen/xen_drm_front.h > > > @@ -11,13 +11,19 @@ > > > #ifndef __XEN_DRM_FRONT_H_ > > > #define __XEN_DRM_FRONT_H_ > > > -#include > > > -#include > > > - > > > #include > > > +#include > > > +#include > > > +#include > > no need to include twice > > with that fixed: > > Acked-by: Oleksandr Andrushchenko Applied, thanks. Sam ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 2/5] xen: sched: null: don't assign down vcpus to pcpus
On 8/25/18 1:21 AM, Dario Faggioli wrote: > If a pCPU has been/is being offlined, we don't want it to be neither > assigned to any pCPU, nor in the wait list. I take it the first `pCPU` should be `vCPU`? Also, English grammar agreement is funny: `neither` needs to agree with `nor`, but the two do *not* agree with the original verb. That is, the sentence should say: "...we want it to be neither assigned to pCPU, nor in the wait list". Both here and in the comment. The other thing is, from a "developmental purity" point of view, I think this series technically has a regression in the middle: cpu offline / online stops working between patch 2 and patch 4. But I'm inclined in this case not to worry too much. :-) Everything else looks good. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...
> -Original Message- > From: Roger Pau Monne > Sent: 15 July 2019 16:46 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Jan Beulich > Subject: Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement > XEN_DOMCTL_get_device_group... > > On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote: > > ... using the new iommu_group infrastructure. > > > > Because 'sibling' devices are now members of the same iommu_group, > > implement the domctl by looking up the iommu_group of the pdev with the > > matching SBDF and then finding all the assigned pdevs that are in the > > group. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Jan Beulich > > > > v2: > > - Re-implement in the absence of a per-group devs list. > > - Make use of pci_sbdf_t. > > --- > > xen/drivers/passthrough/groups.c | 44 ++ > > xen/drivers/passthrough/pci.c| 51 > > ++-- > > xen/include/xen/iommu.h | 2 ++ > > 3 files changed, 48 insertions(+), 49 deletions(-) > > > > diff --git a/xen/drivers/passthrough/groups.c > > b/xen/drivers/passthrough/groups.c > > index 1a2f461c87..6d8064e4f4 100644 > > --- a/xen/drivers/passthrough/groups.c > > +++ b/xen/drivers/passthrough/groups.c > > @@ -12,8 +12,12 @@ > > * GNU General Public License for more details. > > */ > > > > +#include > > #include > > +#include > > #include > > +#include > > +#include > > > > struct iommu_group { > > unsigned int id; > > @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg) > > return 0; > > } > > > > +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf, > > + XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs) > > max_sdevs should be unsigned AFAICT, but it seems to be completely > unused. I think you want to do... > > > +{ > > +struct iommu_group *grp = NULL; > > +struct pci_dev *pdev; > > +unsigned int i = 0; > > + > > +pcidevs_lock(); > > + > > +for_each_pdev ( d, pdev ) > > +{ > > +if ( pdev->sbdf.sbdf == sbdf.sbdf ) > > +{ > > +grp = pdev->grp; > > +break; > > +} > > +} > > + > > +if ( !grp ) > > +goto out; > > + > > +for_each_pdev ( d, pdev ) > > +{ > > if ( i == max_sdevs ) > { > pcidevs_unlock(); > return -ENOSPC; > } Oh, I'm sure I used to have that... I don't know how it got dropped. It certainly needs to be there. Paul > > Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/5] xen: sched: null: refactor core around vcpu_deassign()
On 8/25/18 1:21 AM, Dario Faggioli wrote: > vcpu_deassign() has only one caller: _vcpu_remove(). > Let's consolidate the two functions into one. > > No functional change intended. > > Signed-off-by: Dario Faggioli Acked-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...
On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote: > ... using the new iommu_group infrastructure. > > Because 'sibling' devices are now members of the same iommu_group, > implement the domctl by looking up the iommu_group of the pdev with the > matching SBDF and then finding all the assigned pdevs that are in the > group. > > Signed-off-by: Paul Durrant > --- > Cc: Jan Beulich > > v2: > - Re-implement in the absence of a per-group devs list. > - Make use of pci_sbdf_t. > --- > xen/drivers/passthrough/groups.c | 44 ++ > xen/drivers/passthrough/pci.c| 51 > ++-- > xen/include/xen/iommu.h | 2 ++ > 3 files changed, 48 insertions(+), 49 deletions(-) > > diff --git a/xen/drivers/passthrough/groups.c > b/xen/drivers/passthrough/groups.c > index 1a2f461c87..6d8064e4f4 100644 > --- a/xen/drivers/passthrough/groups.c > +++ b/xen/drivers/passthrough/groups.c > @@ -12,8 +12,12 @@ > * GNU General Public License for more details. > */ > > +#include > #include > +#include > #include > +#include > +#include > > struct iommu_group { > unsigned int id; > @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg) > return 0; > } > > +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf, > + XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs) max_sdevs should be unsigned AFAICT, but it seems to be completely unused. I think you want to do... > +{ > +struct iommu_group *grp = NULL; > +struct pci_dev *pdev; > +unsigned int i = 0; > + > +pcidevs_lock(); > + > +for_each_pdev ( d, pdev ) > +{ > +if ( pdev->sbdf.sbdf == sbdf.sbdf ) > +{ > +grp = pdev->grp; > +break; > +} > +} > + > +if ( !grp ) > +goto out; > + > +for_each_pdev ( d, pdev ) > +{ if ( i == max_sdevs ) { pcidevs_unlock(); return -ENOSPC; } Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile > index 084de77a109e..d42737f31304 100644 > --- a/arch/x86/xen/Makefile > +++ b/arch/x86/xen/Makefile > @@ -1,5 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > -OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y > +OBJECT_FILES_NON_STANDARD_xen-asm_64.o := y > > ifdef CONFIG_FUNCTION_TRACER > # Do not profile debug and lowlevel utilities > @@ -34,7 +34,7 @@ obj-$(CONFIG_XEN_PV)+= mmu_pv.o > obj-$(CONFIG_XEN_PV) += irq.o > obj-$(CONFIG_XEN_PV) += multicalls.o > obj-$(CONFIG_XEN_PV) += xen-asm.o > -obj-$(CONFIG_XEN_PV) += xen-asm_$(BITS).o > +obj-$(CONFIG_XEN_PV) += xen-asm_64.o We should be able to merge xen-asm_64.S into xen-asm.S, shouldn't we? > @@ -1312,15 +1290,7 @@ asmlinkage __visible void __init xen_start_kernel(void) > > /* keep using Xen gdt for now; no urgent need to change it */ > > -#ifdef CONFIG_X86_32 > - pv_info.kernel_rpl = 1; > - if (xen_feature(XENFEAT_supervisor_mode_kernel)) > - pv_info.kernel_rpl = 0; > -#else > pv_info.kernel_rpl = 0; Is kernel_rpl needed anymore? -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups
> -Original Message- > From: Roger Pau Monne > Sent: 15 July 2019 16:35 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Stefano Stabellini > ; Wei Liu ; > Konrad Rzeszutek Wilk ; George Dunlap > ; Andrew > Cooper ; Ian Jackson ; Tim > (Xen.org) ; > Julien Grall ; Jan Beulich > Subject: Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups > > On Mon, Jul 15, 2019 at 01:37:09PM +0100, Paul Durrant wrote: > > Some devices may share a single PCIe initiator id, e.g. if they are actually > > legacy PCI devices behind a bridge, and hence DMA from such devices will > > be subject to the same address translation in the IOMMU. Hence these devices > > should be treated as a unit for the purposes of assignment. There are also > > other reasons why multiple devices should be treated as a unit, e.g. those > > subject to a shared RMRR or those downstream of a bridge that does not > > support ACS. > > > > This patch introduces a new struct iommu_group to act as a container for > > devices that should be treated as a unit, and builds a list of them as > > PCI devices are scanned. The iommu_ops already implement a method, > > get_device_group_id(), that is seemingly intended to return the initiator > > id for a given SBDF so use this as the mechanism for group assignment in > > the first instance. Assignment based on shared RMRR or lack of ACS will be > > dealt with in subsequent patches, as will modifications to the device > > assignment code. > > > > Signed-off-by: Paul Durrant > > LGTM, just two comments below. > > > --- > > Cc: Jan Beulich > > Cc: Andrew Cooper > > Cc: George Dunlap > > Cc: Ian Jackson > > Cc: Julien Grall > > Cc: Konrad Rzeszutek Wilk > > Cc: Stefano Stabellini > > Cc: Tim Deegan > > Cc: Wei Liu > > > > v2: > > - Move code into new drivers/passthrough/groups.c > > - Drop the group index. > > - Handle failure to get group id. > > - Drop the group devs list. > > --- > > xen/drivers/passthrough/Makefile| 1 + > > xen/drivers/passthrough/groups.c| 91 > > + > > xen/drivers/passthrough/x86/iommu.c | 8 +++- > > xen/include/xen/iommu.h | 7 +++ > > xen/include/xen/pci.h | 2 + > > 5 files changed, 108 insertions(+), 1 deletion(-) > > create mode 100644 xen/drivers/passthrough/groups.c > > > > diff --git a/xen/drivers/passthrough/Makefile > > b/xen/drivers/passthrough/Makefile > > index d50ab188c8..8a77110179 100644 > > --- a/xen/drivers/passthrough/Makefile > > +++ b/xen/drivers/passthrough/Makefile > > @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86 > > subdir-$(CONFIG_ARM) += arm > > > > obj-y += iommu.o > > +obj-$(CONFIG_HAS_PCI) += groups.o > > obj-$(CONFIG_HAS_PCI) += pci.o > > obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o > > > > diff --git a/xen/drivers/passthrough/groups.c > > b/xen/drivers/passthrough/groups.c > > new file mode 100644 > > index 00..1a2f461c87 > > --- /dev/null > > +++ b/xen/drivers/passthrough/groups.c > > @@ -0,0 +1,91 @@ > > +/* > > + * Copyright (c) 2019 Citrix Systems Inc. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include > > +#include > > + > > +struct iommu_group { > > +unsigned int id; > > +}; > > + > > +static struct radix_tree_root iommu_groups; > > + > > +void __init iommu_groups_init(void) > > +{ > > +radix_tree_init(_groups); > > +} > > + > > +static struct iommu_group *alloc_iommu_group(unsigned int id) > > +{ > > +struct iommu_group *grp = xzalloc(struct iommu_group); > > + > > +if ( !grp ) > > +return NULL; > > + > > +grp->id = id; > > + > > +if ( radix_tree_insert(_groups, id, grp) ) > > +{ > > +xfree(grp); > > +grp = NULL; > > +} > > + > > +return grp; > > +} > > + > > +static struct iommu_group *get_iommu_group(unsigned int id) > > +{ > > +struct iommu_group *grp = radix_tree_lookup(_groups, id); > > + > > +if ( !grp ) > > +grp = alloc_iommu_group(id); > > + > > +return grp; > > +} > > + > > +int iommu_group_assign(struct pci_dev *pdev, void *arg) > > I'm not sure I see the point of the arg parameter, AFAICT it's > completely unused. It needs to be there because it needs to conform to the all device iterator function callback prototype. It is indeed unused though. > > > +{ > > +const struct iommu_ops *ops = iommu_get_ops(); > > +unsigned int id; > > +struct iommu_group *grp; > > + > > +if (
Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups
On Mon, Jul 15, 2019 at 01:37:09PM +0100, Paul Durrant wrote: > Some devices may share a single PCIe initiator id, e.g. if they are actually > legacy PCI devices behind a bridge, and hence DMA from such devices will > be subject to the same address translation in the IOMMU. Hence these devices > should be treated as a unit for the purposes of assignment. There are also > other reasons why multiple devices should be treated as a unit, e.g. those > subject to a shared RMRR or those downstream of a bridge that does not > support ACS. > > This patch introduces a new struct iommu_group to act as a container for > devices that should be treated as a unit, and builds a list of them as > PCI devices are scanned. The iommu_ops already implement a method, > get_device_group_id(), that is seemingly intended to return the initiator > id for a given SBDF so use this as the mechanism for group assignment in > the first instance. Assignment based on shared RMRR or lack of ACS will be > dealt with in subsequent patches, as will modifications to the device > assignment code. > > Signed-off-by: Paul Durrant LGTM, just two comments below. > --- > Cc: Jan Beulich > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Ian Jackson > Cc: Julien Grall > Cc: Konrad Rzeszutek Wilk > Cc: Stefano Stabellini > Cc: Tim Deegan > Cc: Wei Liu > > v2: > - Move code into new drivers/passthrough/groups.c > - Drop the group index. > - Handle failure to get group id. > - Drop the group devs list. > --- > xen/drivers/passthrough/Makefile| 1 + > xen/drivers/passthrough/groups.c| 91 > + > xen/drivers/passthrough/x86/iommu.c | 8 +++- > xen/include/xen/iommu.h | 7 +++ > xen/include/xen/pci.h | 2 + > 5 files changed, 108 insertions(+), 1 deletion(-) > create mode 100644 xen/drivers/passthrough/groups.c > > diff --git a/xen/drivers/passthrough/Makefile > b/xen/drivers/passthrough/Makefile > index d50ab188c8..8a77110179 100644 > --- a/xen/drivers/passthrough/Makefile > +++ b/xen/drivers/passthrough/Makefile > @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86 > subdir-$(CONFIG_ARM) += arm > > obj-y += iommu.o > +obj-$(CONFIG_HAS_PCI) += groups.o > obj-$(CONFIG_HAS_PCI) += pci.o > obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o > > diff --git a/xen/drivers/passthrough/groups.c > b/xen/drivers/passthrough/groups.c > new file mode 100644 > index 00..1a2f461c87 > --- /dev/null > +++ b/xen/drivers/passthrough/groups.c > @@ -0,0 +1,91 @@ > +/* > + * Copyright (c) 2019 Citrix Systems Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > + > +struct iommu_group { > +unsigned int id; > +}; > + > +static struct radix_tree_root iommu_groups; > + > +void __init iommu_groups_init(void) > +{ > +radix_tree_init(_groups); > +} > + > +static struct iommu_group *alloc_iommu_group(unsigned int id) > +{ > +struct iommu_group *grp = xzalloc(struct iommu_group); > + > +if ( !grp ) > +return NULL; > + > +grp->id = id; > + > +if ( radix_tree_insert(_groups, id, grp) ) > +{ > +xfree(grp); > +grp = NULL; > +} > + > +return grp; > +} > + > +static struct iommu_group *get_iommu_group(unsigned int id) > +{ > +struct iommu_group *grp = radix_tree_lookup(_groups, id); > + > +if ( !grp ) > +grp = alloc_iommu_group(id); > + > +return grp; > +} > + > +int iommu_group_assign(struct pci_dev *pdev, void *arg) I'm not sure I see the point of the arg parameter, AFAICT it's completely unused. > +{ > +const struct iommu_ops *ops = iommu_get_ops(); > +unsigned int id; > +struct iommu_group *grp; > + > +if ( !ops->get_device_group_id ) > +return 0; > + > +id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn); I think I would prefer id to be of signed type here, then when you pass it to get_iommu_group it's already made unsigned. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/4] pci: add all-device iterator function...
On Mon, Jul 15, 2019 at 01:37:08PM +0100, Paul Durrant wrote: > ...and use it for setup_hwdom_pci_devices() and dump_pci_devices(). > > The unlock/process-pending-softirqs/lock sequence that was in > _setup_hwdom_pci_devices() is now done in the generic iterator function, > which does mean it is also done (unnecessarily) in the case of > dump_pci_devices(), since run_all_nonirq_keyhandlers() will call > process_pending_softirqs() before invoking each key handler anyway, but > this is not performance critical code. > > The " segment " headline that was in _dump_pci_devices() has > been dropped because it is non-trivial to deal with it when using a > generic all-device iterator and, since the segment number is included > in every log line anyway, it didn't add much value anyway. > > Signed-off-by: Paul Durrant Reviewed-by: Roger Pau Monné Just some trivial comments. Thanks. > --- > Cc: Jan Beulich > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Ian Jackson > Cc: Julien Grall > Cc: Konrad Rzeszutek Wilk > Cc: Stefano Stabellini > Cc: Tim Deegan > Cc: Wei Liu > > v2: > - New in v2. > --- > xen/drivers/passthrough/pci.c | 120 > +++--- > xen/include/xen/pci.h | 1 + > 2 files changed, 68 insertions(+), 53 deletions(-) > > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c > index e88689425d..179cb7e17e 100644 > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -1134,54 +1134,78 @@ static void __hwdom_init setup_one_hwdom_device(const > struct setup_hwdom *ctxt, > ctxt->d->domain_id, err); > } > > -static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void > *arg) > +static int __hwdom_init setup_hwdom_pci_device(struct pci_dev *pdev, void > *arg) > { > struct setup_hwdom *ctxt = arg; > -int bus, devfn; > +struct domain *d = ctxt->d; > > -for ( bus = 0; bus < 256; bus++ ) > +if ( !pdev->domain ) > { > -for ( devfn = 0; devfn < 256; devfn++ ) > +pdev->domain = d; > +list_add(>domain_list, >pdev_list); > +setup_one_hwdom_device(ctxt, pdev); > +} > +else if ( pdev->domain == dom_xen ) > +{ > +pdev->domain = d; > +setup_one_hwdom_device(ctxt, pdev); > +pdev->domain = dom_xen; > +} > +else if ( pdev->domain != d ) > +printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n", > + pdev->domain->domain_id, pdev->seg, pdev->bus, You can use %pd here to print the domain. > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); > + > +return 0; > +} > + > +struct psdi_ctxt { > +int (*cb)(struct pci_dev *, void *); > +void *arg; > +}; > + > +static int pci_segment_devices_iterate(struct pci_seg *pseg, void *arg) > +{ > +struct psdi_ctxt *ctxt = arg; > +int bus, devfn; unsigned for both the above. > +int rc = 0; > + > +/* > + * We don't iterate by walking pseg->alldevs_list here because that > + * would make the pcidevs_unlock()/lock() sequence below unsafe. > + */ > +for ( bus = 0; !rc && bus < 256; bus++ ) > +for ( devfn = 0; !rc && devfn < 256; devfn++ ) > { > struct pci_dev *pdev = pci_get_pdev(pseg->nr, bus, devfn); > > if ( !pdev ) > continue; > > -if ( !pdev->domain ) > -{ > -pdev->domain = ctxt->d; > -list_add(>domain_list, >d->pdev_list); > -setup_one_hwdom_device(ctxt, pdev); > -} > -else if ( pdev->domain == dom_xen ) > -{ > -pdev->domain = ctxt->d; > -setup_one_hwdom_device(ctxt, pdev); > -pdev->domain = dom_xen; > -} > -else if ( pdev->domain != ctxt->d ) > -printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n", > - pdev->domain->domain_id, pseg->nr, bus, > - PCI_SLOT(devfn), PCI_FUNC(devfn)); > +rc = ctxt->cb(pdev, ctxt->arg); > > -if ( iommu_verbose ) > -{ > -pcidevs_unlock(); > -process_pending_softirqs(); > -pcidevs_lock(); > -} > -} > - > -if ( !iommu_verbose ) > -{ > +/* > + * Err on the safe side and assume the callback has taken > + * a significant amount of time. > + */ > pcidevs_unlock(); > process_pending_softirqs(); > pcidevs_lock(); > } > -} > > -return 0; > +return rc; > +} > + > +int pci_pdevs_iterate(int (*cb)(struct pci_dev *, void *), void *arg) > +{ > +struct psdi_ctxt ctxt = { .cb = cb, .arg = arg }; > +int rc; > + > +pcidevs_lock(); > +rc = pci_segments_iterate(pci_segment_devices_iterate, ); > +
Re: [Xen-devel] [PATCH v7 5/5] x86/xen: Add "nopv" support for HVM guest
On 7/11/19 8:02 AM, Zhenzhong Duan wrote: > PVH guest needs PV extentions to work, so "nopv" parameter should be > ignored for PVH but not for HVM guest. > > If PVH guest boots up via the Xen-PVH boot entry, xen_pvh is set early, > we know it's PVH guest and ignore "nopv" parameter directly. > > If PVH guest boots up via the normal boot entry same as HVM guest, it's > hard to distinguish PVH and HVM guest at that time. In this case, we > have to panic early if PVH is detected and nopv is enabled to avoid a > worse situation later. > > Move xen_platform_hvm() after xen_hvm_guest_late_init() to avoid compile > error. > > Signed-off-by: Zhenzhong Duan > Cc: Boris Ostrovsky > Cc: Juergen Gross > Cc: Stefano Stabellini > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] tools/xenstore: start with empty data base
On Tue, Jan 10, 2017 at 4:14 PM Juergen Gross wrote: > > Today xenstored tries to open a tdb data base file on disk when it is > started. As this is problematic in most cases the scripts used to start > xenstored ensure xenstored won't find such a file in order to start > with an empty xenstore. Sorry to respond to such an old thread, just trying to record this in a useful place: in the distros design session at the recent XenSummit, it turned out that: 1. Most distros had to mount a tmpfs for the xenstore database to prevent xenstore from destroying disk performance 2. xenstored already has an `-I` option which creates a memory-only database At which point it was asked: Why is this option not the default? This patch seems to imply that the main reason at the moment for an external db is debugging; in which case, it does seem like the default should be in-memory, with an external db as a debugging option. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 1/2] x86/traps: guard top-of-stack reads
Nothing guarantees that the original frame's stack pointer points at readable memory. Avoid a (likely nested) crash by attaching exception recovery to the read (making it a single read at the same time). Don't even invoke _show_trace() in case of a non-readable top slot. Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper --- v2: Name asm() arguments. Use explicit "fault" variable. --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -486,17 +486,31 @@ static void _show_trace(unsigned long sp static void show_trace(const struct cpu_user_regs *regs) { -unsigned long *sp = ESP_BEFORE_EXCEPTION(regs); +unsigned long *sp = ESP_BEFORE_EXCEPTION(regs), tos = 0; +bool fault = false; printk("Xen call trace:\n"); +/* Guarded read of the stack top. */ +asm ( "1: mov %[data], %[tos]; 2:\n" + ".pushsection .fixup,\"ax\"\n" + "3: movb $1, %[fault]; jmp 2b\n" + ".popsection\n" + _ASM_EXTABLE(1b, 3b) + : [tos] "+r" (tos), [fault] "+qm" (fault) : [data] "m" (*sp) ); + /* * If RIP looks sensible, or the top of the stack doesn't, print RIP at * the top of the stack trace. */ if ( is_active_kernel_text(regs->rip) || - !is_active_kernel_text(*sp) ) + !is_active_kernel_text(tos) ) printk(" [<%p>] %pS\n", _p(regs->rip), _p(regs->rip)); +else if ( fault ) +{ +printk(" [Fault on access]\n"); +return; +} /* * Else RIP looks bad but the top of the stack looks good. Perhaps we * followed a wild function pointer? Lets assume the top of the stack is a @@ -505,7 +519,7 @@ static void show_trace(const struct cpu_ */ else { -printk(" [<%p>] %pS\n", _p(*sp), _p(*sp)); +printk(" [<%p>] %pS\n", _p(tos), _p(tos)); sp++; } ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/2] x86/traps: widen condition for logging top-of-stack
Despite -fno-omit-frame-pointer the compiler may omit the frame pointer, often for relatively simple leaf functions. (To give a specific example, the case I've run into this with is _pci_hide_device() and gcc 8. Interestingly the even more simple neighboring iommu_has_feature() does get a frame pointer set up, around just a single instruction. But this may be a result of the size-of-asm() effects discussed elsewhere.) Log the top-of-stack value if it looks valid _or_ if RIP looks invalid. Also annotate all stack trace entries with a marker, to indicate their origin: R: register state F: frame pointer based S: raw stack contents Signed-off-by: Jan Beulich --- v3: Tag stack entries consistently, but differently than in v2. v2: Re-base over changes to earlier patch. --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -433,7 +433,7 @@ static void _show_trace(unsigned long sp { addr = *stack++; if ( is_active_kernel_text(addr) ) -printk(" [<%p>] %pS\n", _p(addr), _p(addr)); +printk(" [<%p>] S %pS\n", _p(addr), _p(addr)); } } @@ -476,7 +476,7 @@ static void _show_trace(unsigned long sp addr = frame[1]; } -printk(" [<%p>] %pS\n", _p(addr), _p(addr)); +printk(" [<%p>] F %pS\n", _p(addr), _p(addr)); low = (unsigned long)[2]; } @@ -505,21 +505,26 @@ static void show_trace(const struct cpu_ */ if ( is_active_kernel_text(regs->rip) || !is_active_kernel_text(tos) ) -printk(" [<%p>] %pS\n", _p(regs->rip), _p(regs->rip)); -else if ( fault ) +printk(" [<%p>] R %pS\n", _p(regs->rip), _p(regs->rip)); + +if ( fault ) { printk(" [Fault on access]\n"); return; } + /* - * Else RIP looks bad but the top of the stack looks good. Perhaps we - * followed a wild function pointer? Lets assume the top of the stack is a + * If RIP looks bad or the top of the stack looks good, log the top of + * stack as well. Perhaps we followed a wild function pointer, or we're + * in a function without frame pointer, or in a function prologue before + * the frame pointer gets set up? Let's assume the top of the stack is a * return address; print it and skip past so _show_trace() doesn't print * it again. */ -else +if ( !is_active_kernel_text(regs->rip) || + is_active_kernel_text(tos) ) { -printk(" [<%p>] %pS\n", _p(tos), _p(tos)); +printk(" [<%p>] S %pS\n", _p(tos), _p(tos)); sp++; } ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On 15/07/2019 15:19, Jan Beulich wrote: > On 15.07.2019 15:49, Andrew Cooper wrote: >> On 15/07/2019 11:35, Jan Beulich wrote: >>> With non-empty CONFIG_DOM0_MEM clang5 produces >>> >>> dom0_build.c:344:24: error: use of logical '&&' with constant operand >>> [-Werror,-Wconstant-logical-operand] >>> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >>> ^ ~~ >>> dom0_build.c:344:24: note: use '&' for a bitwise operation >>> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >>> ^~ >>> & >>> dom0_build.c:344:24: note: remove constant to silence this warning >>> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >>> ~^ >>> 1 error generated. >>> >>> Obviously neither of the two suggestions are an option here. Oddly >>> enough swapping the operands of the && helps, while e.g. casting or >>> parenthesizing doesn't. Another workable variant looks to be the use of >>> !! on the constant. >>> >>> Signed-off-by: Jan Beulich >>> --- >>> I'm open to going the !! or yet some different route. No matter which >>> one we choose, I'm afraid it is going to remain guesswork what newer >>> (and future) versions of clang will choke on. >>> >>> --- a/xen/arch/x86/dom0_build.c >>> +++ b/xen/arch/x86/dom0_build.c >>> @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag >>>unsigned long avail = 0, nr_pages, min_pages, max_pages; >>>bool need_paging; >>> >>> -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >>> +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set ) >>>parse_dom0_mem(CONFIG_DOM0_MEM); >> First and foremost, there is an identical construct on the ARM side, >> which wants an equivalent adjustment. > Oh, indeed. I should have remembered ... > >> As to the change, I'm going to suggest what I suggested instead of this >> the first time around, which is strlen(CONFIG_DOM0_MEM) to make the >> logic easier to follow. > How does use of strlen() make anything easier? I think it is a pretty > common pattern to check the first character for nul if all one is > after is to know whether a string is empty. Only for things which are obviously a string. CONFIG_DOM0_MEM isn't. In this case, you have a name which would most obviously be an integer, which is compiling in a context where an array reference is valid, which is confusing to read. As strlen() is implemented with a builtin, it should be evaluated by the compiler, given that CONFIG_DOM0_MEM is a constant, but hopefully won't trigger this warning. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 0/2] x86/traps: improve show_trace()'s top-of-stack handling
1: guard top-of-stack reads 2: widen condition for logging top-of-stack The issue patch 2 fixes (a curious lack of an intermediate call stack entry) was observed in practice; patch 1 is a result of me just looking at the code. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Xen on Distros
On 15.07.2019 16:42, George Dunlap wrote: > On 7/15/19 3:23 PM, Jan Beulich wrote: >> On 15.07.2019 16:11, George Dunlap wrote: >>> There was a long discussion about security patches, with the general >>> proposal being that we should cut a point release for every security issue. >> >> Interesting. Looks like in politics that until a decision fits people >> they keep re-raising the point. Iirc on a prior meeting (Budapest?) >> we had settled on continuing with the current scheme. Were there any >> new arguments towards this alternative model? > > Well I don't know if there were any new arguments because I don't > immediately remember the old discussion. Do we have a summary of the > discussion in Budapest, with its conclusions, anywhere? I don't recall if suitable notes were taken back then; as indicated I'm not even sure which meeting it was at. > The basic idea was that: > > 1. Most distros / packagers are going to want to do an immediate release > anyway. > > 2. Distros generally seemed to be rebasing on top of staging as soon as > the XSA went out anyway (and ISTR this being the recommeneded course of > action) > > So for all intents and purposes, we have something which is, in fact, a > release; all it's missing is a signed tag and a tarball. > > Obviously there are testing implications that would need to be sorted > out before this could become a reality. > > In any case, the ball is in the court of "VOLUNTEER" to write up a > concrete proposal which could be discussed. You'll be able to raise all > your concerns at that point if you want (although having a sketch would > of course be helpful for whoever is writing such a proposal). Sure - I realized soon after having sent the initial reply that perhaps this was the wrong context in the first place to raise my question. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On 15.07.2019 16:44, Tamas K Lengyel wrote: > On Mon, Jul 15, 2019 at 8:13 AM Jan Beulich wrote: >> >> On 15.07.2019 15:41, Tamas K Lengyel wrote: >>> On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich wrote: With non-empty CONFIG_DOM0_MEM clang5 produces dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^ ~~ dom0_build.c:344:24: note: use '&' for a bitwise operation if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^~ & dom0_build.c:344:24: note: remove constant to silence this warning if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ~^ 1 error generated. Obviously neither of the two suggestions are an option here. Oddly enough swapping the operands of the && helps, while e.g. casting or parenthesizing doesn't. Another workable variant looks to be the use of !! on the constant. Signed-off-by: Jan Beulich --- I'm open to going the !! or yet some different route. No matter which one we choose, I'm afraid it is going to remain guesswork what newer (and future) versions of clang will choke on. >>> >>> Is disabling the check itself not an option? Seems to me to be a more >>> sensible option then hacking around it. >> >> How would you envision to disable the check? It's there for a reason >> after all. > > By passing -Wno-constant-logical-operand to the compiler. It looks > like a check for a non-common situation which evidently Xen uses, so > what's the point of keeping it but hacking around with it with tricks > that are fragile? Oh, so you meant disabling the compiler warning. That may be an option, but I wouldn't routinely suggest such as often besides unhelpful instances there are also helpful ones. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Xen on Distros
On 7/15/19 3:42 PM, George Dunlap wrote: > On 7/15/19 3:23 PM, Jan Beulich wrote: >> On 15.07.2019 16:11, George Dunlap wrote: >>> There was a long discussion about security patches, with the general >>> proposal being that we should cut a point release for every security issue. >> >> Interesting. Looks like in politics that until a decision fits people >> they keep re-raising the point. Iirc on a prior meeting (Budapest?) >> we had settled on continuing with the current scheme. Were there any >> new arguments towards this alternative model? If we end up deciding against doing this again, it might be worth creating a ticket somewhere to try to capture the arguments on each side, so that we don't end up re-hashing the same issues in another few years. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On Mon, Jul 15, 2019 at 8:13 AM Jan Beulich wrote: > > On 15.07.2019 15:41, Tamas K Lengyel wrote: > > On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich wrote: > >> > >> With non-empty CONFIG_DOM0_MEM clang5 produces > >> > >> dom0_build.c:344:24: error: use of logical '&&' with constant operand > >> [-Werror,-Wconstant-logical-operand] > >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > >> ^ ~~ > >> dom0_build.c:344:24: note: use '&' for a bitwise operation > >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > >> ^~ > >> & > >> dom0_build.c:344:24: note: remove constant to silence this warning > >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > >> ~^ > >> 1 error generated. > >> > >> Obviously neither of the two suggestions are an option here. Oddly > >> enough swapping the operands of the && helps, while e.g. casting or > >> parenthesizing doesn't. Another workable variant looks to be the use of > >> !! on the constant. > >> > >> Signed-off-by: Jan Beulich > >> --- > >> I'm open to going the !! or yet some different route. No matter which > >> one we choose, I'm afraid it is going to remain guesswork what newer > >> (and future) versions of clang will choke on. > > > > Is disabling the check itself not an option? Seems to me to be a more > > sensible option then hacking around it. > > How would you envision to disable the check? It's there for a reason > after all. By passing -Wno-constant-logical-operand to the compiler. It looks like a check for a non-common situation which evidently Xen uses, so what's the point of keeping it but hacking around with it with tricks that are fragile? Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Xen on Distros
On 7/15/19 3:23 PM, Jan Beulich wrote: > On 15.07.2019 16:11, George Dunlap wrote: >> There was a long discussion about security patches, with the general >> proposal being that we should cut a point release for every security issue. > > Interesting. Looks like in politics that until a decision fits people > they keep re-raising the point. Iirc on a prior meeting (Budapest?) > we had settled on continuing with the current scheme. Were there any > new arguments towards this alternative model? Well I don't know if there were any new arguments because I don't immediately remember the old discussion. Do we have a summary of the discussion in Budapest, with its conclusions, anywhere? The basic idea was that: 1. Most distros / packagers are going to want to do an immediate release anyway. 2. Distros generally seemed to be rebasing on top of staging as soon as the XSA went out anyway (and ISTR this being the recommeneded course of action) So for all intents and purposes, we have something which is, in fact, a release; all it's missing is a signed tag and a tarball. Obviously there are testing implications that would need to be sorted out before this could become a reality. In any case, the ball is in the court of "VOLUNTEER" to write up a concrete proposal which could be discussed. You'll be able to raise all your concerns at that point if you want (although having a sketch would of course be helpful for whoever is writing such a proposal). -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.10-testing test] 138996: regressions - trouble: blocked/fail/pass/starved
flight 138996 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/138996/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-prev 6 xen-buildfail REGR. vs. 138376 build-i386-prev 6 xen-buildfail REGR. vs. 138376 Tests which did not succeed, but are not blocking: test-amd64-amd64-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 138376 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail like 138376 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-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-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail 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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail 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-arm64-arm64-xl-thunderx 2 hosts-allocate
[Xen-devel] [PATCH v2] x86: use POPCNT for hweight() when available
This is faster than using the software implementation, and the insn is available on all half-way recent hardware. Therefore convert generic_hweight() to out-of-line functions (without affecting Arm) and use alternatives patching to replace the function calls. Note that the approach doesn#t work for clang, due to it not recognizing -ffixed-*. Suggested-by: Andrew Cooper Signed-off-by: Jan Beulich --- v2: Also suppress UB sanitizer instrumentation. Reduce macroization in hweight.c. Exclude clang builds. --- Note: Using "g" instead of "X" as the dummy constraint in hweight64() and hweight32(), other than expected, produces slightly better code with gcc 8. --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -31,6 +31,10 @@ obj-y += emul-i8254.o obj-y += extable.o obj-y += flushtlb.o obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o +# clang doesn't appear to know of -ffixed-* +hweight-$(gcc) := hweight.o +hweight-$(clang) := +obj-y += $(hweight-y) obj-y += hypercall.o obj-y += i387.o obj-y += i8259.o @@ -251,6 +255,10 @@ boot/mkelf32: boot/mkelf32.c efi/mkreloc: efi/mkreloc.c $(HOSTCC) $(HOSTCFLAGS) -g -o $@ $< +nocov-y += hweight.o +noubsan-y += hweight.o +hweight.o: CFLAGS += $(foreach reg,cx dx si 8 9 10 11,-ffixed-r$(reg)) + .PHONY: clean clean:: rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32 --- /dev/null +++ b/xen/arch/x86/hweight.c @@ -0,0 +1,21 @@ +#define generic_hweight64 _hweight64 +#define generic_hweight32 _hweight32 +#define generic_hweight16 _hweight16 +#define generic_hweight8 _hweight8 + +#include + +#undef inline +#define inline always_inline + +#include + +#undef generic_hweight8 +#undef generic_hweight16 +#undef generic_hweight32 +#undef generic_hweight64 + +unsigned int generic_hweight8 (unsigned int x) { return _hweight8 (x); } +unsigned int generic_hweight16(unsigned int x) { return _hweight16(x); } +unsigned int generic_hweight32(unsigned int x) { return _hweight32(x); } +unsigned int generic_hweight64(uint64_t x) { return _hweight64(x); } --- a/xen/include/asm-x86/bitops.h +++ b/xen/include/asm-x86/bitops.h @@ -475,9 +475,36 @@ static inline int fls(unsigned int x) * * The Hamming Weight of a number is the total number of bits set in it. */ +#ifndef __clang__ +/* POPCNT encodings with %{r,e}di input and %{r,e}ax output: */ +#define POPCNT_64 ".byte 0xF3, 0x48, 0x0F, 0xB8, 0xC7" +#define POPCNT_32 ".byte 0xF3, 0x0F, 0xB8, 0xC7" + +#define hweight_(n, x, insn, setup, cout, cin) ({ \ +unsigned int res_;\ +/*\ + * For the function call the POPCNT input register needs to be marked \ + * modified as well. Set up a local variable of appropriate type \ + * for this purpose. \ + */ \ +typeof((uint##n##_t)(x) + 0U) val_ = (x); \ +alternative_io(setup "; call generic_hweight" #n, \ + insn, X86_FEATURE_POPCNT, \ + ASM_OUTPUT2([res] "=a" (res_), [val] cout (val_)), \ + [src] cin (val_)); \ +res_; \ +}) +#define hweight64(x) hweight_(64, x, POPCNT_64, "", "+D", "g") +#define hweight32(x) hweight_(32, x, POPCNT_32, "", "+D", "g") +#define hweight16(x) hweight_(16, x, "movzwl %w[src], %[val]; " POPCNT_32, \ + "mov %[src], %[val]", "=", "rm") +#define hweight8(x) hweight_( 8, x, "movzbl %b[src], %[val]; " POPCNT_32, \ + "mov %[src], %[val]", "=", "rm") +#else #define hweight64(x) generic_hweight64(x) #define hweight32(x) generic_hweight32(x) #define hweight16(x) generic_hweight16(x) #define hweight8(x) generic_hweight8(x) +#endif #endif /* _X86_BITOPS_H */ x86: use POPCNT for hweight() when available This is faster than using the software implementation, and the insn is available on all half-way recent hardware. Therefore convert generic_hweight() to out-of-line functions (without affecting Arm) and use alternatives patching to replace the function calls. Note that the approach doesn#t work for clang, due to it not recognizing -ffixed-*. Suggested-by: Andrew Cooper Signed-off-by: Jan Beulich --- v2: Also suppress UB sanitizer instrumentation. Reduce macroization in hweight.c. Exclude clang builds. --- Note: Using "g" instead of "X" as the dummy constraint in hweight64() and hweight32(), other than expected, produces slightly better code with gcc 8. --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -31,6 +31,10 @@ obj-y += emul-i8254.o obj-y += extable.o obj-y += flushtlb.o
Re: [Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code
On Mon, Jul 15, 2019 at 01:37:07PM +0100, Paul Durrant wrote: > It's not vendor specific so it doesn't really belong there. > > Scanning the PCI topology also really doesn't have much to do with IOMMU > initialization. It doesn't depend on there even being an IOMMU. This patch > moves to the call to the beginning of iommu_hardware_setup() but only > places it there because the topology information would be otherwise unused. > > Subsequent patches will actually make use of the PCI topology during > (x86) IOMMU initialization. > > Signed-off-by: Paul Durrant Reviewed-by: Roger Pau Monné I would even consider moving the call to scan_pci_devices into pci_segments_init instead of doing it in the IOMMU code, as you suggest above. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH
On Mon, Jul 15, 2019 at 12:50:29PM +0100, Andrew Cooper wrote: > On 15/07/2019 12:46, Roger Pau Monné wrote: > >> +; > >> +; Jump to the main routine of the pre-SEC code > >> +; skiping the 16-bit part of the routine and > >> +; into the 32-bit flat mode part > >> +; > >> +OneTimeCallRet TransitionFromReal16To32BitFlat > > Since PVH already starts in flat 32bit mode, I'm not sure I see the > > point of this routine, since it seems to be used exclusively to switch > > from 16 to 32b flat mode. The comment mentions skipping that part, but > > I'm not sure I see how that's achieved. > > Its some OVMF local magic. This means "jmp > end_of_TransitionFromReal16To32BitFlat", which is the correct place to > go, but the code really is misleading to read. Oh right, it's OneTimeCallRet. I guess this is obvious if you are familiar with OVMF code, which I'm not. Expanding the comment to mention that jumping to the end of the routine is achieved by using OneTimeCallRet would have helped me, but this might be too verbose. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Design session report: Xen on Distros
On 15.07.2019 16:11, George Dunlap wrote: > There was a long discussion about security patches, with the general > proposal being that we should cut a point release for every security issue. Interesting. Looks like in politics that until a decision fits people they keep re-raising the point. Iirc on a prior meeting (Budapest?) we had settled on continuing with the current scheme. Were there any new arguments towards this alternative model? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 09/35] OvmfPkg/OvmfXen: use a TimerLib instance that depends only on the CPU
On Thu, Jul 04, 2019 at 03:42:07PM +0100, Anthony PERARD wrote: > ACPI Timer does not work in a PVH guest, but local APIC works on both This is not accurate. It's not that the ACPI timer doesn't work, it's just that it's not present. The PM_TMR_BLK should be set to 0 to indicate the lack of PM timer support, or else there's something broken. > PVH and HVM. > > Note that the use of SecPeiDxeTimerLibCpu might be an issue with a > driver of type DXE_RUNTIME_DRIVER. I've attemptde to find out which of ^ attempted > the DXE_RUNTIME_DRIVER uses the TimerLib at runtime. I've done that by > replacing the TimerLib evaluation in > [LibraryClasses.common.DXE_RUNTIME_DRIVER] by a different one and > check every module that uses it (with the --report-file=report build ^ checking > option). > > ResetSystemRuntimeDxe is calling the TimerLib API at runtime to do the > operation "EfiResetCold", so this may never complete if the OS have > disabled the Local APIC Timer. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On 15.07.2019 15:49, Andrew Cooper wrote: > On 15/07/2019 11:35, Jan Beulich wrote: >> With non-empty CONFIG_DOM0_MEM clang5 produces >> >> dom0_build.c:344:24: error: use of logical '&&' with constant operand >> [-Werror,-Wconstant-logical-operand] >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ^ ~~ >> dom0_build.c:344:24: note: use '&' for a bitwise operation >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ^~ >> & >> dom0_build.c:344:24: note: remove constant to silence this warning >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ~^ >> 1 error generated. >> >> Obviously neither of the two suggestions are an option here. Oddly >> enough swapping the operands of the && helps, while e.g. casting or >> parenthesizing doesn't. Another workable variant looks to be the use of >> !! on the constant. >> >> Signed-off-by: Jan Beulich >> --- >> I'm open to going the !! or yet some different route. No matter which >> one we choose, I'm afraid it is going to remain guesswork what newer >> (and future) versions of clang will choke on. >> >> --- a/xen/arch/x86/dom0_build.c >> +++ b/xen/arch/x86/dom0_build.c >> @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag >>unsigned long avail = 0, nr_pages, min_pages, max_pages; >>bool need_paging; >> >> -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set ) >>parse_dom0_mem(CONFIG_DOM0_MEM); > > First and foremost, there is an identical construct on the ARM side, > which wants an equivalent adjustment. Oh, indeed. I should have remembered ... > As to the change, I'm going to suggest what I suggested instead of this > the first time around, which is strlen(CONFIG_DOM0_MEM) to make the > logic easier to follow. How does use of strlen() make anything easier? I think it is a pretty common pattern to check the first character for nul if all one is after is to know whether a string is empty. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection
On Thu, Jul 04, 2019 at 03:42:22PM +0100, Anthony PERARD wrote: > When running as a Xen PVH guest, there is no CMOS to read the memory > size from. Rework GetSystemMemorySize(Below|Above)4gb() so they can > works without CMOS by reading the e820 table. > > Rework XenPublishRamRegions for PVH, handle the Reserve type and explain > about the ACPI type. MTRR settings aren't modified anymore, on HVM, it's > already done by hvmloader, on PVH it is supposed to have sane default. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 > Signed-off-by: Anthony PERARD > Acked-by: Laszlo Ersek > --- > > Notes: > Comment for Xen people: > About MTRR, should we redo the setting in OVMF? Even if in both case of > PVH and HVM, something would have setup the default type to write back > and handle a few other ranges like PCI hole, hvmloader for HVM or and > libxc I think for PVH. That's a tricky question. Ideally we would like the firmware (OVMF) to take care of that, because it already has code to do so. Problem here is that PVH can also be booted without firmware, in which case it needs the hypervisor to have setup some sane initial MTRR state. The statement in the PVH document about initial MTRR state is vague enough that allows Xen to boot into the guest with a minimal MTRR state, that can for example not contain UC regions for the MMIO regions of passed through devices, hence I think OVMF should be in charge of creating a more complete MTRR state if possible. Is this something OVMF already has logic for? Also accounting for the MMIO regions of devices? > (For PVH, it's in the spec as well > https://xenbits.xenproject.org/docs/unstable/misc/pvh.html#mtrr ) > > OvmfPkg/XenPlatformPei/Platform.h | 6 +++ > OvmfPkg/XenPlatformPei/MemDetect.c | 71 ++ > OvmfPkg/XenPlatformPei/Xen.c | 47 ++-- > 3 files changed, 111 insertions(+), 13 deletions(-) > > diff --git a/OvmfPkg/XenPlatformPei/Platform.h > b/OvmfPkg/XenPlatformPei/Platform.h > index db9a62572f..e8e0b835a5 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.h > +++ b/OvmfPkg/XenPlatformPei/Platform.h > @@ -114,6 +114,12 @@ XenPublishRamRegions ( >VOID >); > > +EFI_STATUS > +XenGetE820Map ( > + EFI_E820_ENTRY64 **Entries, > + UINT32 *Count > + ); > + > extern EFI_BOOT_MODE mBootMode; > > extern UINT8 mPhysMemAddressWidth; > diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c > b/OvmfPkg/XenPlatformPei/MemDetect.c > index cb7dd93ad6..3e33e7f414 100644 > --- a/OvmfPkg/XenPlatformPei/MemDetect.c > +++ b/OvmfPkg/XenPlatformPei/MemDetect.c > @@ -96,6 +96,47 @@ Q35TsegMbytesInitialization ( >mQ35TsegMbytes = ExtendedTsegMbytes; > } > > +STATIC > +UINT64 > +GetHighestSystemMemoryAddress ( > + BOOLEAN Below4gb > + ) > +{ > + EFI_E820_ENTRY64*E820Map; > + UINT32 E820EntriesCount; > + EFI_E820_ENTRY64*Entry; > + EFI_STATUS Status; > + UINT32 Loop; > + UINT64 HighestAddress; > + UINT64 EntryEnd; > + > + HighestAddress = 0; > + > + Status = XenGetE820Map (, ); You could maybe initialize this as a global to avoid having to issue a hypercall each time you need to get something from the memory map. > + ASSERT_EFI_ERROR (Status); > + > + for (Loop = 0; Loop < E820EntriesCount; Loop++) { > +Entry = E820Map + Loop; > +EntryEnd = Entry->BaseAddr + Entry->Length; > + > +if (Entry->Type == EfiAcpiAddressRangeMemory && > +EntryEnd > HighestAddress) { > + > + if (Below4gb && (EntryEnd <= BASE_4GB)) { > +HighestAddress = EntryEnd; > + } else if (!Below4gb && (EntryEnd >= BASE_4GB)) { > +HighestAddress = EntryEnd; > + } > +} > + } > + > + // > + // Round down the end address. > + // > + HighestAddress &= ~(UINT64)EFI_PAGE_MASK; > + > + return HighestAddress; You could do the rounding on the return statement. > +} > > UINT32 > GetSystemMemorySizeBelow4gb ( > @@ -105,6 +146,19 @@ GetSystemMemorySizeBelow4gb ( >UINT8 Cmos0x34; >UINT8 Cmos0x35; > > + // > + // In PVH case, there is no CMOS, we have to calculate the memory size > + // from parsing the E820 > + // > + if (XenPvhDetected ()) { IIRC on HVM you can also get the memory map from the hypercall, in which case you could use the same code path for both HVM and PVH. > +UINT64 HighestAddress; > + > +HighestAddress = GetHighestSystemMemoryAddress (TRUE); > +ASSERT (HighestAddress > 0 && HighestAddress <= BASE_4GB); > + > +return HighestAddress; The name of the function here is GetSystemMemorySizeBelow4gb, but you are returning the highest memory address in the range, is this expected? ie: highest address != memory size On HVM there are quite some holes in the memory map, and nothing guarantees there are no memory regions after the holes or non-RAM regions. > + } > + >// >// CMOS 0x34/0x35 specifies the system memory above 16
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On 15.07.2019 15:41, Tamas K Lengyel wrote: > On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich wrote: >> >> With non-empty CONFIG_DOM0_MEM clang5 produces >> >> dom0_build.c:344:24: error: use of logical '&&' with constant operand >> [-Werror,-Wconstant-logical-operand] >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ^ ~~ >> dom0_build.c:344:24: note: use '&' for a bitwise operation >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ^~ >> & >> dom0_build.c:344:24: note: remove constant to silence this warning >> if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >> ~^ >> 1 error generated. >> >> Obviously neither of the two suggestions are an option here. Oddly >> enough swapping the operands of the && helps, while e.g. casting or >> parenthesizing doesn't. Another workable variant looks to be the use of >> !! on the constant. >> >> Signed-off-by: Jan Beulich >> --- >> I'm open to going the !! or yet some different route. No matter which >> one we choose, I'm afraid it is going to remain guesswork what newer >> (and future) versions of clang will choke on. > > Is disabling the check itself not an option? Seems to me to be a more > sensible option then hacking around it. How would you envision to disable the check? It's there for a reason after all. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Design session report: Xen on Distros
Much of this covered things discussed elsewhere: * Allowing multiple versions of the tools to be installed at the same time * Getting rid of external builds There was a long discussion about security patches, with the general proposal being that we should cut a point release for every security issue. One random thing was that xenstored apparently has an 'in-memory-only' option. Since xenstored can't actually be restarted ATM, and most distros seemed to put xenstored in a tmpfs for performance reasons, this should probably be the default. https://hackmd.io/vmacVBYbQiORJ9H4_a9Ivw # Xen on Distros design session * qemu / libxc dependency loop * build system needs "extras" turned off * xenstored / tmpfs / memory-only option? * Disabling auto-download in build system * WGET=/bin/false * Multiple version of Xen / tools? * Debian has co-install * Change some installation paths * /usr/lib/xen/4.11/... * /usr/bin/xl is a shell script * libfsimage is special * Don't need to downgrade to older tools * Gentoo has a ~~dumpster fire~~ something * A hack which stops the package manager to allow you to reboot the box halfway through * Security issues * Building from stable branch / staging branch * Doing a "point release" every XSA? * "Release from staging" is effectively a low-quality release * Idea: Always immediately release from staging? # Actions * [ ] Ian: Post a git branch of Debian co-install to xen-devel * [ ] George: Post systemd / selinux / xenstored patch * [ ] George, Ian: private osstest runs * [ ] VOLUNTEER: Propose / argue for a point release per XSA * [ ] VOLUNTEER: Improve release automation ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 00/60] xen: add core scheduling support
On 05/07/2019 14:56, Dario Faggioli wrote: > On Fri, 2019-07-05 at 14:17 +0100, Sergey Dyasli wrote: >> 1) This crash is quite likely to happen: >> >> [2019-07-04 18:22:46 UTC] (XEN) [ 3425.220660] Watchdog timer detects >> that CPU2 is stuck! >> [2019-07-04 18:22:46 UTC] (XEN) [ 3425.226293] [ Xen-4.13.0- >> 8.0.6-d x86_64 debug=y Not tainted ] >> [...] >> [2019-07-04 18:22:47 UTC] (XEN) [ 3425.501989] Xen call trace: >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.505278][] vcpu_sleep_sync+0x50/0x71 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.511518][] vcpu_pause+0x21/0x23 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.517326][] >> vcpu_set_periodic_timer+0x27/0x73 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.524258][] do_vcpu_op+0x2c9/0x668 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.530238][] compat_vcpu_op+0x250/0x390 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.536566][] pv_hypercall+0x364/0x564 >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.542719][] do_entry_int82+0x26/0x2d >> [2019-07-04 18:22:47 UTC] (XEN) [ >> 3425.548876][] entry_int82+0xbb/0xc0 >> > Mmm... vcpu_set_periodic_timer? > > What kernel is this and when does this crash happen? Hi Dario, I can easily reproduce this crash using a Debian 7 PV VM (2 vCPUs, 2GB RAM) which has the following kernel: # uname -a Linux localhost 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux All I need to do is suspend and resume the VM. Thanks, Sergey ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Design Session report: Toolstacks
Looking through the notes, it seems like the first part of this discussion, re hypervisor upgrade / downgrade & libraries, was partly covered in the distro session, in which Debian's Xen version co-install was discussed and found useful even if we had a hypervisor , Ian Jackson agreed to post Debian's co-install patches. There was a general agreement to improve xl's json handling (including accepting json to xl create), and a discussion about other improvements; various people who proposed them seem likely to come back with patches. -George https://hackmd.io/0vZaSrKBT2iKWzpVMxDVvQ # Xen toolstacks Issues: * Hypervisor upgrade / downgrade * xl not fully-featured * Remote access Potential places: * ~~On top of libxl~~ * Or libxc (symlink swizzling scheme) * Doesn't work w/ statically linked * Doesn't require HV to deal w/ backwards ABI compatibility * On the hypervisor * Need to keep working for a full release cycle * A daemon * xl create w/ json blob * xl does some processing before passing to libxl Hypershell has effective golang libxl bindings (but need to be recompiled against new versions of libxl) Other wishlist: * More events (e.g., backend / frontend disconnect) * Or, have xl / libxl clean up disconnected entries & notify when done * Feature parity on other dom0 operating systems ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On 15/07/2019 11:35, Jan Beulich wrote: > With non-empty CONFIG_DOM0_MEM clang5 produces > > dom0_build.c:344:24: error: use of logical '&&' with constant operand > [-Werror,-Wconstant-logical-operand] > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > ^ ~~ > dom0_build.c:344:24: note: use '&' for a bitwise operation > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > ^~ > & > dom0_build.c:344:24: note: remove constant to silence this warning > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >~^ > 1 error generated. > > Obviously neither of the two suggestions are an option here. Oddly > enough swapping the operands of the && helps, while e.g. casting or > parenthesizing doesn't. Another workable variant looks to be the use of > !! on the constant. > > Signed-off-by: Jan Beulich > --- > I'm open to going the !! or yet some different route. No matter which > one we choose, I'm afraid it is going to remain guesswork what newer > (and future) versions of clang will choke on. > > --- a/xen/arch/x86/dom0_build.c > +++ b/xen/arch/x86/dom0_build.c > @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag > unsigned long avail = 0, nr_pages, min_pages, max_pages; > bool need_paging; > > -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set ) > parse_dom0_mem(CONFIG_DOM0_MEM); First and foremost, there is an identical construct on the ARM side, which wants an equivalent adjustment. As to the change, I'm going to suggest what I suggested instead of this the first time around, which is strlen(CONFIG_DOM0_MEM) to make the logic easier to follow. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich wrote: > > With non-empty CONFIG_DOM0_MEM clang5 produces > > dom0_build.c:344:24: error: use of logical '&&' with constant operand > [-Werror,-Wconstant-logical-operand] > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > ^ ~~ > dom0_build.c:344:24: note: use '&' for a bitwise operation > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > ^~ > & > dom0_build.c:344:24: note: remove constant to silence this warning > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) >~^ > 1 error generated. > > Obviously neither of the two suggestions are an option here. Oddly > enough swapping the operands of the && helps, while e.g. casting or > parenthesizing doesn't. Another workable variant looks to be the use of > !! on the constant. > > Signed-off-by: Jan Beulich > --- > I'm open to going the !! or yet some different route. No matter which > one we choose, I'm afraid it is going to remain guesswork what newer > (and future) versions of clang will choke on. Is disabling the check itself not an option? Seems to me to be a more sensible option then hacking around it. Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Design session notes: Build system gripe session
Below are the raw notes. General agreements seem to be: * Removing external builds from xen build system is an advantage for the main users (developers, distros / packagers) * Out-of-tree build is useful for lots of circumstances; symlink trick used to work around its lack causes lots of problems There didn't seem to be anyone volunteering to hack at the build system, but with enough interest in seeing it change, and a general agreement on the direction we want to go, perhaps someone will step up when they get a chance. -George https://hackmd.io/mAGT5bU9T6-aXJVj88deYw # Build system gripe session ***Link available from the design-sessions session description page*** Topics include: * Moving "external" builds to a separate part of the tree, not built by default * Adding more "external" builds; `pvgrub` for instance * Making it easy to build with no internet access * Using `git submodules` Issues: 1. out of tree build not supported -- shim build still half broken 2. Xen build re-evaluates compiler support for every translation unit. (Switch to newer Kconfig.) 3. meta virtualization build system (yocto) needs to pull simlink tricks, xen's build system stomp on that 4. containerize.sh is broken by symlink tricks in the build system 5. External projects get pulled in. Some think they should be removed, some think more should be added. Solved by raisin but nobody used it For out-of-tree build, for xen, use Kbuild. Would be nice to be able to do (without having to clean or fix or patch): ``` make CONFIG=release.config O=output/release make CONFIG=shim.config O=output/shim ``` Different users have differnt requirements: developers, users, distro packagers. For everyday users we should point people to their distros. Raisin tried to be devstack, which is not developers want or trust. People in favour of removing more stuff from xen.git. Should start with a document describing how to build a xen system -- pull this tree, use this rune etc etc. Or use Android's build system? Git runes in tree should disappear. Need transitional plan for osstest. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...
... using the new iommu_group infrastructure. Because 'sibling' devices are now members of the same iommu_group, implement the domctl by looking up the iommu_group of the pdev with the matching SBDF and then finding all the assigned pdevs that are in the group. Signed-off-by: Paul Durrant --- Cc: Jan Beulich v2: - Re-implement in the absence of a per-group devs list. - Make use of pci_sbdf_t. --- xen/drivers/passthrough/groups.c | 44 ++ xen/drivers/passthrough/pci.c| 51 ++-- xen/include/xen/iommu.h | 2 ++ 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c index 1a2f461c87..6d8064e4f4 100644 --- a/xen/drivers/passthrough/groups.c +++ b/xen/drivers/passthrough/groups.c @@ -12,8 +12,12 @@ * GNU General Public License for more details. */ +#include #include +#include #include +#include +#include struct iommu_group { unsigned int id; @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg) return 0; } +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf, + XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs) +{ +struct iommu_group *grp = NULL; +struct pci_dev *pdev; +unsigned int i = 0; + +pcidevs_lock(); + +for_each_pdev ( d, pdev ) +{ +if ( pdev->sbdf.sbdf == sbdf.sbdf ) +{ +grp = pdev->grp; +break; +} +} + +if ( !grp ) +goto out; + +for_each_pdev ( d, pdev ) +{ +if ( xsm_get_device_group(XSM_HOOK, pdev->sbdf.sbdf) || + pdev->grp != grp ) +continue; + +if ( unlikely(copy_to_guest_offset(buf, i++, >sbdf.sbdf, 1)) ) +{ +pcidevs_unlock(); +return -EFAULT; +} +} + + out: +pcidevs_unlock(); + +return i; +} + /* * Local variables: * mode: C diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 179cb7e17e..3a5e90c176 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1563,53 +1563,6 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) return ret; } -static int iommu_get_device_group( -struct domain *d, u16 seg, u8 bus, u8 devfn, -XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs) -{ -const struct domain_iommu *hd = dom_iommu(d); -struct pci_dev *pdev; -int group_id, sdev_id; -u32 bdf; -int i = 0; -const struct iommu_ops *ops = hd->platform_ops; - -if ( !iommu_enabled || !ops || !ops->get_device_group_id ) -return 0; - -group_id = ops->get_device_group_id(seg, bus, devfn); - -pcidevs_lock(); -for_each_pdev( d, pdev ) -{ -if ( (pdev->seg != seg) || - ((pdev->bus == bus) && (pdev->devfn == devfn)) ) -continue; - -if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) | pdev->devfn) ) -continue; - -sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn); -if ( (sdev_id == group_id) && (i < max_sdevs) ) -{ -bdf = 0; -bdf |= (pdev->bus & 0xff) << 16; -bdf |= (pdev->devfn & 0xff) << 8; - -if ( unlikely(copy_to_guest_offset(buf, i, , 1)) ) -{ -pcidevs_unlock(); -return -1; -} -i++; -} -} - -pcidevs_unlock(); - -return i; -} - void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev) { pcidevs_lock(); @@ -1666,11 +1619,11 @@ int iommu_do_pci_domctl( max_sdevs = domctl->u.get_device_group.max_sdevs; sdevs = domctl->u.get_device_group.sdev_array; -ret = iommu_get_device_group(d, seg, bus, devfn, sdevs, max_sdevs); +ret = iommu_get_device_group(d, PCI_SBDF3(seg, bus, devfn), sdevs, + max_sdevs); if ( ret < 0 ) { dprintk(XENLOG_ERR, "iommu_get_device_group() failed!\n"); -ret = -EFAULT; domctl->u.get_device_group.num_sdevs = 0; } else diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index c93f580fdc..ac764b41f9 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -321,6 +321,8 @@ extern struct page_list_head iommu_pt_cleanup_list; void iommu_groups_init(void); int iommu_group_assign(struct pci_dev *pdev, void *arg); +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf, + XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs); #endif /* CONFIG_HAS_PCI */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/4] pci: add all-device iterator function...
...and use it for setup_hwdom_pci_devices() and dump_pci_devices(). The unlock/process-pending-softirqs/lock sequence that was in _setup_hwdom_pci_devices() is now done in the generic iterator function, which does mean it is also done (unnecessarily) in the case of dump_pci_devices(), since run_all_nonirq_keyhandlers() will call process_pending_softirqs() before invoking each key handler anyway, but this is not performance critical code. The " segment " headline that was in _dump_pci_devices() has been dropped because it is non-trivial to deal with it when using a generic all-device iterator and, since the segment number is included in every log line anyway, it didn't add much value anyway. Signed-off-by: Paul Durrant --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu v2: - New in v2. --- xen/drivers/passthrough/pci.c | 120 +++--- xen/include/xen/pci.h | 1 + 2 files changed, 68 insertions(+), 53 deletions(-) diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e88689425d..179cb7e17e 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1134,54 +1134,78 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt, ctxt->d->domain_id, err); } -static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void *arg) +static int __hwdom_init setup_hwdom_pci_device(struct pci_dev *pdev, void *arg) { struct setup_hwdom *ctxt = arg; -int bus, devfn; +struct domain *d = ctxt->d; -for ( bus = 0; bus < 256; bus++ ) +if ( !pdev->domain ) { -for ( devfn = 0; devfn < 256; devfn++ ) +pdev->domain = d; +list_add(>domain_list, >pdev_list); +setup_one_hwdom_device(ctxt, pdev); +} +else if ( pdev->domain == dom_xen ) +{ +pdev->domain = d; +setup_one_hwdom_device(ctxt, pdev); +pdev->domain = dom_xen; +} +else if ( pdev->domain != d ) +printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n", + pdev->domain->domain_id, pdev->seg, pdev->bus, + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); + +return 0; +} + +struct psdi_ctxt { +int (*cb)(struct pci_dev *, void *); +void *arg; +}; + +static int pci_segment_devices_iterate(struct pci_seg *pseg, void *arg) +{ +struct psdi_ctxt *ctxt = arg; +int bus, devfn; +int rc = 0; + +/* + * We don't iterate by walking pseg->alldevs_list here because that + * would make the pcidevs_unlock()/lock() sequence below unsafe. + */ +for ( bus = 0; !rc && bus < 256; bus++ ) +for ( devfn = 0; !rc && devfn < 256; devfn++ ) { struct pci_dev *pdev = pci_get_pdev(pseg->nr, bus, devfn); if ( !pdev ) continue; -if ( !pdev->domain ) -{ -pdev->domain = ctxt->d; -list_add(>domain_list, >d->pdev_list); -setup_one_hwdom_device(ctxt, pdev); -} -else if ( pdev->domain == dom_xen ) -{ -pdev->domain = ctxt->d; -setup_one_hwdom_device(ctxt, pdev); -pdev->domain = dom_xen; -} -else if ( pdev->domain != ctxt->d ) -printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n", - pdev->domain->domain_id, pseg->nr, bus, - PCI_SLOT(devfn), PCI_FUNC(devfn)); +rc = ctxt->cb(pdev, ctxt->arg); -if ( iommu_verbose ) -{ -pcidevs_unlock(); -process_pending_softirqs(); -pcidevs_lock(); -} -} - -if ( !iommu_verbose ) -{ +/* + * Err on the safe side and assume the callback has taken + * a significant amount of time. + */ pcidevs_unlock(); process_pending_softirqs(); pcidevs_lock(); } -} -return 0; +return rc; +} + +int pci_pdevs_iterate(int (*cb)(struct pci_dev *, void *), void *arg) +{ +struct psdi_ctxt ctxt = { .cb = cb, .arg = arg }; +int rc; + +pcidevs_lock(); +rc = pci_segments_iterate(pci_segment_devices_iterate, ); +pcidevs_unlock(); + +return rc; } void __hwdom_init setup_hwdom_pci_devices( @@ -1189,9 +1213,7 @@ void __hwdom_init setup_hwdom_pci_devices( { struct setup_hwdom ctxt = { .d = d, .handler = handler }; -pcidevs_lock(); -pci_segments_iterate(_setup_hwdom_pci_devices, ); -pcidevs_unlock(); +pci_pdevs_iterate(setup_hwdom_pci_device, ); } #ifdef CONFIG_ACPI @@ -1294,24 +1316,18 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev *pdev) } #endif -static
[Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups
Some devices may share a single PCIe initiator id, e.g. if they are actually legacy PCI devices behind a bridge, and hence DMA from such devices will be subject to the same address translation in the IOMMU. Hence these devices should be treated as a unit for the purposes of assignment. There are also other reasons why multiple devices should be treated as a unit, e.g. those subject to a shared RMRR or those downstream of a bridge that does not support ACS. This patch introduces a new struct iommu_group to act as a container for devices that should be treated as a unit, and builds a list of them as PCI devices are scanned. The iommu_ops already implement a method, get_device_group_id(), that is seemingly intended to return the initiator id for a given SBDF so use this as the mechanism for group assignment in the first instance. Assignment based on shared RMRR or lack of ACS will be dealt with in subsequent patches, as will modifications to the device assignment code. Signed-off-by: Paul Durrant --- Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu v2: - Move code into new drivers/passthrough/groups.c - Drop the group index. - Handle failure to get group id. - Drop the group devs list. --- xen/drivers/passthrough/Makefile| 1 + xen/drivers/passthrough/groups.c| 91 + xen/drivers/passthrough/x86/iommu.c | 8 +++- xen/include/xen/iommu.h | 7 +++ xen/include/xen/pci.h | 2 + 5 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 xen/drivers/passthrough/groups.c diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile index d50ab188c8..8a77110179 100644 --- a/xen/drivers/passthrough/Makefile +++ b/xen/drivers/passthrough/Makefile @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86 subdir-$(CONFIG_ARM) += arm obj-y += iommu.o +obj-$(CONFIG_HAS_PCI) += groups.o obj-$(CONFIG_HAS_PCI) += pci.o obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c new file mode 100644 index 00..1a2f461c87 --- /dev/null +++ b/xen/drivers/passthrough/groups.c @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2019 Citrix Systems Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include + +struct iommu_group { +unsigned int id; +}; + +static struct radix_tree_root iommu_groups; + +void __init iommu_groups_init(void) +{ +radix_tree_init(_groups); +} + +static struct iommu_group *alloc_iommu_group(unsigned int id) +{ +struct iommu_group *grp = xzalloc(struct iommu_group); + +if ( !grp ) +return NULL; + +grp->id = id; + +if ( radix_tree_insert(_groups, id, grp) ) +{ +xfree(grp); +grp = NULL; +} + +return grp; +} + +static struct iommu_group *get_iommu_group(unsigned int id) +{ +struct iommu_group *grp = radix_tree_lookup(_groups, id); + +if ( !grp ) +grp = alloc_iommu_group(id); + +return grp; +} + +int iommu_group_assign(struct pci_dev *pdev, void *arg) +{ +const struct iommu_ops *ops = iommu_get_ops(); +unsigned int id; +struct iommu_group *grp; + +if ( !ops->get_device_group_id ) +return 0; + +id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn); +if ( id < 0 ) +return -ENODATA; + +grp = get_iommu_group(id); +if ( !grp ) +return -ENOMEM; + +if ( iommu_verbose ) +printk(XENLOG_INFO "Assign %04x:%02x:%02x.%u -> IOMMU group %x\n", + pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn), + PCI_FUNC(pdev->devfn), grp->id); + +pdev->grp = grp; + +return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index a7438c9c25..90fc750456 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -43,7 +43,13 @@ int __init iommu_hardware_setup(void) /* x2apic setup may have previously initialised the struct. */ ASSERT(iommu_ops.init == iommu_init_ops->ops->init); -return iommu_init_ops->setup(); +rc = iommu_init_ops->setup(); +if ( rc ) +return rc; + +iommu_groups_init(); + +return pci_pdevs_iterate(iommu_group_assign, NULL); }
[Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code
It's not vendor specific so it doesn't really belong there. Scanning the PCI topology also really doesn't have much to do with IOMMU initialization. It doesn't depend on there even being an IOMMU. This patch moves to the call to the beginning of iommu_hardware_setup() but only places it there because the topology information would be otherwise unused. Subsequent patches will actually make use of the PCI topology during (x86) IOMMU initialization. Signed-off-by: Paul Durrant --- Cc: Suravee Suthikulpanit Cc: Brian Woods Cc: Kevin Tian Cc: Jan Beulich Cc: Andrew Cooper Cc: Wei Liu Cc: "Roger Pau Monné" v2: - Expanded commit comment. - Moved PCI scan to before IOMMU initialization, rather than after it. --- xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 ++- xen/drivers/passthrough/vtd/iommu.c | 4 xen/drivers/passthrough/x86/iommu.c | 6 ++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index 4afbcd1609..3338a8e0e8 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -180,7 +180,8 @@ static int __init iov_detect(void) if ( !amd_iommu_perdev_intremap ) printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not recommended (see XSA-36)!\n"); -return scan_pci_devices(); + +return 0; } int amd_iommu_alloc_root(struct domain_iommu *hd) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 8b27d7e775..b0e3bf26b5 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2372,10 +2372,6 @@ static int __init vtd_setup(void) P(iommu_hap_pt_share, "Shared EPT tables"); #undef P -ret = scan_pci_devices(); -if ( ret ) -goto error; - ret = init_vtd_hw(); if ( ret ) goto error; diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index 0fa6dcc3fd..a7438c9c25 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -28,9 +28,15 @@ struct iommu_ops __read_mostly iommu_ops; int __init iommu_hardware_setup(void) { +int rc; + if ( !iommu_init_ops ) return -ENODEV; +rc = scan_pci_devices(); +if ( rc ) +return rc; + if ( !iommu_ops.init ) iommu_ops = *iommu_init_ops->ops; else -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On 15.07.19 14:32, Peter Zijlstra wrote: On Mon, Jul 15, 2019 at 01:37:37PM +0200, Juergen Gross wrote: The long term plan has been to replace Xen PV guests by PVH. The first victim of that plan are now 32-bit PV guests, as those are used only rather seldom these days. Xen on x86 requires 64-bit support and with Grub2 now supporting PVH officially since version 2.04 there is no need to keep 32-bit PV guest support alive in the Linux kernel. Additionally Meltdown mitigation is not available in the kernel running as 32-bit PV guest, so dropping this mode makes sense from security point of view, too. Juergen Gross (2): x86/xen: remove 32-bit Xen PV guest support x86/paravirt: remove 32-bit support from PARAVIRT_XXL Hooray! Always a pleasure to cheer the community up by sending Xen patches. :-D Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/4] iommu groups + cleanup
This series is a mixture of tidying and some preparatory work for grouping PCI devices for the purposes of assignment. Paul Durrant (4): iommu / x86: move call to scan_pci_devices() out of vendor code pci: add all-device iterator function... iommu: introduce iommu_groups iommu / pci: re-implement XEN_DOMCTL_get_device_group... xen/drivers/passthrough/Makefile| 1 + xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 +- xen/drivers/passthrough/groups.c| 135 ++ xen/drivers/passthrough/pci.c | 171 +++- xen/drivers/passthrough/vtd/iommu.c | 4 - xen/drivers/passthrough/x86/iommu.c | 14 ++- xen/include/xen/iommu.h | 9 ++ xen/include/xen/pci.h | 3 + 8 files changed, 232 insertions(+), 108 deletions(-) create mode 100644 xen/drivers/passthrough/groups.c --- v2: - Drop iommu_get_ops() move and add all-device iterator Cc: Andrew Cooper Cc: Brian Woods Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Julien Grall Cc: Kevin Tian Cc: Konrad Rzeszutek Wilk Cc: "Roger Pau Monné" Cc: Stefano Stabellini Cc: Suravee Suthikulpanit Cc: Tim Deegan Cc: Wei Liu -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
On Mon, Jul 15, 2019 at 01:37:37PM +0200, Juergen Gross wrote: > The long term plan has been to replace Xen PV guests by PVH. The first > victim of that plan are now 32-bit PV guests, as those are used only > rather seldom these days. Xen on x86 requires 64-bit support and with > Grub2 now supporting PVH officially since version 2.04 there is no > need to keep 32-bit PV guest support alive in the Linux kernel. > Additionally Meltdown mitigation is not available in the kernel running > as 32-bit PV guest, so dropping this mode makes sense from security > point of view, too. > > Juergen Gross (2): > x86/xen: remove 32-bit Xen PV guest support > x86/paravirt: remove 32-bit support from PARAVIRT_XXL Hooray! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus bisection] complete test-amd64-amd64-xl-qemuu-ovmf-amd64
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-xl-qemuu-ovmf-amd64 testid xen-boot Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Bug introduced: a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5 Bug not present: 3ab4436f688c2d2f221793953cd05435ca84261c Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/139018/ (Revision log too long, omitted.) For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-ovmf-amd64.xen-boot.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-ovmf-amd64.xen-boot --summary-out=tmp/139018.bisection-summary --basis-template=133580 --blessings=real,real-bisect linux-linus test-amd64-amd64-xl-qemuu-ovmf-amd64 xen-boot Searching for failure / basis pass: 138962 fail [host=baroque0] / 138849 [host=italia0] 138813 [host=italia1] 138780 [host=godello0] 138754 [host=pinot1] 138735 [host=baroque1] 138710 [host=godello1] 138680 [host=albana1] 138661 [host=fiano0] 138639 [host=rimava1] 138612 [host=debina1] 138584 [host=albana0] 138488 [host=italia0] 138386 [host=chardonnay1] 138245 [host=chardonnay0] 138073 [host=elbling1] 137986 [host=elbling0] 137896 [host=debina0] 137739 [host=fiano0] 137686 [host=italia0] 137589 [host=albana1] 137484 [host=debina\ 1] 137388 [host=elbling1] 137283 [host=baroque1] 137191 [host=rimava1] 137125 ok. Failure / basis pass flights: 138962 / 137125 (tree with no url: minios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5 c530a75c1e6a472b0eb9558310b518f0dfcd8860 91cc60bafc7d6e49b7bc85990f895d6228f51364 d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 b541287c3600713feaaaf7608cd405e7b2e4efd0 Basis pass 3ab4436f688c2d2f221793953cd05435ca84261c c530a75c1e6a472b0eb9558310b518f0dfcd8860 5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 0932c20560574696cf87ddd12623e8c423ee821b 81646cea826fa322831fffb43f81e7e0866dc124 Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#3ab4436f688c2d2f221793953cd05435ca84261c-a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/osstest/ovmf.git#5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc-91cc60bafc7d6e49b7bc85990f895d6228f51364 git://xenbits.xen.org/qemu-xen-traditional.\ git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#9cca02d8ffc23e9688a971d858e4ffdff5389b11-9cca02d8ffc23e9688a971d858e4ffdff5389b11 git://xenbits.xen.org/osstest/seabios.git#0932c20560574696cf87ddd12623e8c423ee821b-30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 git://xenbits.xen.org/xen.git#81646cea826fa322831fffb43f81e7e0866dc124-b541287c3600713feaaaf7608cd405e7b2e4efd0 adhoc-revtuple-generator: tree discontiguous: linux-2.6 Loaded 3002 nodes in revision graph Searching for test results: 137098 [host=albana0] 137125 pass 3ab4436f688c2d2f221793953cd05435ca84261c c530a75c1e6a472b0eb9558310b518f0dfcd8860 5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc d0d8ad39ecb51cd7497cd524484fe09f50876798 9cca02d8ffc23e9688a971d858e4ffdff5389b11 0932c20560574696cf87ddd12623e8c423ee821b 81646cea826fa322831fffb43f81e7e0866dc124 137191 [host=rimava1] 137283 [host=baroque1] 137388 [host=elbling1] 137484 [host=debina1] 137589 [host=albana1] 137739 [host=fiano0] 137686 [host=italia0] 137896 [host=debina0] 137986 [host=elbling0] 138073 [host=elbling1] 138245 [host=chardonnay0] 138386 [host=chardonnay1] 138488 [host=italia0] 138584 [host=albana0] 138612 [host=debina1] 138639 [host=rimava1] 138661 [host=fiano0] 138680 [host=albana1] 138710
Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
On 15.07.2019 13:09, Paul Durrant wrote: >> From: Jan Beulich >> Sent: 15 July 2019 11:44 >> >> Well, the problem is that I don't feel well adjusting what a native >> English speaking person has written. > > Ok. How about... > > "This patch adds a helper function, put_page_alloc_ref(), to replace > all the open-coded test-and-clear/put_page occurrences. That helper > function incorporates a check that an additional page reference is > held and will BUG() if it is not." Sounds good. I'll record this for merging in if no other need for a v3 arises. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Make myself libxl golang binding maintainer
On 15/07/2019 13:01, George Dunlap wrote: > Signed-off-by: George Dunlap Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] MAINTAINERS: Make myself libxl golang binding maintainer
Signed-off-by: George Dunlap --- v2: - Put in alphabetical order - Replace spaces with tabs CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan CC: Konrad Wilk CC: Stefano Stabellini CC: Julien Grall --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b8e4daae41..f18567b56a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -234,6 +234,11 @@ S: Supported F: xen/arch/x86/debug.c F: tools/debugger/gdbsx/ +GOLANG BINDINGS +M: George Dunlap +S: Maintained +F: tools/golang + INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT) M: Gang Wei M: Shane Wang -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] MAINTAINERS: Make myself libxl golang binding maintainer
On 7/10/19 3:51 AM, Jan Beulich wrote: > On 09.07.2019 22:13, George Dunlap wrote: >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -343,6 +343,11 @@ M: Marek Marczykowski-Górecki >> >> S: Supported >> F: tools/python >> >> +GOLANG BINDINGS >> +M: George Dunlap >> +S: Maintained >> +F: tools/golang >> + >> QEMU-DM >> M: Ian Jackson >> S: Supported > > This doesn't look to be the alphabetically correct slot to insert > this section. Yet instead of moving it perhaps its title should > be "libxl golang bindings"? Didn't realize this was in alphabetical order. I was going to say 'libxl golang bindings', but the actual directory in question is, like the python bindings it's next to, generic; someone could if they wanted to write golang bindings for any of the other libraries, and they'd be in this directory; and I'd probably be the right person to review them if they went in. > Also please use tabs like surrounding sections do. Ack. v2 on the way. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH
On 15/07/2019 12:46, Roger Pau Monné wrote: >> diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm >> b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm >> new file mode 100644 >> index 00..2a17fed52f >> --- /dev/null >> +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm >> @@ -0,0 +1,49 @@ >> +;-- >> +; @file >> +; An entry point use by Xen when a guest is started in PVH mode. >> +; >> +; Copyright (c) 2019, Citrix Systems, Inc. >> +; >> +; SPDX-License-Identifier: BSD-2-Clause-Patent >> +; >> +;-- >> + >> +BITS32 >> + >> +xenPVHMain: >> +; >> +; 'BP' to indicate boot-strap processor >> +; >> +mov di, 'BP' >> + >> +; >> +; ESP will be used as initial value of the EAX register >> +; in Main.asm >> +; >> +xor esp, esp >> + >> +mov ebx, ADDR_OF(gdtr) >> +lgdt[ebx] >> + >> +mov eax, SEC_DEFAULT_CR0 >> +mov cr0, eax >> + >> +jmp LINEAR_CODE_SEL:ADDR_OF(.jmpToNewCodeSeg) >> +.jmpToNewCodeSeg: >> + >> +mov eax, SEC_DEFAULT_CR4 >> +mov cr4, eax >> + >> +mov ax, LINEAR_SEL >> +mov ds, ax >> +mov es, ax >> +mov fs, ax >> +mov gs, ax >> +mov ss, ax >> + >> +; >> +; Jump to the main routine of the pre-SEC code >> +; skiping the 16-bit part of the routine and >> +; into the 32-bit flat mode part >> +; >> +OneTimeCallRet TransitionFromReal16To32BitFlat > Since PVH already starts in flat 32bit mode, I'm not sure I see the > point of this routine, since it seems to be used exclusively to switch > from 16 to 32b flat mode. The comment mentions skipping that part, but > I'm not sure I see how that's achieved. Its some OVMF local magic. This means "jmp end_of_TransitionFromReal16To32BitFlat", which is the correct place to go, but the code really is misleading to read. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH
On Thu, Jul 04, 2019 at 03:42:04PM +0100, Anthony PERARD wrote: > Add a new entry point for Xen PVH that enter directly in 32bits. > > Information on the expected state of the machine when this entry point > is used can be found at: > https://xenbits.xenproject.org/docs/unstable/misc/pvh.html > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 > Signed-off-by: Anthony PERARD Thanks for doing this! My knowledge of nasm is very limited, so some of the above comments might be completely wrong. > --- > > Notes: > v3: > - rebased, SPDX > - remove `cli' as via PVH the interrupts are guaranteed to be off > - rewrite some comments > > .../XenResetVector/Ia16/ResetVectorVtf0.asm | 81 +++ > OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm| 49 +++ > OvmfPkg/XenResetVector/XenResetVector.nasmb | 1 + > 3 files changed, 131 insertions(+) > create mode 100644 OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > create mode 100644 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > > diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > new file mode 100644 > index 00..958195bc5e > --- /dev/null > +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > @@ -0,0 +1,81 @@ > +;-- > +; @file > +; First code executed by processor after resetting. > +; > +; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved. Extraneous tag? > +; Copyright (c) 2019, Citrix Systems, Inc. > +; > +; SPDX-License-Identifier: BSD-2-Clause-Patent > +; > +;-- > + > +BITS16 > + > +ALIGN 16 Do you need the BITS and ALIGN here? Isn't it enough with the BITS 32 below for the entry point, since DB is already explicitly sized? > + > +; > +; Pad the image size to 4k when page tables are in VTF0 > +; > +; If the VTF0 image has page tables built in, then we need to make > +; sure the end of VTF0 is 4k above where the page tables end. > +; > +; This is required so the page tables will be 4k aligned when VTF0 is > +; located just below 0x1 (4GB) in the firmware device. > +; > +%ifdef ALIGN_TOP_TO_4K_FOR_PAGING > +TIMES (0x1000 - ($ - EndOfPageTables) - (fourGigabytes - > xenPVHEntryPoint)) DB 0 What's the meaning of 0x1000 here? > +%endif > + > +BITS32 > +xenPVHEntryPoint: > +; > +; Entry point to use when running as a Xen PVH guest. (0xffd0) Shouldn't this positioning be set on the linker script instead? > +; > +; Description of the expected state of the machine when this entry point is > +; used can be found at: > +; https://xenbits.xenproject.org/docs/unstable/misc/pvh.html > +; > +jmp xenPVHMain > + > +BITS16 > +ALIGN 16 Is it really needed to specify both? I would assume that setting BITS 16 will already set a suitable alignment. > + > +applicationProcessorEntryPoint: > +; > +; Application Processors entry point > +; > +; GenFv generates code aligned on a 4k boundary which will jump to this > +; location. (0xffe0) This allows the Local APIC Startup IPI to be Also, if xenPVHEntryPoint is at 0x...d0, how can applicationProcessorEntryPoint be at 0x...e0, I guess there's some other code I'm missing that either adds padding between both, or places them in different sections on the resulting binary image? > +; used to wake up the application processors. > +; > +jmp EarlyApInitReal16 > + > +ALIGN 8 > + > +DD 0 Can you remove this DD... > + > +; > +; The VTF signature > +; > +; VTF-0 means that the VTF (Volume Top File) code does not require > +; any fixups. > +; > +vtfSignature: > +DB 'V', 'T', 'F', 0 And instead do DB 0, 0, 0, 0, 'V',...? In any case I'm not sure of the point of setting align to 8 and then writing 32bits of 0s (but again maybe I'm just misreading the code). Maybe you just want to set align to 32 and write the vtf signature? > + > +ALIGN 16 > + > +resetVector: > +; > +; Reset Vector > +; > +; This is where the processor will begin execution > +; > +nop > +nop > +jmp EarlyBspInitReal16 > + > +ALIGN 16 > + > +fourGigabytes: > + > diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > new file mode 100644 > index 00..2a17fed52f > --- /dev/null > +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > @@ -0,0 +1,49 @@ > +;-- > +; @file > +; An entry point use by Xen when a guest is started in PVH mode. > +; > +; Copyright (c) 2019, Citrix Systems, Inc. > +; > +; SPDX-License-Identifier: BSD-2-Clause-Patent > +; > +;-- > + > +BITS32 > + > +xenPVHMain: > +; > +; 'BP' to indicate boot-strap processor > +; > +mov di, 'BP' > + > +
[Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support
Xen is requiring 64-bit machines today. There is no need to carry the burden of 32-bit PV guest support in the kernel any longer, as new guests can be either HVM or PVH, or they can use a 64 bit kernel. Remove the 32-bit Xen PV support from the kernel. Signed-off-by: Juergen Gross --- arch/x86/entry/entry_32.S | 93 arch/x86/include/asm/proto.h | 2 +- arch/x86/include/asm/segment.h | 2 +- arch/x86/include/asm/traps.h | 2 +- arch/x86/xen/Kconfig | 3 +- arch/x86/xen/Makefile | 4 +- arch/x86/xen/apic.c| 17 --- arch/x86/xen/enlighten_pv.c| 45 +- arch/x86/xen/mmu_pv.c | 326 - arch/x86/xen/p2m.c | 4 - arch/x86/xen/setup.c | 44 +- arch/x86/xen/smp_pv.c | 19 +-- arch/x86/xen/xen-asm.S | 14 -- arch/x86/xen/xen-asm_32.S | 207 -- arch/x86/xen/xen-head.S| 6 - arch/x86/xen/xen-ops.h | 5 - drivers/xen/Kconfig| 4 +- 17 files changed, 42 insertions(+), 755 deletions(-) delete mode 100644 arch/x86/xen/xen-asm_32.S diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S index 7b23431be5cb..d4464af28212 100644 --- a/arch/x86/entry/entry_32.S +++ b/arch/x86/entry/entry_32.S @@ -787,16 +787,6 @@ GLOBAL(__begin_SYSENTER_singlestep_region) * will ignore all of the single-step traps generated in this range. */ -#ifdef CONFIG_XEN_PV -/* - * Xen doesn't set %esp to be precisely what the normal SYSENTER - * entry point expects, so fix it up before using the normal path. - */ -ENTRY(xen_sysenter_target) - addl$5*4, %esp /* remove xen-provided frame */ - jmp .Lsysenter_past_esp -#endif - /* * 32-bit SYSENTER entry. * @@ -1249,89 +1239,6 @@ ENTRY(spurious_interrupt_bug) jmp common_exception END(spurious_interrupt_bug) -#ifdef CONFIG_XEN_PV -ENTRY(xen_hypervisor_callback) - pushl $-1 /* orig_ax = -1 => not a system call */ - SAVE_ALL - ENCODE_FRAME_POINTER - TRACE_IRQS_OFF - - /* -* Check to see if we got the event in the critical -* region in xen_iret_direct, after we've reenabled -* events and checked for pending events. This simulates -* iret instruction's behaviour where it delivers a -* pending interrupt when enabling interrupts: -*/ - movlPT_EIP(%esp), %eax - cmpl$xen_iret_start_crit, %eax - jb 1f - cmpl$xen_iret_end_crit, %eax - jae 1f - - jmp xen_iret_crit_fixup - -ENTRY(xen_do_upcall) -1: mov %esp, %eax - callxen_evtchn_do_upcall -#ifndef CONFIG_PREEMPT - callxen_maybe_preempt_hcall -#endif - jmp ret_from_intr -ENDPROC(xen_hypervisor_callback) - -/* - * Hypervisor uses this for application faults while it executes. - * We get here for two reasons: - * 1. Fault while reloading DS, ES, FS or GS - * 2. Fault while executing IRET - * Category 1 we fix up by reattempting the load, and zeroing the segment - * register if the load fails. - * Category 2 we fix up by jumping to do_iret_error. We cannot use the - * normal Linux return path in this case because if we use the IRET hypercall - * to pop the stack frame we end up in an infinite loop of failsafe callbacks. - * We distinguish between categories by maintaining a status value in EAX. - */ -ENTRY(xen_failsafe_callback) - pushl %eax - movl$1, %eax -1: mov 4(%esp), %ds -2: mov 8(%esp), %es -3: mov 12(%esp), %fs -4: mov 16(%esp), %gs - /* EAX == 0 => Category 1 (Bad segment) - EAX != 0 => Category 2 (Bad IRET) */ - testl %eax, %eax - popl%eax - lea 16(%esp), %esp - jz 5f - jmp iret_exc -5: pushl $-1 /* orig_ax = -1 => not a system call */ - SAVE_ALL - ENCODE_FRAME_POINTER - jmp ret_from_exception - -.section .fixup, "ax" -6: xorl%eax, %eax - movl%eax, 4(%esp) - jmp 1b -7: xorl%eax, %eax - movl%eax, 8(%esp) - jmp 2b -8: xorl%eax, %eax - movl%eax, 12(%esp) - jmp 3b -9: xorl%eax, %eax - movl%eax, 16(%esp) - jmp 4b -.previous - _ASM_EXTABLE(1b, 6b) - _ASM_EXTABLE(2b, 7b) - _ASM_EXTABLE(3b, 8b) - _ASM_EXTABLE(4b, 9b) -ENDPROC(xen_failsafe_callback) -#endif /* CONFIG_XEN_PV */ - #ifdef CONFIG_XEN_PVHVM BUILD_INTERRUPT3(xen_hvm_callback_vector, HYPERVISOR_CALLBACK_VECTOR, xen_evtchn_do_upcall) diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h index 6e81788a30c1..e5a13a138b01 100644 --- a/arch/x86/include/asm/proto.h +++ b/arch/x86/include/asm/proto.h @@ -25,7 +25,7 @@ void entry_SYSENTER_compat(void); void
[Xen-devel] [PATCH 2/2] x86/paravirt: remove 32-bit support from PARAVIRT_XXL
The last 32-bit user of stuff under CONFIG_PARAVIRT_XXL is gone. Remove 32-bit specific parts. Signed-off-by: Juergen Gross --- arch/x86/entry/vdso/vdso32/vclock_gettime.c | 1 + arch/x86/include/asm/paravirt.h | 105 arch/x86/include/asm/paravirt_types.h | 20 -- arch/x86/include/asm/pgtable-3level_types.h | 5 -- arch/x86/kernel/cpu/common.c| 8 --- arch/x86/kernel/paravirt.c | 17 - arch/x86/kernel/paravirt_patch_32.c | 36 +- 7 files changed, 15 insertions(+), 177 deletions(-) diff --git a/arch/x86/entry/vdso/vdso32/vclock_gettime.c b/arch/x86/entry/vdso/vdso32/vclock_gettime.c index 9242b28418d5..36f4ce1405cb 100644 --- a/arch/x86/entry/vdso/vdso32/vclock_gettime.c +++ b/arch/x86/entry/vdso/vdso32/vclock_gettime.c @@ -17,6 +17,7 @@ #undef CONFIG_ILLEGAL_POINTER_VALUE #undef CONFIG_SPARSEMEM_VMEMMAP #undef CONFIG_NR_CPUS +#undef CONFIG_PARAVIRT_XXL #define CONFIG_X86_32 1 #define CONFIG_PGTABLE_LEVELS 2 diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h index c25c38a05c1c..60dfa93313a9 100644 --- a/arch/x86/include/asm/paravirt.h +++ b/arch/x86/include/asm/paravirt.h @@ -139,7 +139,6 @@ static inline void __write_cr4(unsigned long x) PVOP_VCALL1(cpu.write_cr4, x); } -#ifdef CONFIG_X86_64 static inline unsigned long read_cr8(void) { return PVOP_CALL0(unsigned long, cpu.read_cr8); @@ -149,7 +148,6 @@ static inline void write_cr8(unsigned long x) { PVOP_VCALL1(cpu.write_cr8, x); } -#endif static inline void arch_safe_halt(void) { @@ -283,12 +281,10 @@ static inline void load_TLS(struct thread_struct *t, unsigned cpu) PVOP_VCALL2(cpu.load_tls, t, cpu); } -#ifdef CONFIG_X86_64 static inline void load_gs_index(unsigned int gs) { PVOP_VCALL1(cpu.load_gs_index, gs); } -#endif static inline void write_ldt_entry(struct desc_struct *dt, int entry, const void *desc) @@ -375,50 +371,28 @@ static inline pte_t __pte(pteval_t val) { pteval_t ret; - if (sizeof(pteval_t) > sizeof(long)) - ret = PVOP_CALLEE2(pteval_t, mmu.make_pte, val, (u64)val >> 32); - else - ret = PVOP_CALLEE1(pteval_t, mmu.make_pte, val); + ret = PVOP_CALLEE1(pteval_t, mmu.make_pte, val); return (pte_t) { .pte = ret }; } static inline pteval_t pte_val(pte_t pte) { - pteval_t ret; - - if (sizeof(pteval_t) > sizeof(long)) - ret = PVOP_CALLEE2(pteval_t, mmu.pte_val, - pte.pte, (u64)pte.pte >> 32); - else - ret = PVOP_CALLEE1(pteval_t, mmu.pte_val, pte.pte); - - return ret; + return PVOP_CALLEE1(pteval_t, mmu.pte_val, pte.pte); } static inline pgd_t __pgd(pgdval_t val) { pgdval_t ret; - if (sizeof(pgdval_t) > sizeof(long)) - ret = PVOP_CALLEE2(pgdval_t, mmu.make_pgd, val, (u64)val >> 32); - else - ret = PVOP_CALLEE1(pgdval_t, mmu.make_pgd, val); + ret = PVOP_CALLEE1(pgdval_t, mmu.make_pgd, val); return (pgd_t) { ret }; } static inline pgdval_t pgd_val(pgd_t pgd) { - pgdval_t ret; - - if (sizeof(pgdval_t) > sizeof(long)) - ret = PVOP_CALLEE2(pgdval_t, mmu.pgd_val, - pgd.pgd, (u64)pgd.pgd >> 32); - else - ret = PVOP_CALLEE1(pgdval_t, mmu.pgd_val, pgd.pgd); - - return ret; + return PVOP_CALLEE1(pgdval_t, mmu.pgd_val, pgd.pgd); } #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION @@ -435,79 +409,48 @@ static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep, pte_t old_pte, pte_t pte) { - - if (sizeof(pteval_t) > sizeof(long)) - /* 5 arg words */ - pv_ops.mmu.ptep_modify_prot_commit(vma, addr, ptep, pte); - else - PVOP_VCALL4(mmu.ptep_modify_prot_commit, - vma, addr, ptep, pte.pte); + PVOP_VCALL4(mmu.ptep_modify_prot_commit, vma, addr, ptep, pte.pte); } static inline void set_pte(pte_t *ptep, pte_t pte) { - if (sizeof(pteval_t) > sizeof(long)) - PVOP_VCALL3(mmu.set_pte, ptep, pte.pte, (u64)pte.pte >> 32); - else - PVOP_VCALL2(mmu.set_pte, ptep, pte.pte); + PVOP_VCALL2(mmu.set_pte, ptep, pte.pte); } static inline void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte) { - if (sizeof(pteval_t) > sizeof(long)) - /* 5 arg words */ - pv_ops.mmu.set_pte_at(mm, addr, ptep, pte); - else - PVOP_VCALL4(mmu.set_pte_at, mm, addr, ptep, pte.pte); +
[Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support
The long term plan has been to replace Xen PV guests by PVH. The first victim of that plan are now 32-bit PV guests, as those are used only rather seldom these days. Xen on x86 requires 64-bit support and with Grub2 now supporting PVH officially since version 2.04 there is no need to keep 32-bit PV guest support alive in the Linux kernel. Additionally Meltdown mitigation is not available in the kernel running as 32-bit PV guest, so dropping this mode makes sense from security point of view, too. Juergen Gross (2): x86/xen: remove 32-bit Xen PV guest support x86/paravirt: remove 32-bit support from PARAVIRT_XXL arch/x86/entry/entry_32.S | 93 arch/x86/entry/vdso/vdso32/vclock_gettime.c | 1 + arch/x86/include/asm/paravirt.h | 105 + arch/x86/include/asm/paravirt_types.h | 20 -- arch/x86/include/asm/pgtable-3level_types.h | 5 - arch/x86/include/asm/proto.h| 2 +- arch/x86/include/asm/segment.h | 2 +- arch/x86/include/asm/traps.h| 2 +- arch/x86/kernel/cpu/common.c| 8 - arch/x86/kernel/paravirt.c | 17 -- arch/x86/kernel/paravirt_patch_32.c | 36 +-- arch/x86/xen/Kconfig| 3 +- arch/x86/xen/Makefile | 4 +- arch/x86/xen/apic.c | 17 -- arch/x86/xen/enlighten_pv.c | 45 +--- arch/x86/xen/mmu_pv.c | 326 +++- arch/x86/xen/p2m.c | 4 - arch/x86/xen/setup.c| 44 +--- arch/x86/xen/smp_pv.c | 19 +- arch/x86/xen/xen-asm.S | 14 -- arch/x86/xen/xen-asm_32.S | 207 -- arch/x86/xen/xen-head.S | 6 - arch/x86/xen/xen-ops.h | 5 - drivers/xen/Kconfig | 4 +- 24 files changed, 57 insertions(+), 932 deletions(-) delete mode 100644 arch/x86/xen/xen-asm_32.S -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [GSoC-2019] About the crossbar and xen
Hi, On 12/07/2019 19:32, Hunyue Yau z wrote: On Friday, July 12, 2019 16:13:32 Julien Grall wrote: Hi, On 7/11/19 7:29 PM, Hunyue Yau wrote: [This mail incorporates comments raised on IRC. I have made some of this mHi,ore verbose to provide context to people that haven't seen the IRC comments.] Thank you for the summary! This will be a bunch of facts on the am5. Someone else will have relate it back to Xen. 1 - The WUGen is a hardware block on the MPU block that can turn interrupts into wake up events if the MPU is in certain deeper sleep states. This applies only to certain interrupts. We can confirm this by looking at the DT's register address. Per the TRM, they are registers for the MPU's PRCM (Power/Reset/Clock Management). In short, this block makes interrupts a possible wake up source. 2 - Earlier kernels did not expose that HW block. See this patch that introduced the WUGen - https://github.com/torvalds/linux/commit/7136d457f365ecc93ddffcdd42ab49a84 73f260b I suspect looking at the before part of the patch should provide clues on how to handle the WUGen. 3 - This may be redundant but more definitions (in the human sense) here: https://www.mjmwired.net/kernel/Documentation/devicetree/bindings/interrup t-controller/ti,omap4-wugen-mpu 4 - For the UART case, I suspect the flow Dennis pointed out is about right. However, that may be different depending on the interrupt source. Unknowns from my point of view - a - Does Xen virtualize power management? If so, this may have have an impact. I would not recommend adding PM virtualization in GSoC. It is tugging on a very long string. Xen does not virtualize power management at the moment. I agree that this is too big for the scope of the GSoC. b - If Xen does not virtualize that, someone needs to decide how much to leave to the guess. c - I wonder if we can do a half way hack where the kernel sets up the PM but Xen hooks to get the interrupt. The HW will do its PM thing and Xen can process the interrupt. Hmmm, the question here is whether the UART would be usuable before Dom0 is setting up the PM? If not, then, we would need to deal with it in Xen. Guesses/possible hacks - - For the interrupts we care about, the cross bar can route things to the MPU unconditionally. This would break the other HW blocks but most of them aren't needed for boot. Sorry for my ignorance, which HW blocks are you talking about? The HW blocks I am referring to are stuff like EVE, IPU, and DSP. Initially, we can even ignore the PRUSS. This should leave just sending interrupts to the MPU. As I understand it, there is no current support for those right now anyways. I think EVE/IPU/DSP require a working cmem driver which is not fully upstreamed. BBAI does use them but that requires a specific kernel. PRUSS would be nice (aka the PRU stuff) eventually as the bits are upstream but not critical for now. Thank you for the clarification. My knowledge on the board is limited, so I will take yours comments as granted :). Cheers, Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.9-testing test] 138992: regressions - trouble: fail/pass/starved
flight 138992 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/138992/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-prev 6 xen-build fail in 138772 REGR. vs. 132889 build-amd64-prev 6 xen-build fail in 138772 REGR. vs. 132889 Tests which are failing intermittently (not blocking): test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail in 138772 pass in 138992 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail in 138772 pass in 138992 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail in 138919 pass in 138772 test-amd64-amd64-xl-qemut-win7-amd64 13 guest-saverestore fail in 138919 pass in 138992 test-amd64-i386-xl-qemut-win7-amd64 13 guest-saverestore fail in 138919 pass in 138992 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 17 guest-stop fail in 138919 pass in 138992 test-amd64-i386-freebsd10-i386 17 guest-localmigrate/x10 fail in 138951 pass in 138992 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 138951 pass in 138992 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail pass in 138842 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail pass in 138919 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 138919 test-amd64-i386-freebsd10-amd64 17 guest-localmigrate/x10 fail pass in 138951 Tests which did not succeed, but are not blocking: test-amd64-i386-migrupgrade 1 build-check(1) blocked in 138772 n/a test-amd64-amd64-migrupgrade 1 build-check(1) blocked in 138772 n/a test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail blocked in 132889 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail blocked in 132889 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail blocked in 132889 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail in 138842 like 132889 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail in 138919 like 132889 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 138951 like 132889 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132889 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 132889 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-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-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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 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-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass
Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
> -Original Message- > From: Jan Beulich > Sent: 15 July 2019 11:44 > To: Paul Durrant > Cc: JulienGrall ; Andrew Cooper > ; George Dunlap > ; Ian Jackson ; Roger Pau > Monne > ; VolodymyrBabchuk ; > Stefano Stabellini > ; xen-devel@lists.xenproject.org; Konrad Rzeszutek > Wilk > ; Tamas K Lengyel ; Tim > (Xen.org) ; Wei Liu > > Subject: Re: [PATCH v2] xen/mm.h: add helper function to test-and-clear > _PGC_allocated > > On 15.07.2019 11:50, Paul Durrant wrote: > >> From: Jan Beulich > >> Sent: 15 July 2019 10:24 > >> > >> On 15.07.2019 11:17, Paul Durrant wrote: > >>> The _PGC_allocated flag is set on a page when it is assigned to a domain > >>> along with an initial reference count of at least 1. To clear this > >>> 'allocation' reference it is necessary to test-and-clear _PGC_allocated > >>> and > >>> then only drop the reference if the test-and-clear succeeds. This is open- > >>> coded in many places. It is also unsafe to test-and-clear _PGC_allocated > >>> unless the caller holds an additional reference. > >>> > >>> This patch adds a helper function, put_page_alloc_ref(), to replace all > >>> the > >>> open-coded test-and-clear/put_page occurrences and incorporates in that a > >>> BUG_ON() an additional page reference not being held. > >> > >> This last sentence reads somewhat strange to me - are there words > >> missing and/or mis-ordered? > > > > Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'? > > I just wanted to express that there was a new check in the helper > > function that the necessary additional reference is held. > > But then still more like "... incorporates in a BUG() on that an > additional ..."? At which point it imo could as well be "... > incorporates in a BUG_ON() that an additional ..." (i.e. just a > word order change from your original sentence). (There's then > perhaps also an "is" missing later in the sentence.) > > >>> Signed-off-by: Paul Durrant > >> > >> With the commit message aspect clarified > > > > I am happy for you to re-word it if you feel it is not clear. > > Well, the problem is that I don't feel well adjusting what a native > English speaking person has written. Ok. How about... "This patch adds a helper function, put_page_alloc_ref(), to replace all the open-coded test-and-clear/put_page occurrences. That helper function incorporates a check that an additional page reference is held and will BUG() if it is not." ? Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
On 15.07.2019 11:50, Paul Durrant wrote: >> From: Jan Beulich >> Sent: 15 July 2019 10:24 >> >> On 15.07.2019 11:17, Paul Durrant wrote: >>> The _PGC_allocated flag is set on a page when it is assigned to a domain >>> along with an initial reference count of at least 1. To clear this >>> 'allocation' reference it is necessary to test-and-clear _PGC_allocated and >>> then only drop the reference if the test-and-clear succeeds. This is open- >>> coded in many places. It is also unsafe to test-and-clear _PGC_allocated >>> unless the caller holds an additional reference. >>> >>> This patch adds a helper function, put_page_alloc_ref(), to replace all the >>> open-coded test-and-clear/put_page occurrences and incorporates in that a >>> BUG_ON() an additional page reference not being held. >> >> This last sentence reads somewhat strange to me - are there words >> missing and/or mis-ordered? > > Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'? > I just wanted to express that there was a new check in the helper > function that the necessary additional reference is held. But then still more like "... incorporates in a BUG() on that an additional ..."? At which point it imo could as well be "... incorporates in a BUG_ON() that an additional ..." (i.e. just a word order change from your original sentence). (There's then perhaps also an "is" missing later in the sentence.) >>> Signed-off-by: Paul Durrant >> >> With the commit message aspect clarified > > I am happy for you to re-word it if you feel it is not clear. Well, the problem is that I don't feel well adjusting what a native English speaking person has written. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V4 9/9] xen/blkback: use helper in vbd_sz()
On Mon, Jul 08, 2019 at 11:47:11AM -0700, Chaitanya Kulkarni wrote: > This patch updates the vbd_sz() macro with newly introduced helper > function to read the nr_sects from block device's hd_parts with the > help of part_nr_sects_read(). > > Signed-off-by: Chaitanya Kulkarni Acked-by: Roger Pau Monné Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/dom0-build: fix build with clang5
With non-empty CONFIG_DOM0_MEM clang5 produces dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^ ~~ dom0_build.c:344:24: note: use '&' for a bitwise operation if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^~ & dom0_build.c:344:24: note: remove constant to silence this warning if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ~^ 1 error generated. Obviously neither of the two suggestions are an option here. Oddly enough swapping the operands of the && helps, while e.g. casting or parenthesizing doesn't. Another workable variant looks to be the use of !! on the constant. Signed-off-by: Jan Beulich --- I'm open to going the !! or yet some different route. No matter which one we choose, I'm afraid it is going to remain guesswork what newer (and future) versions of clang will choke on. --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag unsigned long avail = 0, nr_pages, min_pages, max_pages; bool need_paging; -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set ) parse_dom0_mem(CONFIG_DOM0_MEM); for_each_node_mask ( node, dom0_nodes ) x86/dom0-build: fix build with clang5 With non-empty CONFIG_DOM0_MEM clang5 produces dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand] if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^ ~~ dom0_build.c:344:24: note: use '&' for a bitwise operation if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ^~ & dom0_build.c:344:24: note: remove constant to silence this warning if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) ~^ 1 error generated. Obviously neither of the two suggestions are an option here. Oddly enough swapping the operands of the && helps, while e.g. casting or parenthesizing doesn't. Another workable variant looks to be the use of !! on the constant. Signed-off-by: Jan Beulich --- I'm open to going the !! or yet some different route. No matter which one we choose, I'm afraid it is going to remain guesswork what newer (and future) versions of clang will choke on. --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag unsigned long avail = 0, nr_pages, min_pages, max_pages; bool need_paging; -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set ) parse_dom0_mem(CONFIG_DOM0_MEM); for_each_node_mask ( node, dom0_nodes ) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: [PATCH v2] timers: limit heap size
On Fri, Jul 05, 2019 at 04:06:06PM +, Jan Beulich wrote: > >>> On 05.06.19 at 08:51, wrote: > > First and foremost make timer_softirq_action() avoid growing the heap > > if its new size can't be stored without truncation. 64k entries is a > > lot, and I don't think we're at risk of actually running into the issue, > > but I also think it's better not to allow for hard to debug problems to > > occur in the first place. > > > > Furthermore also adjust the code such the size/limit fields becoming > > unsigned int would at least work from a mere sizing point of view. For > > this also switch various uses of plain int to unsigned int. > > > > Signed-off-by: Jan Beulich Thanks: Reviewed-by: Roger Pau Monné I however wonder whether all this heap timer management plus the extra list is really the best option, using a balanced tree seems like a better option here. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
> -Original Message- > From: Jan Beulich > Sent: 15 July 2019 10:24 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Julien Grall ; > Andrew Cooper > ; Roger Pau Monne ; > VolodymyrBabchuk > ; George Dunlap ; Ian > Jackson > ; Stefano Stabellini ; Konrad > Rzeszutek Wilk > ; Tamas K Lengyel ; Tim > (Xen.org) ; Wei Liu > > Subject: Re: [PATCH v2] xen/mm.h: add helper function to test-and-clear > _PGC_allocated > > On 15.07.2019 11:17, Paul Durrant wrote: > > The _PGC_allocated flag is set on a page when it is assigned to a domain > > along with an initial reference count of at least 1. To clear this > > 'allocation' reference it is necessary to test-and-clear _PGC_allocated and > > then only drop the reference if the test-and-clear succeeds. This is open- > > coded in many places. It is also unsafe to test-and-clear _PGC_allocated > > unless the caller holds an additional reference. > > > > This patch adds a helper function, put_page_alloc_ref(), to replace all the > > open-coded test-and-clear/put_page occurrences and incorporates in that a > > BUG_ON() an additional page reference not being held. > > This last sentence reads somewhat strange to me - are there words > missing and/or mis-ordered? Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'? I just wanted to express that there was a new check in the helper function that the necessary additional reference is held. > > > Signed-off-by: Paul Durrant > > With the commit message aspect clarified I am happy for you to re-word it if you feel it is not clear. With the extra comment in the helper function in v2 then perhaps it is not really necessary to have any additional explanation in the commit comment anyway? > Acked-by: Jan Beulich Thanks, Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Notes from summit design session on branch management
On Fri, Jul 12, 2019 at 10:41 PM Ian Jackson wrote: > > Here are the photos I took of the flipchart: > https://xenbits.xen.org/people/iwj/2019/summit-ci-branch-workshop/ FYI, I can see the directory, but when I click on the individual images, I get this: You don't have permission to access /people/iwj/2019/summit-ci-branch-workshop/IMG_20190711_115507.jpg on this server. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
On 15.07.2019 11:17, Paul Durrant wrote: > The _PGC_allocated flag is set on a page when it is assigned to a domain > along with an initial reference count of at least 1. To clear this > 'allocation' reference it is necessary to test-and-clear _PGC_allocated and > then only drop the reference if the test-and-clear succeeds. This is open- > coded in many places. It is also unsafe to test-and-clear _PGC_allocated > unless the caller holds an additional reference. > > This patch adds a helper function, put_page_alloc_ref(), to replace all the > open-coded test-and-clear/put_page occurrences and incorporates in that a > BUG_ON() an additional page reference not being held. This last sentence reads somewhat strange to me - are there words missing and/or mis-ordered? > Signed-off-by: Paul Durrant With the commit message aspect clarified 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] xen/mm.h: add helper function to test-and-clear _PGC_allocated
> -Original Message- > From: Jan Beulich > Sent: 15 July 2019 10:18 > To: Paul Durrant > Cc: JulienGrall ; Andrew Cooper > ; George Dunlap > ; Ian Jackson ; Roger Pau > Monne > ; Volodymyr Babchuk ; > Stefano Stabellini > ; xen-devel@lists.xenproject.org; Konrad Rzeszutek > Wilk > ; Tamas K Lengyel ; Tim > (Xen.org) ; Wei Liu > > Subject: Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to > test-and-clear _PGC_allocated > > On 15.07.2019 10:45, Paul Durrant wrote: > >> From: Jan Beulich > >> Sent: 10 July 2019 23:53 > >> > >> On 10.07.2019 18:17, Paul Durrant wrote: > >>> @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct > >>> hvm_ioreq_server *s, bool buf) > >>>unmap_domain_page_global(iorp->va); > >>>iorp->va = NULL; > >>> > >>> -/* > >>> - * Check whether we need to clear the allocation reference before > >>> - * dropping the explicit references taken by get_page_and_type(). > >>> - */ > >>> -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) > >>> -put_page(page); > >>> - > >>> +clear_assignment_reference(page); > >>>put_page_and_type(page); > >>>} > >> > >> Is there a specific reason you drop the comment? It doesn't become > >> less relevant than when it was added, does it? > > > > Not sure, since what's actually going on is now internal to the function. > > If I change the function name to clear_allocation_reference() then I > > think the comment probably becomes extraneous. > > Well, the perspective I'm taking is that the ordering constraint > wrt put_page_and_type() doesn't go away and is a relevant part of > what the comment talks about. Ok. Would you be happy fixing the comment to your taste on commit then, as I'm not sure exactly what you want to say? Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to test-and-clear _PGC_allocated
On 15.07.2019 10:45, Paul Durrant wrote: >> From: Jan Beulich >> Sent: 10 July 2019 23:53 >> >> On 10.07.2019 18:17, Paul Durrant wrote: >>> @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server >>> *s, bool buf) >>>unmap_domain_page_global(iorp->va); >>>iorp->va = NULL; >>> >>> -/* >>> - * Check whether we need to clear the allocation reference before >>> - * dropping the explicit references taken by get_page_and_type(). >>> - */ >>> -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) >>> -put_page(page); >>> - >>> +clear_assignment_reference(page); >>>put_page_and_type(page); >>>} >> >> Is there a specific reason you drop the comment? It doesn't become >> less relevant than when it was added, does it? > > Not sure, since what's actually going on is now internal to the function. > If I change the function name to clear_allocation_reference() then I > think the comment probably becomes extraneous. Well, the perspective I'm taking is that the ordering constraint wrt put_page_and_type() doesn't go away and is a relevant part of what the comment talks about. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated
The _PGC_allocated flag is set on a page when it is assigned to a domain along with an initial reference count of at least 1. To clear this 'allocation' reference it is necessary to test-and-clear _PGC_allocated and then only drop the reference if the test-and-clear succeeds. This is open- coded in many places. It is also unsafe to test-and-clear _PGC_allocated unless the caller holds an additional reference. This patch adds a helper function, put_page_alloc_ref(), to replace all the open-coded test-and-clear/put_page occurrences and incorporates in that a BUG_ON() an additional page reference not being held. Signed-off-by: Paul Durrant --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Volodymyr Babchuk Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu Cc: "Roger Pau Monné" Cc: Tamas K Lengyel Cc: George Dunlap v2: - Re-name clear_assignment_reference() to put_page_alloc_ref() - Swap ASSERT() for BUG_ON() - Add an extra comment explaining what put_page_alloc_ref() is doing --- xen/arch/arm/domain.c | 4 +--- xen/arch/x86/domain.c | 3 +-- xen/arch/x86/hvm/ioreq.c | 11 ++- xen/arch/x86/mm.c | 3 +-- xen/arch/x86/mm/mem_sharing.c | 9 +++-- xen/arch/x86/mm/p2m-pod.c | 4 +--- xen/arch/x86/mm/p2m.c | 3 +-- xen/common/grant_table.c | 3 +-- xen/common/memory.c | 5 ++--- xen/common/xenoprof.c | 3 +-- xen/include/xen/mm.h | 14 ++ 11 files changed, 28 insertions(+), 34 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 4f44d5c742..941bbff4fe 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -926,9 +926,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list) */ continue; -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(page); - +put_page_alloc_ref(page); put_page(page); if ( hypercall_preempt_check() ) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 147f96a09e..e791d86892 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -1939,8 +1939,7 @@ static int relinquish_memory( BUG(); } -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(page); +put_page_alloc_ref(page); /* * Forcibly invalidate top-most, still valid page tables at this point diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c index 7a80cfb28b..a79cabb680 100644 --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -398,8 +398,7 @@ static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf) return 0; fail: -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(page); +put_page_alloc_ref(page); put_page_and_type(page); return -ENOMEM; @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, bool buf) unmap_domain_page_global(iorp->va); iorp->va = NULL; -/* - * Check whether we need to clear the allocation reference before - * dropping the explicit references taken by get_page_and_type(). - */ -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(page); - +put_page_alloc_ref(page); put_page_and_type(page); } diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index df2c0130f1..138662e777 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -498,8 +498,7 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d, void free_shared_domheap_page(struct page_info *page) { -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(page); +put_page_alloc_ref(page); if ( !test_and_clear_bit(_PGC_xen_heap, >count_info) ) ASSERT_UNREACHABLE(); page->u.inuse.type_info = 0; diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index f16a3f5324..58d9157fa8 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -1000,8 +1000,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh, mem_sharing_page_unlock(firstpg); /* Free the client page */ -if(test_and_clear_bit(_PGC_allocated, >count_info)) -put_page(cpage); +put_page_alloc_ref(cpage); put_page(cpage); /* We managed to free a domain page. */ @@ -1082,8 +1081,7 @@ int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, shr_handle ret = -EOVERFLOW; goto err_unlock; } -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) -put_page(cpage); +put_page_alloc_ref(cpage); put_page(cpage); } } @@ -1177,8 +1175,7 @@ int
[Xen-devel] [PATCH v3] xen/pv: Fix a boot up hang revealed by int3 self test
Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest") is used to ensure there is a gap setup in int3 exception stack which could be used for inserting call return address. This gap is missed in XEN PV int3 exception entry path, then below panic triggered: [0.772876] general protection fault: [#1] SMP NOPTI [0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 [0.772893] RIP: e030:int3_magic+0x0/0x7 [0.772905] RSP: 3507:82203e98 EFLAGS: 0246 [0.773334] Call Trace: [0.773334] alternative_instructions+0x3d/0x12e [0.773334] check_bugs+0x7c9/0x887 [0.773334] ? __get_locked_pte+0x178/0x1f0 [0.773334] start_kernel+0x4ff/0x535 [0.773334] ? set_init_arg+0x55/0x55 [0.773334] xen_start_kernel+0x571/0x57a For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with %rcx/%r11 on the stack. To convert back to "normal" looking exceptions, the xen thunks do 'xen_*: pop %rcx; pop %r11; jmp *'. E.g. Extracting 'xen_pv_trap xenint3' we have: xen_xenint3: pop %rcx; pop %r11; jmp xenint3 As xenint3 and int3 entry code are same except xenint3 doesn't generate a gap, we can fix it by using int3 and drop useless xenint3. Signed-off-by: Zhenzhong Duan Cc: Boris Ostrovsky Cc: Juergen Gross Cc: Stefano Stabellini Cc: Andy Lutomirski Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Andrew Cooper --- bootup test pass with PV guest. v3: set ist_okay to false for int3 per PeterZ add Andrew's comments to patch description v2: fix up description. --- arch/x86/entry/entry_64.S| 1 - arch/x86/include/asm/traps.h | 2 +- arch/x86/xen/enlighten_pv.c | 2 +- arch/x86/xen/xen-asm_64.S| 1 - 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 0ea4831..35a66fc 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -1176,7 +1176,6 @@ idtentry stack_segmentdo_stack_segment has_error_code=1 #ifdef CONFIG_XEN_PV idtentry xennmido_nmi has_error_code=0 idtentry xendebug do_debughas_error_code=0 -idtentry xenint3 do_int3 has_error_code=0 #endif idtentry general_protectiondo_general_protection has_error_code=1 diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h index 7d6f3f3..f2bd284 100644 --- a/arch/x86/include/asm/traps.h +++ b/arch/x86/include/asm/traps.h @@ -40,7 +40,7 @@ asmlinkage void xen_divide_error(void); asmlinkage void xen_xennmi(void); asmlinkage void xen_xendebug(void); -asmlinkage void xen_xenint3(void); +asmlinkage void xen_int3(void); asmlinkage void xen_overflow(void); asmlinkage void xen_bounds(void); asmlinkage void xen_invalid_op(void); diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 4722ba2..30c14cb 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -596,12 +596,12 @@ struct trap_array_entry { static struct trap_array_entry trap_array[] = { { debug, xen_xendebug,true }, - { int3,xen_xenint3, true }, { double_fault,xen_double_fault,true }, #ifdef CONFIG_X86_MCE { machine_check, xen_machine_check, true }, #endif { nmi, xen_xennmi, true }, + { int3,xen_int3,false }, { overflow,xen_overflow,false }, #ifdef CONFIG_IA32_EMULATION { entry_INT80_compat, xen_entry_INT80_compat, false }, diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S index 1e9ef0b..ebf610b 100644 --- a/arch/x86/xen/xen-asm_64.S +++ b/arch/x86/xen/xen-asm_64.S @@ -32,7 +32,6 @@ xen_pv_trap divide_error xen_pv_trap debug xen_pv_trap xendebug xen_pv_trap int3 -xen_pv_trap xenint3 xen_pv_trap xennmi xen_pv_trap overflow xen_pv_trap bounds -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test
On 15/07/2019 07:54, Jan Beulich wrote: > On 15.07.2019 07:05, Zhenzhong Duan wrote: >> On 2019/7/12 22:06, Andrew Cooper wrote: >>> On 11/07/2019 03:15, Zhenzhong Duan wrote: Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call() selftest") is used to ensure there is a gap setup in exception stack which could be used for inserting call return address. This gap is missed in XEN PV int3 exception entry path, then below panic triggered: [ 0.772876] general protection fault: [#1] SMP NOPTI [ 0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11 [ 0.772893] RIP: e030:int3_magic+0x0/0x7 [ 0.772905] RSP: 3507:82203e98 EFLAGS: 0246 [ 0.773334] Call Trace: [ 0.773334] alternative_instructions+0x3d/0x12e [ 0.773334] check_bugs+0x7c9/0x887 [ 0.773334] ? __get_locked_pte+0x178/0x1f0 [ 0.773334] start_kernel+0x4ff/0x535 [ 0.773334] ? set_init_arg+0x55/0x55 [ 0.773334] xen_start_kernel+0x571/0x57a As xenint3 and int3 entry code are same except xenint3 doesn't generate a gap, we can fix it by using int3 and drop useless xenint3. >>> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with >>> %rcx/%r11 on the stack. >>> >>> To convert back to "normal" looking exceptions, the xen thunks do `pop >>> %rcx; pop %r11; jmp do_*`... >> I will add this to commit message. diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 0ea4831..35a66fc 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -1176,7 +1176,6 @@ idtentry stack_segment do_stack_segment has_error_code=1 #ifdef CONFIG_XEN_PV idtentry xennmi do_nmi has_error_code=0 idtentry xendebug do_debug has_error_code=0 -idtentry xenint3 do_int3 has_error_code=0 #endif >>> What is confusing is why there are 3 extra magic versions here. At a >>> guess, I'd say something to do with IST settings (given the vectors), >>> but I don't see why #DB/#BP would need to be different in the first >>> place. (NMI sure, but that is more to do with the crazy hoops needing >>> to be jumped through to keep native functioning safely.) >> xenint3 will be removed in this patch safely as it don't use IST now. >> >> But debug and nmi need paranoid_entry which will read MSR_GS_BASE to >> determine >> >> if swapgs is needed. I guess PV guesting running in ring3 will #GP with >> swapgs? > Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit > PV kernel mode always gets entered with kernel GS base already set. Hence > finding out whether to switch the GS base is specifically not something that > any exception entry point would need to do (and it should actively try to > avoid it, for performance reasons). Indeed. The SWAPGS PVOP is implemented as a nop for x86 PV, to simply the entry assembly (rather than doubling up all entry vectors). ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ping²: [PATCH 4/4] x86/PV: remove unnecessary toggle_guest_pt() overhead
>>> On 27.05.19 at 11:25, wrote: On 13.03.19 at 13:39, wrote: > > While the mere updating of ->pv_cr3 and ->root_pgt_changed aren't overly > > expensive (but still needed only for the toggle_guest_mode() path), the > > effect of the latter on the exit-to-guest path is not insignificant. > > Move the logic into toggle_guest_mode(). > > > > Signed-off-by: Jan Beulich > > I think I did address the one concern you had. > > Jan https://lists.xenproject.org/archives/html/xen-devel/2019-04/msg00306.html Ping? > > --- a/xen/arch/x86/pv/domain.c > > +++ b/xen/arch/x86/pv/domain.c > > @@ -349,18 +349,10 @@ bool __init xpti_pcid_enabled(void) > > > > static void _toggle_guest_pt(struct vcpu *v) > > { > > -const struct domain *d = v->domain; > > -struct cpu_info *cpu_info = get_cpu_info(); > > unsigned long cr3; > > > > v->arch.flags ^= TF_kernel_mode; > > update_cr3(v); > > -if ( d->arch.pv.xpti ) > > -{ > > -cpu_info->root_pgt_changed = true; > > -cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) | > > - (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0); > > -} > > > > /* > > * Don't flush user global mappings from the TLB. Don't tick TLB clock. > > @@ -368,15 +360,11 @@ static void _toggle_guest_pt(struct vcpu > > * In shadow mode, though, update_cr3() may need to be accompanied by a > > * TLB flush (for just the incoming PCID), as the top level page table > > may > > * have changed behind our backs. To be on the safe side, suppress the > > - * no-flush unconditionally in this case. The XPTI CR3 write, if > > enabled, > > - * will then need to be a flushing one too. > > + * no-flush unconditionally in this case. > > */ > > cr3 = v->arch.cr3; > > -if ( shadow_mode_enabled(d) ) > > -{ > > +if ( shadow_mode_enabled(v->domain) ) > > cr3 &= ~X86_CR3_NOFLUSH; > > -cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH; > > -} > > write_cr3(cr3); > > > > if ( !(v->arch.flags & TF_kernel_mode) ) > > @@ -392,6 +380,8 @@ static void _toggle_guest_pt(struct vcpu > > > > void toggle_guest_mode(struct vcpu *v) > > { > > +const struct domain *d = v->domain; > > + > > ASSERT(!is_pv_32bit_vcpu(v)); > > > > /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */ > > @@ -405,6 +395,21 @@ void toggle_guest_mode(struct vcpu *v) > > asm volatile ( "swapgs" ); > > > > _toggle_guest_pt(v); > > + > > +if ( d->arch.pv.xpti ) > > +{ > > +struct cpu_info *cpu_info = get_cpu_info(); > > + > > +cpu_info->root_pgt_changed = true; > > +cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) | > > + (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0); > > +/* > > + * As in _toggle_guest_pt() the XPTI CR3 write needs to be a TLB- > > + * flushing one too for shadow mode guests. > > + */ > > +if ( shadow_mode_enabled(d) ) > > +cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH; > > +} > > } > > > > void toggle_guest_pt(struct vcpu *v) > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ping: [PATCH v2] x86/HVM: p2m_ram_ro is incompatible with device pass-through
On 03.07.2019 13:36, Jan Beulich wrote: > The write-discard property of the type can't be represented in IOMMU > page table entries. Make sure the respective checks / tracking can't > race, by utilizing the domain lock. The other sides of the sharing/ > paging/log-dirty exclusion checks should subsequently perhaps also be > put under that lock then. > > Take the opportunity and also convert neighboring bool_t to bool in > struct hvm_domain. > > Signed-off-by: Jan Beulich Alongside Paul's R-b could I get an ack or otherwise from you? Thanks, Jan > --- > v2: Don't set p2m_ram_ro_used when failing the request. > > --- a/xen/arch/x86/hvm/dm.c > +++ b/xen/arch/x86/hvm/dm.c > @@ -255,16 +255,33 @@ static int set_mem_type(struct domain *d > >mem_type = array_index_nospec(data->mem_type, ARRAY_SIZE(memtype)); > > -if ( mem_type == HVMMEM_ioreq_server ) > +switch ( mem_type ) >{ >unsigned int flags; > > +case HVMMEM_ioreq_server: >if ( !hap_enabled(d) ) >return -EOPNOTSUPP; > >/* Do not change to HVMMEM_ioreq_server if no ioreq server mapped. > */ >if ( !p2m_get_ioreq_server(d, ) ) >return -EINVAL; > + > +break; > + > +case HVMMEM_ram_ro: > +/* p2m_ram_ro can't be represented in IOMMU mappings. */ > +domain_lock(d); > +if ( has_iommu_pt(d) ) > +rc = -EXDEV; > +else > +d->arch.hvm.p2m_ram_ro_used = true; > +domain_unlock(d); > + > +if ( rc ) > +return rc; > + > +break; >} > >while ( iter < data->nr ) > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -1448,17 +1448,36 @@ static int assign_device(struct domain * >if ( !iommu_enabled || !hd->platform_ops ) >return 0; > > -/* Prevent device assign if mem paging or mem sharing have been > - * enabled for this domain */ > -if ( unlikely(d->arch.hvm.mem_sharing_enabled || > - vm_event_check_ring(d->vm_event_paging) || > +domain_lock(d); > + > +/* > + * Prevent device assignment if any of > + * - mem paging > + * - mem sharing > + * - the p2m_ram_ro type > + * - global log-dirty mode > + * are in use by this domain. > + */ > +if ( unlikely(vm_event_check_ring(d->vm_event_paging) || > +#ifdef CONFIG_HVM > + (is_hvm_domain(d) && > + (d->arch.hvm.mem_sharing_enabled || > +d->arch.hvm.p2m_ram_ro_used)) || > +#endif > p2m_get_hostp2m(d)->global_logdirty) ) > +{ > +domain_unlock(d); >return -EXDEV; > +} > >if ( !pcidevs_trylock() ) > +{ > +domain_unlock(d); >return -ERESTART; > +} > >rc = iommu_construct(d); > +domain_unlock(d); >if ( rc ) >{ >pcidevs_unlock(); > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -156,10 +156,11 @@ struct hvm_domain { > >struct viridian_domain *viridian; > > -bool_t hap_enabled; > -bool_t mem_sharing_enabled; > -bool_t qemu_mapcache_invalidate; > -bool_t is_s3_suspended; > +bool hap_enabled; > +bool mem_sharing_enabled; > +bool p2m_ram_ro_used; > +bool qemu_mapcache_invalidate; > +bool is_s3_suspended; > >/* > * TSC value that VCPUs use to calculate their tsc_offset value. > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ping³: [PATCH 1/2] core-parking: interact with runtime SMT-disabling
On 03.07.2019 13:29, Jan Beulich wrote: On 27.05.19 at 11:36, wrote: > On 12.04.19 at 13:41, wrote: >> On 11.04.19 at 21:06, wrote: On 11/04/2019 13:45, Jan Beulich wrote: > When disabling SMT at runtime, secondary threads should no longer be > candidates for bringing back up in response to _PUD ACPI events. Purge > them from the tracking array. > > Doing so involves adding locking to guard accounting data in the core > parking code. While adding the declaration for the lock take the liberty > to drop two unnecessary forward function declarations. > > Signed-off-by: Jan Beulich I can certainly appreciate these arguments, but surely the converse is true. When SMT-enable is used, the newly-onlined threads are now eligible to be parked. >>> >>> And nothing will keep them from getting parked. >>> At the moment, this looks asymetric. >>> >>> It does, but that's a result of core_parking.c only recording CPUs >>> it has parked, not ones it could park. >> >> Did my responses address your concerns? >> >> Jan Ping? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate
On 12.07.2019 04:28, Jan Beulich wrote: > On 11.07.2019 19:13, Tamas K Lengyel wrote: >>> @@ -629,6 +697,14 @@ static void *hvmemul_map_linear_addr( >>> >>>ASSERT(p2mt == p2m_ram_logdirty || !p2m_is_readonly(p2mt)); >>>} >>> + >>> +if ( curr->arch.vm_event && >>> +curr->arch.vm_event->send_event && >> >> Why not fold these checks into hvm_emulate_send_vm_event since.. > > I had asked for at least the first of the checks to be pulled > out of the function, for the common case to be affected as > little as possible. > >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -3224,6 +3224,14 @@ static enum hvm_translation_result __hvm_copy( >>>return HVMTRANS_bad_gfn_to_mfn; >>>} >>> >>> +if ( unlikely(v->arch.vm_event) && >>> +v->arch.vm_event->send_event && >> >> .. you seem to just repeat them here again? > > I agree that the duplication makes no sense. > The first check is on the hvmemul_map_linear_addr() path and the second is on hvmemul_insn_fetch() path. There are 2 distinct ways to reach that if and therefore the check is not duplicated. Thanks, Alex ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter optional
> -Original Message- > From: Jan Beulich > Sent: 01 July 2019 13:00 > To: xen-devel@lists.xenproject.org; xen-devel@lists.xenproject.org > Cc: Andrew Cooper ; Wei Liu ; Roger > Pau Monne > ; Paul Durrant > Subject: [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter > optional > > A majority of callers wants just a single iteration handled. Allow to > express this by passing in a NULL pointer, instead of setting up a local > variable just to hold the "1" to pass in here. > > Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant > --- > Note that this conflicts with additions/changes made by "x86emul: > further work". Whatever goes in later will need re-basing. > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -788,14 +788,14 @@ static int hvmemul_virtual_to_linear( > enum x86_segment seg, > unsigned long offset, > unsigned int bytes_per_rep, > -unsigned long *reps, > +unsigned long *reps_p, > enum hvm_access_type access_type, > struct hvm_emulate_ctxt *hvmemul_ctxt, > unsigned long *linear) > { > struct segment_register *reg; > int okay; > -unsigned long max_reps = 4096; > +unsigned long reps = 1; > > if ( seg == x86_seg_none ) > { > @@ -803,62 +803,72 @@ static int hvmemul_virtual_to_linear( > return X86EMUL_OKAY; > } > > -/* > - * If introspection has been enabled for this domain, and we're emulating > - * becase a vm_reply asked us to (i.e. not doing regular IO) reps should > - * be at most 1, since optimization might otherwise cause a single > - * vm_event being triggered for repeated writes to a whole page. > - */ > -if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) && > - current->arch.vm_event->emulate_flags != 0 ) > - max_reps = 1; > +if ( reps_p ) > +{ > +unsigned long max_reps = 4096; > > -/* > - * Clip repetitions to avoid overflow when multiplying by @bytes_per_rep. > - * The chosen maximum is very conservative but it's what we use in > - * hvmemul_linear_to_phys() so there is no point in using a larger value. > - */ > -*reps = min_t(unsigned long, *reps, max_reps); > +/* > + * If introspection has been enabled for this domain, and we're > + * emulating because a vm_reply asked us to (i.e. not doing regular > IO) > + * reps should be at most 1, since optimization might otherwise > cause a > + * single vm_event being triggered for repeated writes to a whole > page. > + */ > +if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) && > + current->arch.vm_event->emulate_flags != 0 ) > + max_reps = 1; > + > +/* > + * Clip repetitions to avoid overflow when multiplying by > + * @bytes_per_rep. The chosen maximum is very conservative but it's > + * what we use in hvmemul_linear_to_phys() so there is no point in > + * using a larger value. > + */ > +reps = *reps_p = min_t(unsigned long, *reps_p, max_reps); > +} > > reg = hvmemul_get_seg_reg(seg, hvmemul_ctxt); > if ( IS_ERR(reg) ) > return -PTR_ERR(reg); > > -if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) ) > +if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (reps > 1) ) > { > /* >* x86_emulate() clips the repetition count to ensure we don't wrap >* the effective-address index register. Hence this assertion holds. >*/ > -ASSERT(offset >= ((*reps - 1) * bytes_per_rep)); > +ASSERT(offset >= ((reps - 1) * bytes_per_rep)); > okay = hvm_virtual_to_linear_addr( > -seg, reg, offset - (*reps - 1) * bytes_per_rep, > -*reps * bytes_per_rep, access_type, > +seg, reg, offset - (reps - 1) * bytes_per_rep, > +reps * bytes_per_rep, access_type, > hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear); > -*linear += (*reps - 1) * bytes_per_rep; > +*linear += (reps - 1) * bytes_per_rep; > if ( hvmemul_ctxt->ctxt.addr_size != 64 ) > *linear = (uint32_t)*linear; > } > else > { > okay = hvm_virtual_to_linear_addr( > -seg, reg, offset, *reps * bytes_per_rep, access_type, > +seg, reg, offset, reps * bytes_per_rep, access_type, > hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear); > } > > if ( okay ) > return X86EMUL_OKAY; > > -/* If this is a string operation, emulate each iteration separately. */ > -if ( *reps != 1 ) > -return X86EMUL_UNHANDLEABLE; > +if ( reps_p ) > +{ > +/* If this is a string operation, emulate each iteration separately. > */ > +if ( reps != 1 ) > +return
[Xen-devel] Ping²: [PATCH v2] timers: limit heap size
On 05.07.2019 18:06, Jan Beulich wrote: On 05.06.19 at 08:51, wrote: >> First and foremost make timer_softirq_action() avoid growing the heap >> if its new size can't be stored without truncation. 64k entries is a >> lot, and I don't think we're at risk of actually running into the issue, >> but I also think it's better not to allow for hard to debug problems to >> occur in the first place. >> >> Furthermore also adjust the code such the size/limit fields becoming >> unsigned int would at least work from a mere sizing point of view. For >> this also switch various uses of plain int to unsigned int. >> >> Signed-off-by: Jan Beulich >> --- >> v2: Log (once) when heap limit would have been exceeded. >> >> --- a/xen/common/timer.c >> +++ b/xen/common/timer.c >> @@ -63,9 +63,9 @@ static struct heap_metadata *heap_metada >> } >> >> /* Sink down element @pos of @heap. */ >> -static void down_heap(struct timer **heap, int pos) >> +static void down_heap(struct timer **heap, unsigned int pos) >> { >> -int sz = heap_metadata(heap)->size, nxt; >> +unsigned int sz = heap_metadata(heap)->size, nxt; >> struct timer *t = heap[pos]; >> >> while ( (nxt = (pos << 1)) <= sz ) >> @@ -84,7 +84,7 @@ static void down_heap(struct timer **hea >> } >> >> /* Float element @pos up @heap. */ >> -static void up_heap(struct timer **heap, int pos) >> +static void up_heap(struct timer **heap, unsigned int pos) >> { >> struct timer *t = heap[pos]; >> >> @@ -103,8 +103,8 @@ static void up_heap(struct timer **heap, >> /* Delete @t from @heap. Return TRUE if new top of heap. */ >> static int remove_from_heap(struct timer **heap, struct timer *t) >> { >> -int sz = heap_metadata(heap)->size; >> -int pos = t->heap_offset; >> +unsigned int sz = heap_metadata(heap)->size; >> +unsigned int pos = t->heap_offset; >> >> if ( unlikely(pos == sz) ) >> { >> @@ -130,7 +130,7 @@ static int remove_from_heap(struct timer >> /* Add new entry @t to @heap. Return TRUE if new top of heap. */ >> static int add_to_heap(struct timer **heap, struct timer *t) >> { >> -int sz = heap_metadata(heap)->size; >> +unsigned int sz = heap_metadata(heap)->size; >> >> /* Fail if the heap is full. */ >> if ( unlikely(sz == heap_metadata(heap)->limit) ) >> @@ -463,9 +463,17 @@ static void timer_softirq_action(void) >> if ( unlikely(ts->list != NULL) ) >> { >> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */ >> -int old_limit = heap_metadata(heap)->limit; >> -int new_limit = ((old_limit + 1) << 4) - 1; >> -struct timer **newheap = xmalloc_array(struct timer *, new_limit + >> 1); >> +unsigned int old_limit = heap_metadata(heap)->limit; >> +unsigned int new_limit = ((old_limit + 1) << 4) - 1; >> +struct timer **newheap = NULL; >> + >> +/* Don't grow the heap beyond what is representable in its >> metadata. */ >> +if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit && >> + new_limit + 1 ) >> +newheap = xmalloc_array(struct timer *, new_limit + 1); >> +else >> +printk_once(XENLOG_WARNING "CPU%u: timer heap limit reached\n", >> +smp_processor_id()); >> if ( newheap != NULL ) >> { >> spin_lock_irq(>lock); >> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke >> struct timers *ts; >> unsigned long flags; >> s_time_t now = NOW(); >> -inti, j; >> +unsigned int i, j; >> >> printk("Dumping timer queues:\n"); >> >> @@ -556,7 +561,7 @@ static void dump_timerq(unsigned char ke >> spin_lock_irqsave(>lock, flags); >> for ( j = 1; j <= heap_metadata(ts->heap)->size; j++ ) >> dump_timer(ts->heap[j], now); >> -for ( t = ts->list, j = 0; t != NULL; t = t->list_next, j++ ) >> +for ( t = ts->list; t != NULL; t = t->list_next ) >> dump_timer(t, now); >> spin_unlock_irqrestore(>lock, flags); >> } >> >> >> >> > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ping: [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter optional
On 01.07.2019 13:59, Jan Beulich wrote: > A majority of callers wants just a single iteration handled. Allow to > express this by passing in a NULL pointer, instead of setting up a local > variable just to hold the "1" to pass in here. > > Signed-off-by: Jan Beulich > --- > Note that this conflicts with additions/changes made by "x86emul: > further work". Whatever goes in later will need re-basing. > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -788,14 +788,14 @@ static int hvmemul_virtual_to_linear( >enum x86_segment seg, >unsigned long offset, >unsigned int bytes_per_rep, > -unsigned long *reps, > +unsigned long *reps_p, >enum hvm_access_type access_type, >struct hvm_emulate_ctxt *hvmemul_ctxt, >unsigned long *linear) >{ >struct segment_register *reg; >int okay; > -unsigned long max_reps = 4096; > +unsigned long reps = 1; > >if ( seg == x86_seg_none ) >{ > @@ -803,62 +803,72 @@ static int hvmemul_virtual_to_linear( >return X86EMUL_OKAY; >} > > -/* > - * If introspection has been enabled for this domain, and we're emulating > - * becase a vm_reply asked us to (i.e. not doing regular IO) reps should > - * be at most 1, since optimization might otherwise cause a single > - * vm_event being triggered for repeated writes to a whole page. > - */ > -if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) && > - current->arch.vm_event->emulate_flags != 0 ) > - max_reps = 1; > +if ( reps_p ) > +{ > +unsigned long max_reps = 4096; > > -/* > - * Clip repetitions to avoid overflow when multiplying by @bytes_per_rep. > - * The chosen maximum is very conservative but it's what we use in > - * hvmemul_linear_to_phys() so there is no point in using a larger value. > - */ > -*reps = min_t(unsigned long, *reps, max_reps); > +/* > + * If introspection has been enabled for this domain, and we're > + * emulating because a vm_reply asked us to (i.e. not doing regular > IO) > + * reps should be at most 1, since optimization might otherwise > cause a > + * single vm_event being triggered for repeated writes to a whole > page. > + */ > +if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) && > + current->arch.vm_event->emulate_flags != 0 ) > + max_reps = 1; > + > +/* > + * Clip repetitions to avoid overflow when multiplying by > + * @bytes_per_rep. The chosen maximum is very conservative but it's > + * what we use in hvmemul_linear_to_phys() so there is no point in > + * using a larger value. > + */ > +reps = *reps_p = min_t(unsigned long, *reps_p, max_reps); > +} > >reg = hvmemul_get_seg_reg(seg, hvmemul_ctxt); >if ( IS_ERR(reg) ) >return -PTR_ERR(reg); > > -if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) ) > +if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (reps > 1) ) >{ >/* > * x86_emulate() clips the repetition count to ensure we don't wrap > * the effective-address index register. Hence this assertion > holds. > */ > -ASSERT(offset >= ((*reps - 1) * bytes_per_rep)); > +ASSERT(offset >= ((reps - 1) * bytes_per_rep)); >okay = hvm_virtual_to_linear_addr( > -seg, reg, offset - (*reps - 1) * bytes_per_rep, > -*reps * bytes_per_rep, access_type, > +seg, reg, offset - (reps - 1) * bytes_per_rep, > +reps * bytes_per_rep, access_type, >hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear); > -*linear += (*reps - 1) * bytes_per_rep; > +*linear += (reps - 1) * bytes_per_rep; >if ( hvmemul_ctxt->ctxt.addr_size != 64 ) >*linear = (uint32_t)*linear; >} >else >{ >okay = hvm_virtual_to_linear_addr( > -seg, reg, offset, *reps * bytes_per_rep, access_type, > +seg, reg, offset, reps * bytes_per_rep, access_type, >hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear); >} > >if ( okay ) >return X86EMUL_OKAY; > > -/* If this is a string operation, emulate each iteration separately. */ > -if ( *reps != 1 ) > -return X86EMUL_UNHANDLEABLE; > +if ( reps_p ) > +{ > +/* If this is a string operation, emulate each iteration separately. > */ > +if ( reps != 1 ) > +return X86EMUL_UNHANDLEABLE; > + > +*reps_p = 0; > +} > >/* > * Leave exception injection to the caller for non-user segments: We > * neither know the exact error code to be used, nor can we easily > * determine the
Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to test-and-clear _PGC_allocated
> -Original Message- > From: Jan Beulich > Sent: 10 July 2019 23:53 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Julien Grall ; > Andrew Cooper > ; Roger Pau Monne ; > Volodymyr Babchuk > ; George Dunlap ; Ian > Jackson > ; Stefano Stabellini ; Konrad > Rzeszutek Wilk > ; Tamas K Lengyel ; Tim > (Xen.org) ; Wei Liu > > Subject: Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to > test-and-clear _PGC_allocated > > On 10.07.2019 18:17, Paul Durrant wrote: > > @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server > > *s, bool buf) > > unmap_domain_page_global(iorp->va); > > iorp->va = NULL; > > > > -/* > > - * Check whether we need to clear the allocation reference before > > - * dropping the explicit references taken by get_page_and_type(). > > - */ > > -if ( test_and_clear_bit(_PGC_allocated, >count_info) ) > > -put_page(page); > > - > > +clear_assignment_reference(page); > > put_page_and_type(page); > > } > > Is there a specific reason you drop the comment? It doesn't become > less relevant than when it was added, does it? Not sure, since what's actually going on is now internal to the function. If I change the function name to clear_allocation_reference() then I think the comment probably becomes extraneous. > > > --- a/xen/include/xen/mm.h > > +++ b/xen/include/xen/mm.h > > @@ -658,4 +658,15 @@ static inline void > > share_xen_page_with_privileged_guests( > > share_xen_page_with_guest(page, dom_xen, flags); > > } > > > > +static inline void clear_assignment_reference(struct page_info *page) > > I think the function should have 'page' in it's name. Perhaps > page_deassign() / page_dealloc() are also misleading, but how > about page_put_alloc() or page_put_alloc_ref()? > Ok, I think page_put_alloc_ref() is most descriptive (particularly w.r.t. the above discussion). > > +{ > > +/* > > + * It is unsafe to clear _PGC_allocated without holding an additional > > + * reference. > > + */ > > +ASSERT((page->count_info & PGC_count_mask) > 1); > > While this isn't really in line with our goal of wanting to limit > damage also in release builds, I agree that there's no really good > alternative here. Crashing the owner of the page wouldn't help > much, and bailing from the function wouldn't necessarily be better > either. Hence I think this would better be BUG_ON(). Ok. > > > +if ( test_and_clear_bit(_PGC_allocated, >count_info) ) > > +put_page(page); > > +} > > On the whole I have to admit I'm not entirely convinced the "open- > coding" as you call it (to me it's not really open-coding as long as > there is no helper) is such a bad thing here: Without the helper it > is slightly more obvious at the use sites what's actually going on. > But maybe that's indeed just me. I still think a helper is better, but I'll add a comment to describe what it is doing. Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 138998: all pass - PUSHED
flight 138998 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/138998/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 70565e64227dfa254d8a0703dd60dc74bd8b5e6e baseline version: ovmf 55b9bbf40a1cf9788dd6a7b093851076851fc670 Last test of basis 138966 2019-07-13 22:12:19 Z1 days Testing same since 138998 2019-07-14 17:21:25 Z0 days1 attempts People who touched revisions under test: Jordan Justen 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 55b9bbf40a..70565e6422 70565e64227dfa254d8a0703dd60dc74bd8b5e6e -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel