[Xen-devel] [ovmf baseline-only test] 75573: tolerable FAIL
This run is configured for baseline tests only. flight 75573 ovmf real [real] http://osstest.xensource.com/osstest/logs/75573/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: build-amd64-libvirt 6 libvirt-buildfail like 75565 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail like 75565 build-i386-libvirt6 libvirt-buildfail like 75565 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail like 75565 version targeted for testing: ovmf e048ce883c8e9f746a655ca5a4c8c0ce34198999 baseline version: ovmf 93f98985826a6eba30584e9b2ada754b3da17990 Last test of basis75565 2018-11-02 23:52:34 Z3 days Testing same since75573 2018-11-05 11:25:33 Z0 days1 attempts People who touched revisions under test: Dong, Eric Eric Dong Ruiyu Ni jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. commit e048ce883c8e9f746a655ca5a4c8c0ce34198999 Author: Dong, Eric Date: Fri Nov 2 16:27:48 2018 +0800 UefiCpuPkg/MpInitLib: Rollback old change 2a5997f8. In some special cases, after BSP sends Init-sipi-sipi signal AP needs more time to start the Ap procedure. In this case BSP may think AP has finished its task but in fact AP hasn't began yet. Rollback former change to keep the status which only be used when AP really finished task. Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong Reviewed-by: Ruiyu Ni commit 2552661848c35ace83b843df301840b18ed7c5d5 Author: Ruiyu Ni Date: Fri Nov 2 17:21:37 2018 +0800 Maintainers.txt: Change package maintainer of UefiCpuPkg Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Reviewed-by: Eric Dong commit 23d14cae073b52873ac6fbe6acfa3ddedb4673b8 Author: Ruiyu Ni Date: Fri Nov 2 17:19:03 2018 +0800 Maintainers.txt: Change package maintainer of IntelSiliconPkg Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Reviewed-by: Jiewen Yao ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/2] memory_hotplug: Free pages as higher order
On 2018-11-06 03:14, Andrew Morton wrote: On Mon, 05 Nov 2018 15:12:27 +0530 Arun KS wrote: On 2018-10-22 16:03, Arun KS wrote: > On 2018-10-19 13:37, Michal Hocko wrote: >> On Thu 18-10-18 19:18:25, Andrew Morton wrote: >> [...] >>> So this patch needs more work, yes? >> >> Yes, I've talked to Arun (he is offline until next week) offlist and >> he >> will play with this some more. > > Converted totalhigh_pages, totalram_pages and zone->managed_page to > atomic and tested hot add. Latency is not effected with this change. > Will send out a separate patch on top of this one. Hello Andrew/Michal, Will this be going in subsequent -rcs? I thought were awaiting a new version? "Will send out a separate patch on top of this one"? Sorry for confusion. I sent out an incremental patch converting counters to atomics. https://patchwork.kernel.org/cover/10657217/ I do think a resend would be useful, please. Ensure the changelog is updated to capture the above info and any other worthy issues which arose during review. Will do that. Regards, Arun ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.14 test] 129438: regressions - FAIL
flight 129438 linux-4.14 real [real] http://logs.test-lab.xenproject.org/osstest/logs/129438/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-examine 4 memdisk-try-append fail REGR. vs. 128921 Tests which are failing intermittently (not blocking): test-amd64-amd64-libvirt 7 xen-boot fail in 129413 pass in 129438 test-amd64-amd64-xl-pvhv2-intel 7 xen-bootfail pass in 129413 test-amd64-i386-qemut-rhel6hvm-intel 12 guest-start/redhat.repeat fail pass in 129413 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail in 129413 like 128921 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-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-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 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-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-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-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: linux50961e4888a1d53544ac4ea6f185fc27ee4fee4f baseline version: linuxe7405910ca5553eae8744af4e5c
[Xen-devel] [linux-linus bisection] complete test-amd64-amd64-xl-shadow
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-xl-shadow 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: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Bug introduced: 71e56028173bc84f01456a5679d8be9d681b49f1 Bug not present: c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/129484/ (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-shadow.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-shadow.xen-boot --summary-out=tmp/129484.bisection-summary --basis-template=125898 --blessings=real,real-bisect linux-linus test-amd64-amd64-xl-shadow xen-boot Searching for failure / basis pass: 129417 fail [host=baroque0] / 128945 [host=huxelrebe0] 128920 [host=debina1] 128885 [host=huxelrebe1] 128861 [host=pinot1] 128835 [host=debina0] 128727 [host=albana1] 128663 [host=albana0] 128599 [host=rimava1] 128520 [host=huxelrebe0] 128493 [host=baroque1] 128476 [host=italia0] 128461 ok. Failure / basis pass flights: 129417 / 128461 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest 71e56028173bc84f01456a5679d8be9d681b49f1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 de5b678ca4dcdfa83e322491d478d66df56c1986 2cf113891a38cc05434bc9876ffc107a990887be Basis pass c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 359970fd8b781fac2ddcbc84dd5b890075fa08ef Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1-71e56028173bc84f01456a5679d8be9d681b49f1 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#de5b678ca4dcdfa83e322491d478d66df56c1986-de5b678ca4dcdfa83e322491d478d66df56c1986 git://xenbits.xen.org/xen.git#359970fd8b781fac2ddcbc84dd5b890075fa08ef-2cf113891a38cc05434bc9876ffc107a990887be adhoc-revtuple-generator: tree discontiguous: linux-2.6 Loaded 2005 nodes in revision graph Searching for test results: 128369 [host=godello0] 128407 [host=fiano0] 128438 [host=huxelrebe1] 128476 [host=italia0] 128461 pass c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 359970fd8b781fac2ddcbc84dd5b890075fa08ef 128493 [host=baroque1] 128520 [host=huxelrebe0] 128599 [host=rimava1] 128663 [host=albana0] 128727 [host=albana1] 128861 [host=pinot1] 128835 [host=debina0] 128885 [host=huxelrebe1] 128920 [host=debina1] 128945 [host=huxelrebe0] 128970 fail irrelevant 129005 fail irrelevant 129072 fail irrelevant 129167 fail irrelevant 129258 fail irrelevant 129304 fail irrelevant 129389 fail irrelevant 129348 fail irrelevant 129443 pass c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 2916951c1bb943e79bf965cde66a78b0e841455b 129437 pass c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 359970fd8b781fac2ddcbc84dd5b890075fa08ef 129441 fail irrelevant 129417 fail 71e56028173bc84f01456a5679d8be9d681b49f1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 de5b678ca4dcdfa83e322491d478d66df56c1986 2cf113891a38cc05434bc9876ffc107a990887be 129444 pass c1d84a1b42ef70d8ae601df9cadedc7ed4f1beb1 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 57077cc42ea03a788f03cb01dcf1cee491d80992 129480 pass c1d84a1b42ef70d8ae
[Xen-devel] [libvirt test] 129434: regressions - FAIL
flight 129434 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/129434/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 129353 build-i386-pvops 6 kernel-build fail REGR. vs. 129353 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 129353 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 129353 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 7a10a6a598ab4e8599c867060a7232a06c663e51 baseline version: libvirt 48080527d6e364f213affd8517bb99a665d38440 Last test of basis 129353 2018-11-03 04:18:56 Z2 days Testing same since 129434 2018-11-05 04:19:08 Z0 days1 attempts People who touched revisions under test: Daniel Veillard jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops fail test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. comm
Re: [Xen-devel] [RFC 15/16] xen/arm: Implement Set/Way operations
Hi Stefano, On 11/5/18 9:10 PM, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: Set/Way operations are used to perform maintenance on a given cache. At the moment, Set/Way operations are not trapped and therefore a guest OS will directly act on the local cache. However, a vCPU may migrate to another pCPU in the middle of the processor. This will result to have cache with stall data (Set/Way are not propagated) potentially causing crash. This may be the cause of heisenbug noticed in Osstest [1]. Furthermore, Set/Way operations are not available on system cache. This means that OS, such as Linux 32-bit, relying on those operations to fully clean the cache before disabling MMU may break because data may sits in system caches and not in RAM. For more details about Set/Way, see the talk "The Art of Virtualizing Cache Maintenance" given at Xen Summit 2018 [2]. In the context of Xen, we need to trap Set/Way operations and emulate them. From the Arm Arm (B1.14.4 in DDI 046C.c), Set/Way operations are difficult to virtualized. So we can assume that a guest OS using them will suffer the consequence (i.e slowness) until developer removes all the usage of Set/Way. As the software is not allowed to infer the Set/Way to Physical Address mapping, Xen will need to go through the guest P2M and clean & invalidate all the entries mapped. Because Set/Way happen in batch (a loop on all Set/Way of a cache), Xen would need to go through the P2M for every instructions. This is quite expensive and would severely impact the guest OS. The implementation is re-using the KVM policy to limit the number of flush: - If we trap a Set/Way operations, we enable VM trapping (i.e ^ remove 'a' HVC_EL2.TVM) to detect cache being turned on/off, and do a full clean. "do a full clean" straight away, right? May I suggest a rewording of this item: - as soon as we trap a set/way operation, we enable VM trapping (i.e. HVC_EL2.TVM, it ll allow us to detect cache being turned on/off), then we do a full clean Sure. - We clean the caches when turning on and off "We clean the caches when the guest turns caches on or off" Sure. - Once the caches are enabled, we stop trapping VM instructions [1] https://lists.xenproject.org/archives/html/xen-devel/2017-09/msg03191.html [2] https://fr.slideshare.net/xen_com_mgr/virtualizing-cache Signed-off-by: Julien Grall --- xen/arch/arm/arm64/vsysreg.c | 27 +- xen/arch/arm/p2m.c | 68 xen/arch/arm/traps.c | 2 +- xen/arch/arm/vcpreg.c| 23 +++ xen/include/asm-arm/p2m.h| 16 +++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c index 1517879697..43c6c3e30d 100644 --- a/xen/arch/arm/arm64/vsysreg.c +++ b/xen/arch/arm/arm64/vsysreg.c @@ -40,7 +40,20 @@ static bool vreg_emulate_##reg(struct cpu_user_regs *regs, \ } /* Defining helpers for emulating sysreg registers. */ -TVM_REG(SCTLR_EL1) +static bool vreg_emulate_SCTLR_EL1(struct cpu_user_regs *regs, uint64_t *r, + bool read) +{ +struct vcpu *v = current; +bool cache_enabled = vcpu_has_cache_enabled(v); + +GUEST_BUG_ON(read); +WRITE_SYSREG(*r, SCTLR_EL1); + +p2m_toggle_cache(v, cache_enabled); + +return true; +} + TVM_REG(TTBR0_EL1) TVM_REG(TTBR1_EL1) TVM_REG(TCR_EL1) @@ -84,6 +97,18 @@ void do_sysreg(struct cpu_user_regs *regs, break; /* + * HCR_EL2.TSW + * + * ARMv8 (DDI 0487B.b): Table D1-42 + */ +case HSR_SYSREG_DCISW: +case HSR_SYSREG_DCCSW: +case HSR_SYSREG_DCCISW: +if ( hsr.sysreg.read ) Shouldn't it be !hsr.sysreg.read ? Hmmm yes. +p2m_set_way_flush(current); +break; + +/* * HCR_EL2.TVM * * ARMv8 (DDI 0487B.b): Table D1-37 diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index df6b48d73b..a3d4c563b1 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1564,6 +1564,74 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) return 0; } +static void p2m_flush_vm(struct vcpu *v) +{ +struct p2m_domain *p2m = p2m_get_hostp2m(v->domain); + +/* XXX: Handle preemption */ Yes, good to have this reminder. Maybe add "we'd want to break the operation down when it takes too long". I am planning to handle this before the series is actually merged. This is a massive security hole and easy to exploit. +p2m_cache_flush_range(v->domain, p2m->lowest_mapped_gfn, + p2m->max_mapped_gfn); +} + +/* + * See note at ARMv7 ARM B1.14.4 (DDI 0406C.c) (TL;DR: S/W ops are not + * easily virtualized). + * + * Main problems: + * - S/W ops are local to a CPU (not broadcast) + * - We have line migration behind our back (speculation
Re: [Xen-devel] [RFC 16/16] xen/arm: Track page accessed between batch of Set/Way operations
Hi Stefano, On 11/5/18 9:35 PM, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: At the moment, the implementation of Set/Way operations will go through all the entries of the guest P2M and flush them. However, this is very expensive and may render unusable a guest OS using them. For instance, Linux 32-bit will use Set/Way operations during secondary CPU bring-up. As the implementation is really expensive, it may be possible to hit the CPU bring-up timeout. To limit the Set/Way impact, we track what pages has been of the guest has been accessed between batch of Set/Way operations. This is done using bit[0] (aka valid bit) of the P2M entry. This is going to improve performance of ill-mannered guests at the cost of hurting performance of well-mannered guests. Is it really a good trade-off? Should this behavior at least be configurable with a Xen command line? Well, we have the choice between not been able to boot Linux 32-bit anymore or have a slight impact at the boot time for all guests. As you may have noticed the command line is been suggested below. I didn't yet implemented as we agreed at Connect it would be good to start getting feedback on it. This patch adds a new per-arch helper is introduced to perform actions just before the guest is first unpaused. This will be used to invalidate the P2M to track access from the start of the guest. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu --- xen/arch/arm/domain.c | 14 ++ xen/arch/arm/domain_build.c | 7 +++ xen/arch/arm/p2m.c | 32 +++- xen/arch/x86/domain.c | 4 xen/common/domain.c | 5 - xen/include/asm-arm/p2m.h | 2 ++ xen/include/xen/domain.h| 2 ++ 7 files changed, 64 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index feebbf5a92..f439f4657a 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -738,6 +738,20 @@ int arch_domain_soft_reset(struct domain *d) return -ENOSYS; } +void arch_domain_creation_finished(struct domain *d) +{ +/* + * To avoid flushing the whole guest RAM on the first Set/Way, we + * invalidate the P2M to track what has been accessed. + * + * This is only turned when IOMMU is not used or the page-table are + * not shared because bit[0] (e.g valid bit) unset will result + * IOMMU fault that could be not fixed-up. + */ +if ( !iommu_use_hap_pt(d) ) +p2m_invalidate_root(p2m_get_hostp2m(d)); +} + static int is_guest_pv32_psr(uint32_t psr) { switch (psr & PSR_MODE_MASK) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index f552154e93..de96516faa 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2249,6 +2249,13 @@ int __init construct_dom0(struct domain *d) v->is_initialised = 1; clear_bit(_VPF_down, &v->pause_flags); +/* + * XXX: We probably want a command line option to invalidate or not + * the P2M. This is because invalidating the P2M will not work with + * IOMMU, however if this is not done there will be an impact. Why can't we check on iommu_use_hap_pt(d) like in arch_domain_creation_finished? In any case, I agree it is a good idea to introduce a command line parameter to toggle the p2m_invalidate_root call at domain creation on/off. There are cases where it should be off even if an IOMMU is present. I actually forgot to remove that code as Dom0 should be covered by the change below. I am not entirely to understand your last sentence, this feature is turned off when an IOMMU is present. So what is your use case here? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 10/16] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM
Hi Stefano, On 11/5/18 7:47 PM, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: A follow-up patch will require to emulate some accesses to some co-processors registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes to the virtual memory control registers will be trapped to the hypervisor. This patch adds the infrastructure to passthrough the access to host registers. For convenience a bunch of helpers have been added to generate the different helpers. Note that HCR_EL2.TVM will be set in a follow-up patch dynamically. Signed-off-by: Julien Grall --- xen/arch/arm/vcpreg.c| 144 +++ xen/include/asm-arm/cpregs.h | 1 + 2 files changed, 145 insertions(+) diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index b04d996fd3..49529b97cd 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -24,6 +24,122 @@ #include #include +/* + * Macros to help generating helpers for registers trapped when + * HCR_EL2.TVM is set. + * + * Note that it only traps NS write access from EL1. + * + * - TVM_REG() should not be used outside of the macros. It is there to + *help defining TVM_REG32() and TVM_REG64() + * - TVM_REG32(regname, xreg) and TVM_REG64(regname, xreg) are used to + *resp. generate helper accessing 32-bit and 64-bit register. "regname" + *been the Arm32 name and "xreg" the Arm64 name. ^ is Please add that we use the Arm64 reg name to call WRITE_SYSREG in the Xen source code even on Arm32 in general I am not sure to understand this. It is common use in Xen to use arm64 name when code is for both architecture. So why would I need a specific comment here? + * - UPDATE_REG32_COMBINED(lowreg, hireg, xreg) are used to generate a TVM_REG32_COMBINED + * pair of registers share the same Arm32 registers. "lowreg" and + * "higreg" been resp. the Arm32 name and "xreg" the Arm64 name. "lowreg" + * will use xreg[31:0] and "hireg" will use xreg[63:32]. Please add that xreg is unused in the Arm32 case. Why do you think that? xreg is actually used. It will get expanded to whatever is the co-processor encoding and caught by reg... in TVM_REG(). + */ + +/* The name is passed from the upper macro to workaround macro expansion. */ +#define TVM_REG(sz, func, reg...) \ +static bool func(struct cpu_user_regs *regs, uint##sz##_t *r, bool read)\ +{ \ +GUEST_BUG_ON(read); \ +WRITE_SYSREG##sz(*r, reg); \ +\ +return true;\ +} + +#define TVM_REG32(regname, xreg) TVM_REG(32, vreg_emulate_##regname, xreg) +#define TVM_REG64(regname, xreg) TVM_REG(64, vreg_emulate_##regname, xreg) + +#ifdef CONFIG_ARM_32 +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ +/* Use TVM_REG directly to workaround macro expansion. */ \ +TVM_REG(32, vreg_emulate_##lowreg, lowreg) \ +TVM_REG(32, vreg_emulate_##hireg, hireg) + +#else /* CONFIG_ARM_64 */ +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ +static bool vreg_emulate_##xreg(struct cpu_user_regs *regs, uint32_t *r,\ +bool read, bool hi) \ +{ \ +register_t reg = READ_SYSREG(xreg); \ +\ +GUEST_BUG_ON(read); \ +if ( hi ) /* reg[63:32] is AArch32 register hireg */\ +{ \ +reg &= GENMASK(31, 0); \ Move GENMASK before the if? It's the same regardless Actually, the second GENMASK is incorrect. It should have been GENMASK(63, 32) as we want to update only the lowreg. So I will fix the mask instead. /* + * HCR_EL2.TVM + * + * ARMv8 (DDI 0487B.b): Table D1-37 In 0487D.a is D1-99 I haven't had the chance to download the latest spec (it was released last week). I will update to the new spec. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 129473: tolerable all pass - PUSHED
flight 129473 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/129473/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass version targeted for testing: xen ec651bd24603aacd4843008bd6f2e395ce92adae baseline version: xen f8810d333e2aa73920e144ae2ddec2ce1ef59af8 Last test of basis 129467 2018-11-05 17:01:00 Z0 days Testing same since 129473 2018-11-05 20:00:57 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Julien Grall Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git f8810d333e..ec651bd246 ec651bd24603aacd4843008bd6f2e395ce92adae -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.19 test] 129428: regressions - FAIL
flight 129428 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/129428/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 129313 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 129313 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 129313 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 129313 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 129313 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 129313 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 129313 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 129313 Tests which are failing intermittently (not blocking): test-amd64-i386-examine 8 reboot fail pass in 129412 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail never pass test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail never pass test-amd64-i386-freebsd10-amd64 11 guest-start fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-freebsd10-i386 11 guest-start fail never pass test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail never pass test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail never pass 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 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 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-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-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail 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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail ne
Re: [Xen-devel] [PATCH v5 1/2] memory_hotplug: Free pages as higher order
On Mon, 05 Nov 2018 15:12:27 +0530 Arun KS wrote: > On 2018-10-22 16:03, Arun KS wrote: > > On 2018-10-19 13:37, Michal Hocko wrote: > >> On Thu 18-10-18 19:18:25, Andrew Morton wrote: > >> [...] > >>> So this patch needs more work, yes? > >> > >> Yes, I've talked to Arun (he is offline until next week) offlist and > >> he > >> will play with this some more. > > > > Converted totalhigh_pages, totalram_pages and zone->managed_page to > > atomic and tested hot add. Latency is not effected with this change. > > Will send out a separate patch on top of this one. > Hello Andrew/Michal, > > Will this be going in subsequent -rcs? I thought were awaiting a new version? "Will send out a separate patch on top of this one"? I do think a resend would be useful, please. Ensure the changelog is updated to capture the above info and any other worthy issues which arose during review. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 16/16] xen/arm: Track page accessed between batch of Set/Way operations
On Mon, 8 Oct 2018, Julien Grall wrote: > At the moment, the implementation of Set/Way operations will go through > all the entries of the guest P2M and flush them. However, this is very > expensive and may render unusable a guest OS using them. > > For instance, Linux 32-bit will use Set/Way operations during secondary > CPU bring-up. As the implementation is really expensive, it may be possible > to hit the CPU bring-up timeout. > > To limit the Set/Way impact, we track what pages has been of the guest > has been accessed between batch of Set/Way operations. This is done > using bit[0] (aka valid bit) of the P2M entry. This is going to improve performance of ill-mannered guests at the cost of hurting performance of well-mannered guests. Is it really a good trade-off? Should this behavior at least be configurable with a Xen command line? > This patch adds a new per-arch helper is introduced to perform actions just > before the guest is first unpaused. This will be used to invalidate the > P2M to track access from the start of the guest. > > Signed-off-by: Julien Grall > > --- > > Cc: Stefano Stabellini > Cc: Julien Grall > Cc: Andrew Cooper > Cc: George Dunlap > Cc: Ian Jackson > Cc: Jan Beulich > Cc: Konrad Rzeszutek Wilk > Cc: Tim Deegan > Cc: Wei Liu > --- > xen/arch/arm/domain.c | 14 ++ > xen/arch/arm/domain_build.c | 7 +++ > xen/arch/arm/p2m.c | 32 +++- > xen/arch/x86/domain.c | 4 > xen/common/domain.c | 5 - > xen/include/asm-arm/p2m.h | 2 ++ > xen/include/xen/domain.h| 2 ++ > 7 files changed, 64 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index feebbf5a92..f439f4657a 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -738,6 +738,20 @@ int arch_domain_soft_reset(struct domain *d) > return -ENOSYS; > } > > +void arch_domain_creation_finished(struct domain *d) > +{ > +/* > + * To avoid flushing the whole guest RAM on the first Set/Way, we > + * invalidate the P2M to track what has been accessed. > + * > + * This is only turned when IOMMU is not used or the page-table are > + * not shared because bit[0] (e.g valid bit) unset will result > + * IOMMU fault that could be not fixed-up. > + */ > +if ( !iommu_use_hap_pt(d) ) > +p2m_invalidate_root(p2m_get_hostp2m(d)); > +} > + > static int is_guest_pv32_psr(uint32_t psr) > { > switch (psr & PSR_MODE_MASK) > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index f552154e93..de96516faa 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -2249,6 +2249,13 @@ int __init construct_dom0(struct domain *d) > v->is_initialised = 1; > clear_bit(_VPF_down, &v->pause_flags); > > +/* > + * XXX: We probably want a command line option to invalidate or not > + * the P2M. This is because invalidating the P2M will not work with > + * IOMMU, however if this is not done there will be an impact. Why can't we check on iommu_use_hap_pt(d) like in arch_domain_creation_finished? In any case, I agree it is a good idea to introduce a command line parameter to toggle the p2m_invalidate_root call at domain creation on/off. There are cases where it should be off even if an IOMMU is present. Aside from these two questions, the rest of the patch looks correct. > + */ > +p2m_invalidate_root(p2m_get_hostp2m(d)); > > return 0; > } > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index a3d4c563b1..8e0c31d7ac 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1079,6 +1079,22 @@ static void p2m_invalidate_table(struct p2m_domain > *p2m, mfn_t mfn) > p2m->need_flush = true; > } > > +/* > + * Invalidate all entries in the root page-tables. This is > + * useful to get fault on entry and do an action. > + */ > +void p2m_invalidate_root(struct p2m_domain *p2m) > +{ > +unsigned int i; > + > +p2m_write_lock(p2m); > + > +for ( i = 0; i < P2M_ROOT_LEVEL; i++ ) > +p2m_invalidate_table(p2m, page_to_mfn(p2m->root + i)); > + > +p2m_write_unlock(p2m); > +} > + > bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn) > { > struct p2m_domain *p2m = p2m_get_hostp2m(d); > @@ -1539,7 +1555,8 @@ int p2m_cache_flush_range(struct domain *d, gfn_t > start, gfn_t end) > > for ( ; gfn_x(start) < gfn_x(end); start = next_gfn ) > { > -mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL); > +bool valid; > +mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, &valid); > > next_gfn = gfn_next_boundary(start, order); > > @@ -1547,6 +1564,13 @@ int p2m_cache_flush_range(struct domain *d, gfn_t > start, gfn_t end) > if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) ) > continue; > > +/* > + * Page with va
[Xen-devel] [freebsd-master test] 129446: all pass - PUSHED
flight 129446 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/129446/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd d843b0f4abd4782caf9abffd5f7628b51d65d541 baseline version: freebsd 2f480ce108ecce5a6d0e78d011781a5e33bd4c67 Last test of basis 129229 2018-10-31 09:19:07 Z5 days Failing since129318 2018-11-02 09:18:52 Z3 days2 attempts Testing same since 129446 2018-11-05 09:19:04 Z0 days1 attempts People who touched revisions under test: 0mp <0...@freebsd.org> ae andrew araujo arichardson bapt brooks bwidawsk bz cem des dteske emaste erj eugen glebius hselasky imp jhb jhibbits jkim jtl kevans kib kp markj mav mckusick miwi mmacy np oshogbo rmacklem trasz tsoome tuexen vangyzen yuripv 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 2f480ce108e..d843b0f4abd d843b0f4abd4782caf9abffd5f7628b51d65d541 -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 15/16] xen/arm: Implement Set/Way operations
On Mon, 8 Oct 2018, Julien Grall wrote: > Set/Way operations are used to perform maintenance on a given cache. > At the moment, Set/Way operations are not trapped and therefore a guest > OS will directly act on the local cache. However, a vCPU may migrate to > another pCPU in the middle of the processor. This will result to have > cache with stall data (Set/Way are not propagated) potentially causing > crash. This may be the cause of heisenbug noticed in Osstest [1]. > > Furthermore, Set/Way operations are not available on system cache. This > means that OS, such as Linux 32-bit, relying on those operations to > fully clean the cache before disabling MMU may break because data may > sits in system caches and not in RAM. > > For more details about Set/Way, see the talk "The Art of Virtualizing > Cache Maintenance" given at Xen Summit 2018 [2]. > > In the context of Xen, we need to trap Set/Way operations and emulate > them. From the Arm Arm (B1.14.4 in DDI 046C.c), Set/Way operations are > difficult to virtualized. So we can assume that a guest OS using them will > suffer the consequence (i.e slowness) until developer removes all the usage > of Set/Way. > > As the software is not allowed to infer the Set/Way to Physical Address > mapping, Xen will need to go through the guest P2M and clean & > invalidate all the entries mapped. > > Because Set/Way happen in batch (a loop on all Set/Way of a cache), Xen > would need to go through the P2M for every instructions. This is quite > expensive and would severely impact the guest OS. The implementation is > re-using the KVM policy to limit the number of flush: > - If we trap a Set/Way operations, we enable VM trapping (i.e ^ remove 'a' > HVC_EL2.TVM) to detect cache being turned on/off, and do a full > clean. "do a full clean" straight away, right? May I suggest a rewording of this item: - as soon as we trap a set/way operation, we enable VM trapping (i.e. HVC_EL2.TVM, it ll allow us to detect cache being turned on/off), then we do a full clean > - We clean the caches when turning on and off "We clean the caches when the guest turns caches on or off" > - Once the caches are enabled, we stop trapping VM instructions > > [1] https://lists.xenproject.org/archives/html/xen-devel/2017-09/msg03191.html > [2] https://fr.slideshare.net/xen_com_mgr/virtualizing-cache > > Signed-off-by: Julien Grall > --- > xen/arch/arm/arm64/vsysreg.c | 27 +- > xen/arch/arm/p2m.c | 68 > > xen/arch/arm/traps.c | 2 +- > xen/arch/arm/vcpreg.c| 23 +++ > xen/include/asm-arm/p2m.h| 16 +++ > 5 files changed, 134 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c > index 1517879697..43c6c3e30d 100644 > --- a/xen/arch/arm/arm64/vsysreg.c > +++ b/xen/arch/arm/arm64/vsysreg.c > @@ -40,7 +40,20 @@ static bool vreg_emulate_##reg(struct cpu_user_regs *regs, > \ > } > > /* Defining helpers for emulating sysreg registers. */ > -TVM_REG(SCTLR_EL1) > +static bool vreg_emulate_SCTLR_EL1(struct cpu_user_regs *regs, uint64_t *r, > + bool read) > +{ > +struct vcpu *v = current; > +bool cache_enabled = vcpu_has_cache_enabled(v); > + > +GUEST_BUG_ON(read); > +WRITE_SYSREG(*r, SCTLR_EL1); > + > +p2m_toggle_cache(v, cache_enabled); > + > +return true; > +} > + > TVM_REG(TTBR0_EL1) > TVM_REG(TTBR1_EL1) > TVM_REG(TCR_EL1) > @@ -84,6 +97,18 @@ void do_sysreg(struct cpu_user_regs *regs, > break; > > /* > + * HCR_EL2.TSW > + * > + * ARMv8 (DDI 0487B.b): Table D1-42 > + */ > +case HSR_SYSREG_DCISW: > +case HSR_SYSREG_DCCSW: > +case HSR_SYSREG_DCCISW: > +if ( hsr.sysreg.read ) Shouldn't it be !hsr.sysreg.read ? > +p2m_set_way_flush(current); > +break; > + > +/* > * HCR_EL2.TVM > * > * ARMv8 (DDI 0487B.b): Table D1-37 > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index df6b48d73b..a3d4c563b1 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1564,6 +1564,74 @@ int p2m_cache_flush_range(struct domain *d, gfn_t > start, gfn_t end) > return 0; > } > > +static void p2m_flush_vm(struct vcpu *v) > +{ > +struct p2m_domain *p2m = p2m_get_hostp2m(v->domain); > + > +/* XXX: Handle preemption */ Yes, good to have this reminder. Maybe add "we'd want to break the operation down when it takes too long". > +p2m_cache_flush_range(v->domain, p2m->lowest_mapped_gfn, > + p2m->max_mapped_gfn); > +} > + > +/* > + * See note at ARMv7 ARM B1.14.4 (DDI 0406C.c) (TL;DR: S/W ops are not > + * easily virtualized). > + * > + * Main problems: > + * - S/W ops are local to a CPU (not broadcast) > + * - We have line migration behind our back (speculation) > + * - System caches
[Xen-devel] [ovmf test] 129454: all pass - PUSHED
flight 129454 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/129454/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf e40f8efb8b06e023689120452e7ed5db199363de baseline version: ovmf e048ce883c8e9f746a655ca5a4c8c0ce34198999 Last test of basis 129430 2018-11-05 02:41:29 Z0 days Testing same since 129454 2018-11-05 11:41:45 Z0 days1 attempts People who touched revisions under test: Bob Feng BobCF Fu Siyuan Jim Dailey jim.dai...@dell.com Liming Gao shenglei Shenglei Zhang Sumit Garg 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 e048ce883c..e40f8efb8b e40f8efb8b06e023689120452e7ed5db199363de -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy
> From: Sergey Dyasli > > This finally (after literally years of work!) marks the point where the > toolstack can ask the hypervisor for the current CPUID configuration of a > specific domain. > > Introduce a new flask access vector and update the default policies. > > Also extend xen-cpuid's --policy mode to be able to take a domid and dump a > specific domains CPUID and MSR policy. > > Signed-off-by: Andrew Cooper > Signed-off-by: Sergey Dyasli Acked-by: Daniel De Graaf ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/5] x86/sysctl: Implement XEN_SYSCTL_get_cpu_policy
> From: Sergey Dyasli > > Provide a SYSCTL for the toolstack to obtain complete system CPUID and MSR > policy information. > > For the flask side of things, this subop is closely related to > {phys,cputopo,numa}info, so shares the physinfo access vector. Acked-by: Daniel De Graaf ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 11/16] xen/arm: vsysreg: Add wrapper to handle sysreg access trapped by HCR_EL2.TVM
On Mon, 8 Oct 2018, Julien Grall wrote: > A follow-up patch will require to emulate some accesses to system > registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes to the > virtual memory control registers will be trapped to the hypervisor. > > This patch adds the infrastructure to passthrough the access to the host > registers. > > Note that HCR_EL2.TVM will be set in a follow-up patch dynamically. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/arm64/vsysreg.c | 57 > > 1 file changed, 57 insertions(+) > > diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c > index 6e60824572..1517879697 100644 > --- a/xen/arch/arm/arm64/vsysreg.c > +++ b/xen/arch/arm/arm64/vsysreg.c > @@ -23,6 +23,46 @@ > #include > #include > > +/* > + * Macro to help generating helpers for registers trapped when > + * HCR_EL2.TVM is set. > + * > + * Note that it only traps NS write access from EL1. > + */ > +#define TVM_REG(reg)\ > +static bool vreg_emulate_##reg(struct cpu_user_regs *regs, \ > + uint64_t *r, bool read) \ > +{ \ > +GUEST_BUG_ON(read); \ > +WRITE_SYSREG64(*r, reg);\ > +\ > +return true;\ > +} > + > +/* Defining helpers for emulating sysreg registers. */ > +TVM_REG(SCTLR_EL1) > +TVM_REG(TTBR0_EL1) > +TVM_REG(TTBR1_EL1) > +TVM_REG(TCR_EL1) > +TVM_REG(ESR_EL1) > +TVM_REG(FAR_EL1) > +TVM_REG(AFSR0_EL1) > +TVM_REG(AFSR1_EL1) > +TVM_REG(MAIR_EL1) > +TVM_REG(AMAIR_EL1) > +TVM_REG(CONTEXTIDR_EL1) > + > +/* Macro to generate easily case for co-processor emulation */ > +#define GENERATE_CASE(reg) \ > +case HSR_SYSREG_##reg: \ > +{ \ > +bool res; \ > +\ > +res = vreg_emulate_sysreg64(regs, hsr, vreg_emulate_##reg); \ > +ASSERT(res);\ > +break; \ > +} > + > void do_sysreg(struct cpu_user_regs *regs, > const union hsr hsr) > { > @@ -44,6 +84,23 @@ void do_sysreg(struct cpu_user_regs *regs, > break; > > /* > + * HCR_EL2.TVM > + * > + * ARMv8 (DDI 0487B.b): Table D1-37 You might want to provide a more up to date reference. In any case: Reviewed-by: Stefano Stabellini > + */ > +GENERATE_CASE(SCTLR_EL1) > +GENERATE_CASE(TTBR0_EL1) > +GENERATE_CASE(TTBR1_EL1) > +GENERATE_CASE(TCR_EL1) > +GENERATE_CASE(ESR_EL1) > +GENERATE_CASE(FAR_EL1) > +GENERATE_CASE(AFSR0_EL1) > +GENERATE_CASE(AFSR1_EL1) > +GENERATE_CASE(MAIR_EL1) > +GENERATE_CASE(AMAIR_EL1) > +GENERATE_CASE(CONTEXTIDR_EL1) > + > +/* > * MDCR_EL2.TDRA > * > * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57 > -- > 2.11.0 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy
On 05/11/18 17:52, Roger Pau Monné wrote: > On Mon, Nov 05, 2018 at 11:16:40AM +, Andrew Cooper wrote: >> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c >> index 95ed853..2c41031 100644 >> --- a/tools/misc/xen-cpuid.c >> +++ b/tools/misc/xen-cpuid.c >> @@ -3,6 +3,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> >> #include >> >> @@ -309,11 +311,13 @@ int main(int argc, char **argv) >> { >> enum { MODE_UNKNOWN, MODE_INFO, MODE_DETAIL, MODE_INTERPRET, >> MODE_POLICY } >> mode = MODE_UNKNOWN; >> +int domid = -1; > Would it be better to use DOMID_INVALID instead of -1? That doesn't exist anywhere consistent or useful to use in this case. (Also, confusingly, DOMID_INVALID is 0x7ff4 in the public ABI, which is different to the INVALID_DOMID of ~0/-1 used by various bits of libxc and libxl) ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] automation: build some customised configs
On Fri, Nov 02, 2018 at 07:33:28PM +, Wei Liu wrote: > Introduce a new directory to put in configs we care about. Modify > build script to build with those configs. > > While we only introduce x86 configs initially, provision for non-x86 > configs. > > Signed-off-by: Wei Liu > --- > Cc: Jan Beulich > > Jan, feel free to put configs here. > --- > automation/configs/x86/hvm_only_config | 2 ++ > automation/configs/x86/no_hvm_pv_config | 2 ++ > automation/configs/x86/pv_only_config | 2 ++ > automation/scripts/build| 15 +++ > 4 files changed, 21 insertions(+) > create mode 100644 automation/configs/x86/hvm_only_config > create mode 100644 automation/configs/x86/no_hvm_pv_config > create mode 100644 automation/configs/x86/pv_only_config > > diff --git a/automation/configs/x86/hvm_only_config > b/automation/configs/x86/hvm_only_config > new file mode 100644 > index 00..e82cc04d69 > --- /dev/null > +++ b/automation/configs/x86/hvm_only_config > @@ -0,0 +1,2 @@ > +CONFIG_HVM=y > +# CONFIG_PV is not set > diff --git a/automation/configs/x86/no_hvm_pv_config > b/automation/configs/x86/no_hvm_pv_config > new file mode 100644 > index 00..ed853cd358 > --- /dev/null > +++ b/automation/configs/x86/no_hvm_pv_config > @@ -0,0 +1,2 @@ > +# CONFIG_HVM is not set > +# CONFIG_PV is not set > diff --git a/automation/configs/x86/pv_only_config > b/automation/configs/x86/pv_only_config > new file mode 100644 > index 00..aca77b64d4 > --- /dev/null > +++ b/automation/configs/x86/pv_only_config > @@ -0,0 +1,2 @@ > +CONFIG_PV=y > +# CONFIG_HVM is not set > diff --git a/automation/scripts/build b/automation/scripts/build > index c463b060d4..0cde1c7794 100755 > --- a/automation/scripts/build > +++ b/automation/scripts/build > @@ -31,3 +31,18 @@ fi > ./configure "${cfgargs[@]}" > > make -j$(nproc) dist > + > +# Build all the configs we care about > +case ${XEN_TARGET_ARCH} in > +x86_64) arch=x86 ;; > +*) exit 0 ;; > +esac > + > +cfg_dir="automation/configs/${arch}" > +for cfg in `ls ${cfg_dir}`; do > +echo "Building $cfg" > +rm -f xen/.config > +make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} > XEN_CONFIG_EXPERT=y defconfig > +make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y > +done > + > -- > 2.11.0 > Seems very reasonable to me. Acked-by: Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 10/16] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM
On Mon, 8 Oct 2018, Julien Grall wrote: > A follow-up patch will require to emulate some accesses to some > co-processors registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes > to the virtual memory control registers will be trapped to the hypervisor. > > This patch adds the infrastructure to passthrough the access to host > registers. For convenience a bunch of helpers have been added to > generate the different helpers. > > Note that HCR_EL2.TVM will be set in a follow-up patch dynamically. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/vcpreg.c| 144 > +++ > xen/include/asm-arm/cpregs.h | 1 + > 2 files changed, 145 insertions(+) > > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index b04d996fd3..49529b97cd 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -24,6 +24,122 @@ > #include > #include > > +/* > + * Macros to help generating helpers for registers trapped when > + * HCR_EL2.TVM is set. > + * > + * Note that it only traps NS write access from EL1. > + * > + * - TVM_REG() should not be used outside of the macros. It is there to > + *help defining TVM_REG32() and TVM_REG64() > + * - TVM_REG32(regname, xreg) and TVM_REG64(regname, xreg) are used to > + *resp. generate helper accessing 32-bit and 64-bit register. "regname" > + *been the Arm32 name and "xreg" the Arm64 name. ^ is Please add that we use the Arm64 reg name to call WRITE_SYSREG in the Xen source code even on Arm32 in general > + * - UPDATE_REG32_COMBINED(lowreg, hireg, xreg) are used to generate a TVM_REG32_COMBINED > + * pair of registers share the same Arm32 registers. "lowreg" and > + * "higreg" been resp. the Arm32 name and "xreg" the Arm64 name. "lowreg" > + * will use xreg[31:0] and "hireg" will use xreg[63:32]. Please add that xreg is unused in the Arm32 case. > + */ > + > +/* The name is passed from the upper macro to workaround macro expansion. */ > +#define TVM_REG(sz, func, reg...) \ > +static bool func(struct cpu_user_regs *regs, uint##sz##_t *r, bool read)\ > +{ \ > +GUEST_BUG_ON(read); \ > +WRITE_SYSREG##sz(*r, reg); \ > +\ > +return true;\ > +} > + > +#define TVM_REG32(regname, xreg) TVM_REG(32, vreg_emulate_##regname, xreg) > +#define TVM_REG64(regname, xreg) TVM_REG(64, vreg_emulate_##regname, xreg) > + > +#ifdef CONFIG_ARM_32 > +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ > +/* Use TVM_REG directly to workaround macro expansion. */ \ > +TVM_REG(32, vreg_emulate_##lowreg, lowreg) \ > +TVM_REG(32, vreg_emulate_##hireg, hireg) > + > +#else /* CONFIG_ARM_64 */ > +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ > +static bool vreg_emulate_##xreg(struct cpu_user_regs *regs, uint32_t *r,\ > +bool read, bool hi) \ > +{ \ > +register_t reg = READ_SYSREG(xreg); \ > +\ > +GUEST_BUG_ON(read); \ > +if ( hi ) /* reg[63:32] is AArch32 register hireg */\ > +{ \ > +reg &= GENMASK(31, 0); \ Move GENMASK before the if? It's the same regardless > +reg |= ((uint64_t)*r) << 32;\ > +} \ > +else /* reg[31:0] is AArch32 register lowreg. */\ > +{ \ > +reg &= GENMASK(31, 0); \ > +reg |= *r; \ > +} \ > +WRITE_SYSREG(reg, xreg);\ > +\ > +return true;\ > +} \ > +\ > +static bool vreg_emulate_##lowreg(struct cpu_user_regs *regs, uint32_t *r, \ > +
[Xen-devel] [xen-unstable-smoke test] 129467: tolerable all pass - PUSHED
flight 129467 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/129467/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass version targeted for testing: xen f8810d333e2aa73920e144ae2ddec2ce1ef59af8 baseline version: xen 3cbecb3c6307a23d5a2227b8f48eb395131e998e Last test of basis 129451 2018-11-05 11:00:32 Z0 days Testing same since 129467 2018-11-05 17:01:00 Z0 days1 attempts People who touched revisions under test: Jan Beulich Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 3cbecb3c63..f8810d333e f8810d333e2aa73920e144ae2ddec2ce1ef59af8 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/3] tools: No longer advertise GIT_HTTP env var
In "build: add autoconf to replace custom checks in tools/check" --enable-githttp was introduced. But we missed this comment where it was advertised. Signed-off-by: Ian Jackson CC: Paul Durrant CC: Wei Liu --- config/Tools.mk.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/Tools.mk.in b/config/Tools.mk.in index d5985c0d09..98245f63c9 100644 --- a/config/Tools.mk.in +++ b/config/Tools.mk.in @@ -40,7 +40,7 @@ XEN_TOOLS_RPATH := @rpath@ # Download GIT repositories via HTTP or GIT's own protocol? # GIT's protocol is faster and more robust, when it works at all (firewalls # may block it). We make it the default, but if your GIT repository downloads -# fail or hang, please specify GIT_HTTP=y in your environment. +# fail or hang, please pass --enable-githttp to configure. GIT_HTTP?= @githttp@ # Optional components -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/3] tools: ipxe: Correct download error handling
This shell fragment lacked set -e. So, eg if the download failed a broken ipxe.tar.gz would be left behind. Signed-off-by: Ian Jackson CC: Paul Durrant CC: Wei Liu --- tools/firmware/etherboot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/firmware/etherboot/Makefile b/tools/firmware/etherboot/Makefile index 3868f876ea..fd8dfdf5a7 100644 --- a/tools/firmware/etherboot/Makefile +++ b/tools/firmware/etherboot/Makefile @@ -33,7 +33,7 @@ $(ROM): $(ROMS) $(MAKE) -C $D/src bin/$(*F).rom $T: - if ! $(FETCHER) _$T $(IPXE_TARBALL_URL); then \ + set -e; if ! $(FETCHER) _$T $(IPXE_TARBALL_URL); then \ $(GIT) clone $(IPXE_GIT_URL) $D.git; \ (cd $D.git && $(GIT) archive --format=tar --prefix=$D/ \ $(IPXE_GIT_TAG) | gzip -n >../_$T); \ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] tools: Once again honour GIT_HTTP env var
In "build: add autoconf to replace custom checks in tools/check" --enable-githttp was introduced. But that had the effect of uncondtionally setting GIT_HTTP from the configure variable. But the env var is advertised in some places as the way to specify this behaviour, and overriding it is just unfriendly. Signed-off-by: Ian Jackson CC: Paul Durrant CC: Wei Liu --- config/Tools.mk.in| 2 +- config/Toplevel.mk.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/Tools.mk.in b/config/Tools.mk.in index 1e5cc20bf7..d5985c0d09 100644 --- a/config/Tools.mk.in +++ b/config/Tools.mk.in @@ -41,7 +41,7 @@ XEN_TOOLS_RPATH := @rpath@ # GIT's protocol is faster and more robust, when it works at all (firewalls # may block it). We make it the default, but if your GIT repository downloads # fail or hang, please specify GIT_HTTP=y in your environment. -GIT_HTTP:= @githttp@ +GIT_HTTP?= @githttp@ # Optional components XENSTAT_XENTOP := @monitors@ diff --git a/config/Toplevel.mk.in b/config/Toplevel.mk.in index 1d991895ea..4ecacbb37d 100644 --- a/config/Toplevel.mk.in +++ b/config/Toplevel.mk.in @@ -1,2 +1,2 @@ SUBSYSTEMS := @SUBSYSTEMS@ -GIT_HTTP := @githttp@ +GIT_HTTP ?= @githttp@ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [OSSTEST PATCH] ts-xen-build: Force reliance on ipxe tarball
xen.git/tools/firmware/etherboot/Makefile tries to get a tarball from xen-extfiles first and if that fails, tries cloning from ipxe.org. ipxe.org is sometimes down (or half-down) and when that happens without a tarball the build breaks and is hard to fix. We would like, therefore, to ensure that the tarball is always made before the commit which refers to it is in xen.git#master. osstest has no knowledge of ipxe as a separate thing and just lets xen.git have whatever version is in Config.mk. So osstest never needs to specify particular ipxe version by hash, or the like. So simply make osstest rely on the tarball existing, by having it specify a completely invalid URL for the git clone. This will detect attempts to update IPXE_GIT_TAG without making a corresponding tarball. CC: Paul Durrant CC: Wei Liu --- ts-xen-build | 1 + 1 file changed, 1 insertion(+) diff --git a/ts-xen-build b/ts-xen-build index 48bf062f..6ddfc533 100755 --- a/ts-xen-build +++ b/ts-xen-build @@ -95,6 +95,7 @@ sub checkout () { echo >>.config debug=$debug_build echo >>.config GIT_HTTP=y echo >>.config LIBLEAFDIR_x86_64=lib + echo >>.config IPXE_GIT_URL=file:osstest/IPXE-GIT-FORBIDDEN echo >>.config KERNELS='' END (${enable_livepatch} ?
Re: [Xen-devel] dom0/pvh: Dom0 PVH with PCI passthrough support status
On Mon, Nov 05, 2018 at 04:17:56PM +0200, Alexandru Vasile wrote: > On Mon, Nov 05, 2018 at 12:57 PM, Wei Liu wrote: > > On Mon, Nov 05, 2018 at 11:58:09AM +0200, Alexandru Vasile wrote: > > > Hello, > > > (XEN) event_channel.c:319:d0v1 EVTCHNOP failure: domain 1, error -22 > > > (XEN) event_channel.c:319:d0v3 EVTCHNOP failure: domain 1, error -22 > > Do you perhaps have more than one xenstored / xenconsoled running? > > The processes listed by 'ps -aux | grep xen' immediately after dom0 boots > are oxenstored, xenconsoled and xenwatchdogd. > > I did start 'xencommons ' from my install folder due to xl showing the name > of Dom0 as '(null)' and also because of a difference in output error from xl > when creating a HVM DomU with passthrough (the output is captured in files > xl_output_clean[0] and xl_output_xencommons[1]). > > > > (XEN) Assertion 'fdom != p2m->domain' failed at p2m.c:504 > > You can work around this with: > > > > ---8<--- > > From 9437054299c1d360eb4fedd065d51965e560fc0c Mon Sep 17 00:00:00 2001 > > From: Wei Liu > > Date: Fri, 2 Nov 2018 14:55:04 + > > Subject: [PATCH] DROP DONT POST > > > > --- > > xen/arch/x86/mm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > > index f043e43ac7..9c154783e4 100644 > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -4443,7 +4443,7 @@ int arch_acquire_resource(struct domain *d, unsigned > > int type, > > switch ( type ) > > { > > -#ifdef CONFIG_HVM > > +#if 0 > > case XENMEM_resource_ioreq_server: > > { > > ioservid_t ioservid = id; > > -- 2.11.0 > > Thank you, after applying this patch dom0 no longer freezes or reboots, > therefore I was able to capture all the error messages from the 'xl create' > command [0] [1]. So the error message is: libxl: error: libxl_qmp.c:334:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 18 to pirq -1 failed: Function not implemented I haven't looked into the code, but I think QEMU is likely trying to use a PHYSDEVOP_ or some other hypercall that's not been made available to PVH. If you want to debug this you will have to figure out which hypercall is being blocked by looking into the QEMU code and finding where that error message is coming from. > Another problem that I discovered while investigating pci passthrough is > that Dom0 randomly freezes. I encountered this problem multiple times, but > on two occasions I was able to capture the output form the serial console: > the problem will reset dom0 after 5 seconds [2] or will cause the system to > never reset [3]. > > I later applied the patch from [4] as seeing this is a recurrent problem, > but I could not reproduce the bug even when stressing the system. This is currently being looked into by Intel, and seems to be some kind of hardware bug/errata. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 1/6] docs/qemu-deprivilege: Revise and update with status and future plans
On 11/05/2018 06:07 PM, George Dunlap wrote: > docs/qemu-deprivilege.txt had some basic instructions for using > dm_restrict, but it was incomplete, misleading, and stale. > > Update the docs in a number of ways. > > First, separate user-facing documentation and technical description > into docs/features and docs/design, respectively. > > In the feature doc: > > * Introduce a section mentioning minimim versions of Linux, Xen, and > qemu required (TBD) > > * Fix the discussion of qemu userid. Mention xen-qemuuser-range-base, > and provide example shell code that actually has some hope of working > (instead of failing out after creating 900 userids). > > * Describe how to enable restrictions, as well as features which > probably don't or definitely don't work. > > In the design doc, introduce a "Technical Details" section which > describes specifically what restrictions are currently done, and also > what restrictions we are looking at doing in the future. > > The idea here is that as we implement the various items for the > future, we move them from "Restrictions still to do" to "Restrictions > done". This can also act as a design document -- a place for public > discussion of what can or should be done and how. > > Also add an entry to SUPPORT.md. Sorry, I thought I'd removed this -- the SUPPORT.md changes have been moved to another file. I can fix this up on check-in if necessary. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 4/6] tools/dm_restrict: Unshare mount and IPC namespaces on Linux
QEMU running under Xen doesn't need mount or IPC functionality. Create and enter separate namespaces for each of these before executing QEMU, so that in the event that other restrictions fail, the process won't be able to even name system mount points or exsting non-file-based IPC descriptors to attempt to attack them. Unsharing is something a process can only do to itself (it would seem); so add an os-specific "dm_preexec_restrict()" hook just before we exec() the device model. Also add checks to depriv-process-checker.sh to verify that dm is running in a new namespace (or at least, a different one than the caller). Suggested-by: Ross Lagerwall Signed-off-by: George Dunlap Acked-by: Ian Jackson --- Changes since v3: - Fix some more style issues Changes since v2: - Return an error rather than calling exit() - Use LOGE() and print to the current stderr fd, rather than printing to the new stderr fd via write() - Use r for external return values rather than rc. CC: Ian Jackson CC: Wei Liu CC: Anthony Perard --- docs/designs/qemu-deprivilege.md | 12 ++-- tools/libxl/libxl_dm.c | 5 + tools/libxl/libxl_freebsd.c | 5 + tools/libxl/libxl_internal.h | 5 + tools/libxl/libxl_linux.c| 14 ++ tools/libxl/libxl_netbsd.c | 5 + 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md index 039540..a461ebbadd 100644 --- a/docs/designs/qemu-deprivilege.md +++ b/docs/designs/qemu-deprivilege.md @@ -78,12 +78,6 @@ Then adds the following to the qemu command-line: '''Tested''': Not tested -## Restrictions / improvements still to do - -This lists potential restrictions still to do. It is meant to be -listed in order of ease of implementation, with low-hanging fruit -first. - ## Namespaces for unused functionality (Linux only) '''Description''': QEMU doesn't use the functionality associated with @@ -111,6 +105,12 @@ call: [qemu-namespaces]: https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg04723.html +# Restrictions / improvements still to do + +This lists potential restrictions still to do. It is meant to be +listed in order of ease of implementation, with low-hanging fruit +first. + ### Basic RLIMITs '''Description''': A number of limits on the resources that a given diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index ad3efcc783..278cfd6e6e 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -2393,6 +2393,11 @@ retry_transaction: goto out_close; if (!rc) { /* inner child */ setsid(); +if (libxl_defbool_val(b_info->dm_restrict)) { +rc = libxl__local_dm_preexec_restrict(gc); +if (rc) +_exit(-1); +} libxl__exec(gc, null, logfile_w, logfile_w, dm, args, envs); } diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c index 6442ccec72..f7ef4a8910 100644 --- a/tools/libxl/libxl_freebsd.c +++ b/tools/libxl/libxl_freebsd.c @@ -245,3 +245,8 @@ int libxl__pci_topology_init(libxl__gc *gc, { return ERROR_NI; } + +int libxl__local_dm_preexec_restrict(libxl__gc *gc) +{ +return 0; +} diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index ff889385fe..e498435e16 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -3774,6 +3774,11 @@ struct libxl__dm_spawn_state { _hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*); +/* + * Called after forking but before executing the local devicemodel. + */ +_hidden int libxl__local_dm_preexec_restrict(libxl__gc *gc); + /* Stubdom device models. */ typedef struct { diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index 6ef0abc693..c7a345f4bb 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -307,6 +307,20 @@ int libxl__pci_topology_init(libxl__gc *gc, return err; } +int libxl__local_dm_preexec_restrict(libxl__gc *gc) +{ +int r; + +/* Unshare mount and IPC namespaces. These are unused by QEMU. */ +r = unshare(CLONE_NEWNS | CLONE_NEWIPC); +if (r) { +LOGE(ERROR, "libxl: Mount and IPC namespace unfailed"); +return ERROR_FAIL; +} + +return 0; +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c index 2edfb00641..dce3f1fdce 100644 --- a/tools/libxl/libxl_netbsd.c +++ b/tools/libxl/libxl_netbsd.c @@ -124,3 +124,8 @@ int libxl__pci_topology_init(libxl__gc *gc, { return ERROR_NI; } + +void libxl__local_dm_preexec_restrict(libxl__gc *gc, int stderrfd) +{ +return; +} -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 1/6] docs/qemu-deprivilege: Revise and update with status and future plans
docs/qemu-deprivilege.txt had some basic instructions for using dm_restrict, but it was incomplete, misleading, and stale. Update the docs in a number of ways. First, separate user-facing documentation and technical description into docs/features and docs/design, respectively. In the feature doc: * Introduce a section mentioning minimim versions of Linux, Xen, and qemu required (TBD) * Fix the discussion of qemu userid. Mention xen-qemuuser-range-base, and provide example shell code that actually has some hope of working (instead of failing out after creating 900 userids). * Describe how to enable restrictions, as well as features which probably don't or definitely don't work. In the design doc, introduce a "Technical Details" section which describes specifically what restrictions are currently done, and also what restrictions we are looking at doing in the future. The idea here is that as we implement the various items for the future, we move them from "Restrictions still to do" to "Restrictions done". This can also act as a design document -- a place for public discussion of what can or should be done and how. Also add an entry to SUPPORT.md. Signed-off-by: George Dunlap --- Changes since v3: - Fix typo (32->16) - Use an example value not close to the `nobody` uids, but still a multiple of 2^16. - Mention that using a multiple of 2^16 may have advantages. - Have the example create a group as well - Reorganize two comments on the "range-base" method for clarity Changes since v2: - Extraneous privcmd / evtchn instances aren't closed - Expand description of how to test fd deprivileging - Rework and clarify two namespace sections, give reference for QEMU NAK - Add more information about migration technical challenges - In UID section, mention possibility of container ID collisions. - Fix name of design document. - Add SUPPORT.md statement. Specify Linux, to make sure that FreeBSD is evaluated separately. - Mention that `-sandbox` is a blacklist and why Changes since v1: - Break into two, and move into appropriate directories (rather than 'misc') - Updated version requirements - Distinguish between features which "don't yet work" and features which we never expect to work - Update description of xen-restrict functionality - Reorder and expand further restrictions - Make it more clear which restrictions are available on Linux only - Include detailed description of how to kill a process - Add RLIMIT_NPROC as something we can do without further changes to qemu - Document the need to check for the sandbox feature before using it Thank you to Ross Lagerwall, whose description of what XenServer is doing formed much of the basis for the text here. CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan CC: Konrad Wilk CC: Stefano Stabellini CC: Julien Grall CC: Anthony Perard CC: Ross Lagerwall --- docs/designs/qemu-deprivilege.md | 322 ++ docs/features/qemu-deprivilege.pandoc | 101 docs/misc/qemu-deprivilege.txt| 36 --- 3 files changed, 423 insertions(+), 36 deletions(-) create mode 100644 docs/designs/qemu-deprivilege.md create mode 100644 docs/features/qemu-deprivilege.pandoc delete mode 100644 docs/misc/qemu-deprivilege.txt diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md new file mode 100644 index 00..787ae1ac7c --- /dev/null +++ b/docs/designs/qemu-deprivilege.md @@ -0,0 +1,322 @@ +# Introduction + +The goal of deprilvileging qemu is this: Even if there is a bug (for +example in qemu) which permits a domain to gain control of the device +model, the compromised device model process is prevented from +violating the system's overall security properties. Ie, a guest +cannot "escape" from the virtualisation by using a qemu bug. + +This document lists the various technical measures which we either +have taken, or plan to take to effect this goal. Some of them are +required to be considered secure (that is, there are known attack +vectors which they close); others are "just in case" (that is, there +are no known attack vectors, but we perform the restrictions to reduce +the possibility of unknown attack vectors). + +# Restrictions done + +The following restrictions are currently implemented. + +## Having qemu switch user + +'''Description''': As mentioned above, having QEMU switch to a +non-root user, one per domain id. Not being the root user limits what +a compromised QEMU process can do to the system, and having one user +per domain id limits what a comprimised QEMU process can do to the +QEMU processes of other VMs. + +'''Implementation''': The toolstack adds the following to the qemu command-line: + +-runas : + +'''How to test''': + +grep /proc//status [UG]id + +'''Testing Status''': Not tested + +## Xen library / file-descriptor restrictions + +'''Description''': Close and restrict Xen-related file descriptors. +Specifically: + * Close all xenstore-rela
[Xen-devel] [PATCH v4 5/6] tools/dm_depriv: Add first cut RLIMITs
Limit the ability of a potentially compromised QEMU to consume system resources. Key limits: - RLIMIT_FSIZE (file size): 256KiB - RLIMIT_NPROC (after uid changes to a unique uid) Probably unnecessary limits but why not: - RLIMIT_CORE: 0 - RLIMIT_MSGQUEUE: 0 - RLIMIT_LOCKS: 0 - RLIMIT_MEMLOCK: 0 NB that we do not yet set RLIMIT_AS (total virtual memory) or RLIMIT_NOFILES (number of open files), since these require more care and/or more coordination with QEMU to implement. Suggested-by: Ross Lagerwall Signed-off-by: George Dunlap --- Changes since v3: - Align RLIMIT_ENTRY list for easier reading - Fix wrong format string specifier - Get rid of some trailing whitespace Changes since v2: - Use a macro to define rlimit entries - Use RLIMIT_NLIMITS as an end-of-list marker, rather than -1 - Various style clean-ups CC: Ian Jackson CC: Wei Liu CC: Anthony Perard --- docs/designs/qemu-deprivilege.md | 12 - tools/libxl/libxl_linux.c| 42 ++-- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md index a461ebbadd..e984064da6 100644 --- a/docs/designs/qemu-deprivilege.md +++ b/docs/designs/qemu-deprivilege.md @@ -105,12 +105,6 @@ call: [qemu-namespaces]: https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg04723.html -# Restrictions / improvements still to do - -This lists potential restrictions still to do. It is meant to be -listed in order of ease of implementation, with low-hanging fruit -first. - ### Basic RLIMITs '''Description''': A number of limits on the resources that a given @@ -137,6 +131,12 @@ are specified; this does not apply to QEMU running as a Xen DM. '''Tested''': Not tested +# Restrictions / improvements still to do + +This lists potential restrictions still to do. It is meant to be +listed in order of ease of implementation, with low-hanging fruit +first. + ### Further RLIMITs RLIMIT_AS limits the total amount of memory; but this includes the diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index c7a345f4bb..ac9526d731 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -12,11 +12,12 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Lesser General Public License for more details. */ - + #include "libxl_osdeps.h" /* must come before any other headers */ #include "libxl_internal.h" - +#include + int libxl__try_phy_backend(mode_t st_mode) { if (S_ISBLK(st_mode) || S_ISREG(st_mode)) { @@ -307,9 +308,31 @@ int libxl__pci_topology_init(libxl__gc *gc, return err; } +static struct { +int resource; +rlim_t limit; +} rlimits[] = { +#define RLIMIT_ENTRY(r, l) \ +{ .resource = r, .limit = l } +/* Big enough for log files, not big enough for a DoS */ +RLIMIT_ENTRY(RLIMIT_FSIZE,256*1024), + +/* Shouldn't need any of these */ +RLIMIT_ENTRY(RLIMIT_NPROC,0), +RLIMIT_ENTRY(RLIMIT_CORE, 0), +RLIMIT_ENTRY(RLIMIT_MSGQUEUE, 0), +RLIMIT_ENTRY(RLIMIT_LOCKS,0), +RLIMIT_ENTRY(RLIMIT_MEMLOCK, 0), + +/* End-of-list marker */ +RLIMIT_ENTRY(RLIMIT_NLIMITS, 0), +}; +#undef RLIMIT_ENTRY + int libxl__local_dm_preexec_restrict(libxl__gc *gc) { int r; +unsigned i; /* Unshare mount and IPC namespaces. These are unused by QEMU. */ r = unshare(CLONE_NEWNS | CLONE_NEWIPC); @@ -318,6 +341,21 @@ int libxl__local_dm_preexec_restrict(libxl__gc *gc) return ERROR_FAIL; } +/* Set various "easy" rlimits */ +for (i = 0; rlimits[i].resource != RLIMIT_NLIMITS; i++) { +struct rlimit rlim; + +rlim.rlim_cur = rlim.rlim_max = rlimits[i].limit; + +r = setrlimit(rlimits[i].resource, &rlim); +if (r < 0) { +LOGE(ERROR, "Setting rlimit %d to %llu failed\n", + rlimits[i].resource, + (unsigned long long)rlimits[i].limit); +return ERROR_FAIL; +} +} + return 0; } -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 2/6] SUPPORT.md: Add qemu-depriv section
Signed-off-by: George Dunlap --- Changes since v3: - Moved from the qemu-depriv doc patches. - Reword to include the possibility of having a non-dom0 "devicemodel" domain which may want to be protected - Specify `Linux dom0` as the currently-tech-supported window CC: Ian Jackson CC: Wei Liu CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan CC: Konrad Wilk CC: Stefano Stabellini CC: Julien Grall CC: Anthony Perard CC: Ross Lagerwall --- SUPPORT.md | 20 1 file changed, 20 insertions(+) diff --git a/SUPPORT.md b/SUPPORT.md index 4f203da84a..1f0f5857a7 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -525,6 +525,26 @@ Vulnerabilities of a device model stub domain to a hostile driver domain (either compromised or untrusted) are excluded from security support. +### Device Model Deprivileging + +Status, Linux dom0: Tech Preview, with limited support + +This means adding extra restrictions to a device model in order to +prevent a compromised device model from attack the rest of the domain +it's running in (normally dom0). + +"Tech preview with limited support" means we will not issue XSAs for +the _additional_ functionality provided by the feature; but we will +issue XSAs in the event that enabling this feature opens up a security +hole that would not be present without the feature disabled. + +For example, while this is classified as tech preview, a bug in libxl +which failed to change the user ID of QEMU would not receive an XSA, +since without this feature the user ID wouldn't be changed. But a +change which made it possible for a compromised guest to read +arbitrary files on the host filesystem without compromising QEMU would +be issued an XSA, since that does weaken security. + ### KCONFIG Expert Status: Experimental -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
When dm_restrict is enabled, ask QEMU to chroot into an empty directory. * Create /var/run/qemu/root-domid (deleting the old one if it's there) * Pass the -chroot option to QEMU Rather than running `rm -rf` on the directory before creating it (since there is no library function to do this), simply rmdir the directory, relying on the fact that the previous QEMU instance, if properly restricted, shouldn't have been able to write anything anyway. Suggested-by: Ross Lagerwall Signed-off-by: George Dunlap Acked-by: Ian Jackson --- Changes since v2: - Style fixes - Testing moved to a different patch CC: Ian Jackson CC: Wei Liu CC: Anthony Perard --- docs/designs/qemu-deprivilege.md | 12 +- tools/libxl/libxl_dm.c | 41 +++- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md index 787ae1ac7c..039540 100644 --- a/docs/designs/qemu-deprivilege.md +++ b/docs/designs/qemu-deprivilege.md @@ -61,12 +61,6 @@ source tree.) '''Testing status''': Tested -# Restrictions / improvements still to do - -This lists potential restrictions still to do. It is meant to be -listed in order of ease of implementation, with low-hanging fruit -first. - ## Chroot '''Description''': Qemu runs in its own chroot, such that even if it @@ -84,6 +78,12 @@ Then adds the following to the qemu command-line: '''Tested''': Not tested +## Restrictions / improvements still to do + +This lists potential restrictions still to do. It is meant to be +listed in order of ease of implementation, with low-hanging fruit +first. + ## Namespaces for unused functionality (Linux only) '''Description''': QEMU doesn't use the functionality associated with diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 26eb16af34..ad3efcc783 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -1410,9 +1410,48 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, } } -if (libxl_defbool_val(b_info->dm_restrict)) +if (libxl_defbool_val(b_info->dm_restrict)) { +char *chroot_dir = GCSPRINTF("%s/qemu-root-%d", + libxl__run_dir_path(), guest_domid); +int r; + flexarray_append(dm_args, "-xen-domid-restrict"); +/* + * Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-%d + * + * There is no library function to do the equivalent of `rm + * -rf`. However deprivileged QEMU in theory shouldn't be + * able to write any files, as the chroot would be owned by + * root, but it would be running as an unprivileged process. + * So in theory, old chroots should always be empty. + * + * rmdir the directory before attempting to create + * it; if it returns anything other than ENOENT, fail domain + * creation. + */ +r = rmdir(chroot_dir); +if (r != 0 && errno != ENOENT) { +LOGED(ERROR, guest_domid, + "failed to remove existing chroot dir %s", chroot_dir); +return ERROR_FAIL; +} + +for (;;) { +r = mkdir(chroot_dir, ); +if (!r) +break; +if (errno == EINTR) continue; +LOGED(ERROR, guest_domid, + "failed to create chroot dir %s", chroot_dir); +return ERROR_FAIL; +} + +/* Add "-chroot [dir]" to command-line */ +flexarray_append(dm_args, "-chroot"); +flexarray_append(dm_args, chroot_dir); +} + if (state->saved_state) { /* This file descriptor is meant to be used by QEMU */ *dm_state_fd = open(state->saved_state, O_RDONLY); -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 6/6] RFC: test/depriv: Add a tool to check process-level depriv
Add a tool to check whether the various process-level deprivileging operations have actually taken place on the process. The tool takes a domname or domid, and returns success or failure. Signed-off-by: George Dunlap --- Changes since v3: - Use xen-qemuuser-range-base's gid rather than hard-coding `nobody` - Change FIXME about not handling other userid schemes into an NB. Changes since v2: - Make grep for Uid line more strict - Fix Gid grep, make more strict - Match strictly more than one space - Look up the group ID for `nobody` rather than hard-coding it - Move tests from other patches into one patch - Remove suffix (in case we change the language) - Install in the path NB this patch is included for reference only, while I consider whether to leave this as a stand-alone script, or whether to merge osstest's fd checker functionality into it (perhaps changing the language to perl at the same time). Reviews of the general detection algorithm are welcome, but there's no need for a detailed review of the code until the script is in its final form. CC: Ian Jackson CC: Wei Liu CC: Stefano Stabellini CC: Anthony Perard CC: Ross Lagerwall --- tools/tests/depriv/Makefile | 2 +- tools/tests/depriv/depriv-process-checker | 148 ++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100755 tools/tests/depriv/depriv-process-checker diff --git a/tools/tests/depriv/Makefile b/tools/tests/depriv/Makefile index 3cba28da25..1b3d09e97d 100644 --- a/tools/tests/depriv/Makefile +++ b/tools/tests/depriv/Makefile @@ -23,7 +23,7 @@ LDLIBS += $(LDLIBS_libxendevicemodel) LDLIBS += $(LDLIBS_libxentoolcore) LDLIBS += $(LDLIBS_libxentoollog) -INSTALL_PRIVBIN-y += depriv-fd-checker +INSTALL_PRIVBIN-y += depriv-fd-checker depriv-process-checker INSTALL_PRIVBIN := $(INSTALL_PRIVBIN-y) TARGETS += $(INSTALL_PRIVBIN) diff --git a/tools/tests/depriv/depriv-process-checker b/tools/tests/depriv/depriv-process-checker new file mode 100755 index 00..4f9f0d7fbc --- /dev/null +++ b/tools/tests/depriv/depriv-process-checker @@ -0,0 +1,148 @@ +#!/bin/bash + +domain="$1" + +if [[ "$domain" =~ ^[0-9]+$ ]] ; then +domid="$domain" +else +domid=$(xl domid "$domain") +fi + +dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid 2>/dev/null) +if [[ -z "$dmpid" ]] ; then +echo "xenstore-read failed" +exit 1 +fi + +failed="false" + +# TEST: Process / group id +# +# Read /proc//status, checking Uid and Gid lines +# +# Uid should be xen-qemuuser-range-base+$domid +# Gid should be gid for xen-qemuuser-range-base +# +# NB this doesn't handle other configurations (e.g., +# xen-qemuuser-shared). +echo -n "Process UID: " +tgt_uid=$(id -u xen-qemuuser-range-base) +tgt_uid=$(( $tgt_uid + $domid )) + +# Example input: +# Uid: 1193119311931193 +input=$(grep ^Uid: /proc/$dmpid/status) +if [[ "$input" =~ ^Uid:[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)$ ]] ; then +result="PASSED" +for i in {1..4}; do + if [[ "${BASH_REMATCH[$i]}" != "$tgt_uid" ]] ; then + result="FAILED" + failed="true" + break + fi +done +else +result="FAILED" +failed="true" +fi +echo $result + +# Example input: +# Gid: 10020 10020 10020 10020 +echo -n "Process GID: " +tgt_gid=$(id -g xen-qemuuser-range-base) +input=$(grep ^Gid: /proc/$dmpid/status) +if [[ "$input" =~ ^Gid:[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)$ ]] ; then +result="PASSED" +for i in {1..4}; do + if [[ "${BASH_REMATCH[$i]}" != "$tgt_gid" ]] ; then + result="FAILED" + failed="true" + break + fi +done +else +result="FAILED" +failed="true" +fi +echo $result + +# TEST: chroot +# +# Read /proc//root to see if it's correct. +echo -n "Chroot: " +if [[ -n "$XEN_RUN_DIR" ]] ; then +tgt_chroot=$XEN_RUN_DIR/qemu-root-$domid +root=$(readlink /proc/$dmpid/root) +if [[ "$root" != "$tgt_chroot" ]] ; then + echo "FAILED" + failed="true" +else + echo "PASSED" +fi +else +echo "FAILED (XEN_RUN_DIR undefined)" +failed="true" +fi + +# TEST: Namespace unsharing +# +# Read /proc//ns/ and make sure it's not equal to +# the current processes' value +for nsname in ipc mnt; do +echo -n "Unshare namespace $nsname: " +dmns=$(readlink /proc/$dmpid/ns/$nsname) +myns=$(readlink /proc/self/ns/$nsname) + +if [[ "$dmns" == "$myns" ]] ; then + echo "FAILED" + failed="true" +else + echo "PASSED" +fi +done + +# TEST: RLIMITs +# +# Read /proc//limits +function check_rlimit() { +limit_name=$1 +limit_string=$2 +tgt=$3 + +echo -n "rlimit $limit_name: " +input=$(grep "^$limit_string" /proc/$dmpid/limits) + +if [[ -z "$input" ]] ; then + echo "Couldn't find limit $limit" + echo FAILED + failed="true" + return +fi +
Re: [Xen-devel] [RFC 09/16] xen/arm: p2m: Introduce a function to resolve translation fault
On Mon, 5 Nov 2018, Julien Grall wrote: > On 02/11/2018 23:27, Stefano Stabellini wrote: > > On Mon, 8 Oct 2018, Julien Grall wrote: > > > Currently a Stage-2 translation fault could happen: > > > 1) MMIO emulation > > > 2) When the page-tables is been updated using Break-Before-Make > >^ have > > > > > 3) Page not mapped > > > > > > A follow-up patch will re-purpose the valid bit in an entry to generate > > > translation fault. This would be used to do an action on each entries to > > ^entry > > > > > track page used for a given period. > > ^pages > > > > > > > > > > A new function is introduced to try to resolve a translation fault. This > > > will include 2) and the new way to generate fault explained above. > > > > I can see the code does what you describe, but I don't understand why we > > are doing this. What is missing in the commit message is the explanation > > of the relationship between the future goal of repurposing the valid bit > > and the introduction of a function to handle Break-Before-Make stage2 > > faults. Does it fix an issue with Break-Before-Make that we currently > > have? Or it becomes needed due to the repurposing of valid? If so, why? > > This does not fix any issue with BBM. The valid bit adds a 4th reasons for > translation fault. Both BBM and the valid bit will require to walk the > page-tables. > > For the valid bit, we will need to walk the page-table in order to fixup the > entry (i.e set valid bit). We also can't use p2m_lookup(...) has it only tell > you the mapping exists, the valid bit may still not be set. > > So we need to provide a new helper to walk the page-table and fixup an entry. OK. Please expand a bit the commit message. > > > To avoid invalidating all the page-tables entries in one go. It is > > > possible to invalidate the top-level table and then on trap invalidate > > > the table one-level down. This will be repeated until a block/page entry > > > has been reached. > > > > > > At the moment, there are no action done when reaching a block/page entry > > > but setting the valid bit to 1. > > > > > > Signed-off-by: Julien Grall > > > --- > > > xen/arch/arm/p2m.c | 127 > > > +++ > > > xen/arch/arm/traps.c | 7 +-- > > > 2 files changed, 131 insertions(+), 3 deletions(-) > > > > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > > index ec956bc151..af445d3313 100644 > > > --- a/xen/arch/arm/p2m.c > > > +++ b/xen/arch/arm/p2m.c > > > @@ -1043,6 +1043,133 @@ int p2m_set_entry(struct p2m_domain *p2m, > > > return rc; > > > } > > > +/* Invalidate all entries in the table. The p2m should be write locked. > > > */ > > > +static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) > > > +{ > > > +lpae_t *table; > > > +unsigned int i; > > > + > > > +ASSERT(p2m_is_write_locked(p2m)); > > > + > > > +table = map_domain_page(mfn); > > > + > > > +for ( i = 0; i < LPAE_ENTRIES; i++ ) > > > +{ > > > +lpae_t pte = table[i]; > > > + > > > +pte.p2m.valid = 0; > > > + > > > +p2m_write_pte(&table[i], pte, p2m->clean_pte); > > > +} > > > + > > > +unmap_domain_page(table); > > > + > > > +p2m->need_flush = true; > > > +} > > > + > > > +bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn) > > > +{ > > > +struct p2m_domain *p2m = p2m_get_hostp2m(d); > > > +unsigned int level = 0; > > > +bool resolved = false; > > > +lpae_t entry, *table; > > > +paddr_t addr = gfn_to_gaddr(gfn); > > > + > > > +/* Convenience aliases */ > > > +const unsigned int offsets[4] = { > > > +zeroeth_table_offset(addr), > > > +first_table_offset(addr), > > > +second_table_offset(addr), > > > +third_table_offset(addr) > > > +}; > > > + > > > +p2m_write_lock(p2m); > > > + > > > +/* This gfn is higher than the highest the p2m map currently holds */ > > > +if ( gfn_x(gfn) > gfn_x(p2m->max_mapped_gfn) ) > > > +goto out; > > > + > > > +table = p2m_get_root_pointer(p2m, gfn); > > > +/* > > > + * The table should always be non-NULL because the gfn is below > > > + * p2m->max_mapped_gfn and the root table pages are always present. > > > + */ > > > +BUG_ON(table == NULL); > > > + > > > +/* > > > + * Go down the page-tables until an entry has the valid bit unset or > > > + * a block/page entry has been hit. > > > + */ > > > +for ( level = P2M_ROOT_LEVEL; level <= 3; level++ ) > > > +{ > > > +int rc; > > > + > > > +entry = table[offsets[level]]; > > > + > > > +if ( level == 3 ) > > > +break; > > > + > > > +/* Stop as soon as we hit an entry with the valid bit unset. */ > > > +if ( !lpae_is_valid(entry) ) > > > +break; > > > + > > > +rc
Re: [Xen-devel] [PATCH v3 5/5] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy
On Mon, Nov 05, 2018 at 11:16:40AM +, Andrew Cooper wrote: > From: Sergey Dyasli > > This finally (after literally years of work!) marks the point where the > toolstack can ask the hypervisor for the current CPUID configuration of a > specific domain. > > Introduce a new flask access vector and update the default policies. > > Also extend xen-cpuid's --policy mode to be able to take a domid and dump a > specific domains CPUID and MSR policy. > > Signed-off-by: Andrew Cooper > Signed-off-by: Sergey Dyasli Reviewed-by: Roger Pau Monné > diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c > index 95ed853..2c41031 100644 > --- a/tools/misc/xen-cpuid.c > +++ b/tools/misc/xen-cpuid.c > @@ -3,6 +3,8 @@ > #include > #include > #include > +#include > +#include > > #include > > @@ -309,11 +311,13 @@ int main(int argc, char **argv) > { > enum { MODE_UNKNOWN, MODE_INFO, MODE_DETAIL, MODE_INTERPRET, MODE_POLICY > } > mode = MODE_UNKNOWN; > +int domid = -1; Would it be better to use DOMID_INVALID instead of -1? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: put x86emul_{read, write}_dr under CONFIG_PV
On 05/11/18 17:38, Wei Liu wrote: > A build breakage is discovered by a non-debug build. Debug build > worked because the ASSERT made the compiler eliminate the rest of the > functions. > > Currently they are PV only. There are comments alluding to possible > future HVM support but we can cross the bride when we get there. > > Signed-off-by: Wei Liu Hmm - this is disappointing, but probably the least bad course of action. Longterm I want to merge the PV and HVM CR's into arch_vcpu which avoids one of the two build breakages, but the dr7 path will always have a PV conditional part. Acked-by: Andrew Cooper ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86: put x86emul_{read, write}_dr under CONFIG_PV
A build breakage is discovered by a non-debug build. Debug build worked because the ASSERT made the compiler eliminate the rest of the functions. Currently they are PV only. There are comments alluding to possible future HVM support but we can cross the bride when we get there. Signed-off-by: Wei Liu --- xen/arch/x86/x86_emulate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c index 886bd87e59..b1dfc9f261 100644 --- a/xen/arch/x86/x86_emulate.c +++ b/xen/arch/x86/x86_emulate.c @@ -89,6 +89,7 @@ int x86emul_write_xcr(unsigned int reg, uint64_t val, return X86EMUL_OKAY; } +#ifdef CONFIG_PV /* Called with NULL ctxt in hypercall context. */ int x86emul_read_dr(unsigned int reg, unsigned long *val, struct x86_emulate_ctxt *ctxt) @@ -155,6 +156,7 @@ int x86emul_write_dr(unsigned int reg, unsigned long val, return X86EMUL_EXCEPTION; } } +#endif /* CONFIG_PV */ /* * Local variables: -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 14/16] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit)
Hi Stefano, On 05/11/2018 17:31, Stefano Stabellini wrote: On Mon, 5 Nov 2018, Julien Grall wrote: Hi Stefano, On 02/11/2018 23:44, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: With the recent changes, a P2M entry may be populated but may as not valid. In some situation, it would be useful to know whether the entry has been marked available to guest in order to perform a specific action. So extend p2m_get_entry to return the value of bit[0] (valid bit). Signed-off-by: Julien Grall --- xen/arch/arm/mem_access.c | 6 +++--- xen/arch/arm/p2m.c| 20 xen/include/asm-arm/p2m.h | 3 ++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c index 9239bdf323..f434510b2a 100644 --- a/xen/arch/arm/mem_access.c +++ b/xen/arch/arm/mem_access.c @@ -70,7 +70,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, * No setting was found in the Radix tree. Check if the * entry exists in the page-tables. */ -mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL); +mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) return -ESRCH; @@ -199,7 +199,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, * We had a mem_access permission limiting the access, but the page type * could also be limiting, so we need to check that as well. */ -mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL); +mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) goto err; @@ -405,7 +405,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr, gfn = gfn_next_boundary(gfn, order) ) { p2m_type_t t; -mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order); +mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order, NULL); if ( !mfn_eq(mfn, INVALID_MFN) ) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 12b459924b..df6b48d73b 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -306,10 +306,14 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, * * If the entry is not present, INVALID_MFN will be returned and the * page_order will be set according to the order of the invalid range. + * + * valid will contain the value of bit[0] (e.g valid bit) of the + * entry. */ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, p2m_type_t *t, p2m_access_t *a, -unsigned int *page_order) +unsigned int *page_order, +bool *valid) { paddr_t addr = gfn_to_gaddr(gfn); unsigned int level = 0; @@ -317,6 +321,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, int rc; mfn_t mfn = INVALID_MFN; p2m_type_t _t; +bool _valid; /* Convenience aliases */ const unsigned int offsets[4] = { @@ -334,6 +339,10 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, *t = p2m_invalid; +/* Allow valid to be NULL */ +valid = valid?: &_valid; +*valid = false; Why not a simple: if ( valid ) *valid = false; especially given that you do the same if ( valid ) check below. In fact, it doesn' look like we need _valid? I thought I dropped the if ( valid ) below. I would actually prefer to keep _valid and avoid using if ( ... ) everywhere. This makes the code slightly easier to follow. _valid is a good trick, but I don't think is worth doing it in this change: it is easy to follow anyway. Up to you, I am fine either way. I will use if ( valid ) in the next version. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 14/16] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit)
On Mon, 5 Nov 2018, Julien Grall wrote: > Hi Stefano, > > On 02/11/2018 23:44, Stefano Stabellini wrote: > > On Mon, 8 Oct 2018, Julien Grall wrote: > > > With the recent changes, a P2M entry may be populated but may as not > > > valid. In some situation, it would be useful to know whether the entry > > > has been marked available to guest in order to perform a specific > > > action. So extend p2m_get_entry to return the value of bit[0] (valid bit). > > > > > > Signed-off-by: Julien Grall > > > --- > > > xen/arch/arm/mem_access.c | 6 +++--- > > > xen/arch/arm/p2m.c| 20 > > > xen/include/asm-arm/p2m.h | 3 ++- > > > 3 files changed, 21 insertions(+), 8 deletions(-) > > > > > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > > > index 9239bdf323..f434510b2a 100644 > > > --- a/xen/arch/arm/mem_access.c > > > +++ b/xen/arch/arm/mem_access.c > > > @@ -70,7 +70,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t > > > gfn, > > >* No setting was found in the Radix tree. Check if the > > >* entry exists in the page-tables. > > >*/ > > > -mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL); > > > +mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL, NULL); > > > if ( mfn_eq(mfn, INVALID_MFN) ) > > > return -ESRCH; > > > @@ -199,7 +199,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, > > > unsigned long flag, > > >* We had a mem_access permission limiting the access, but the page > > > type > > >* could also be limiting, so we need to check that as well. > > >*/ > > > -mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL); > > > +mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL, NULL); > > > if ( mfn_eq(mfn, INVALID_MFN) ) > > > goto err; > > > @@ -405,7 +405,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, > > > uint32_t nr, > > > gfn = gfn_next_boundary(gfn, order) ) > > > { > > > p2m_type_t t; > > > -mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order); > > > +mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order, NULL); > > > if ( !mfn_eq(mfn, INVALID_MFN) ) > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > > index 12b459924b..df6b48d73b 100644 > > > --- a/xen/arch/arm/p2m.c > > > +++ b/xen/arch/arm/p2m.c > > > @@ -306,10 +306,14 @@ static int p2m_next_level(struct p2m_domain *p2m, > > > bool read_only, > > >* > > >* If the entry is not present, INVALID_MFN will be returned and the > > >* page_order will be set according to the order of the invalid range. > > > + * > > > + * valid will contain the value of bit[0] (e.g valid bit) of the > > > + * entry. > > >*/ > > > mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, > > > p2m_type_t *t, p2m_access_t *a, > > > -unsigned int *page_order) > > > +unsigned int *page_order, > > > +bool *valid) > > > { > > > paddr_t addr = gfn_to_gaddr(gfn); > > > unsigned int level = 0; > > > @@ -317,6 +321,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, > > > int rc; > > > mfn_t mfn = INVALID_MFN; > > > p2m_type_t _t; > > > +bool _valid; > > > /* Convenience aliases */ > > > const unsigned int offsets[4] = { > > > @@ -334,6 +339,10 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t > > > gfn, > > > *t = p2m_invalid; > > > +/* Allow valid to be NULL */ > > > +valid = valid?: &_valid; > > > +*valid = false; > > > > Why not a simple: > > > >if ( valid ) > > *valid = false; > > > > especially given that you do the same if ( valid ) check below. > > In fact, it doesn' look like we need _valid? > > I thought I dropped the if ( valid ) below. I would actually prefer to keep > _valid and avoid using if ( ... ) everywhere. > > This makes the code slightly easier to follow. _valid is a good trick, but I don't think is worth doing it in this change: it is easy to follow anyway. Up to you, I am fine either way. > > > > > > > /* XXX: Check if the mapping is lower than the mapped gfn */ > > > /* This gfn is higher than the highest the p2m map currently holds > > > */ > > > @@ -379,6 +388,9 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, > > >* to the GFN. > > >*/ > > > mfn = mfn_add(mfn, gfn_x(gfn) & ((1UL << level_orders[level]) - > > > 1)); > > > + > > > +if ( valid ) > > > +*valid = lpae_is_valid(entry); > > > } > > > out_unmap: > > > @@ -397,7 +409,7 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, > > > p2m_type_t *t) > > > struct p2m_domain *p2m = p2m_get_hostp2m(d); > > > p2m_read_lock(p2m); > > > -mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL); > > > +mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL, NULL); > > >
Re: [Xen-devel] [PATCH v3 5/7] x86: make entry point code build when !CONFIG_PV
On 05/11/18 17:08, Wei Liu wrote: > On Mon, Nov 05, 2018 at 02:33:23PM +, Andrew Cooper wrote: >> On 02/11/18 15:55, Wei Liu wrote: >>> Skip building x86_64/compat/entry.S and put CONFIG_PV in >>> x86_64/entry.S. >>> >>> Signed-off-by: Wei Liu >>> --- >>> v3: >>> 1. make CR4_PV32_RESTORE expand to nothing when !PV >>> 2. use unconditional jmp and add assertions >>> >>> v2: new >>> --- >>> xen/arch/x86/x86_64/Makefile| 2 +- >>> xen/arch/x86/x86_64/entry.S | 39 +- >>> xen/include/asm-x86/asm_defns.h | 4 +++- >>> 3 files changed, 43 insertions(+), 2 deletions(-) >>> >>> diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile >>> index f336a6a..4bfa148 100644 >>> --- a/xen/arch/x86/x86_64/Makefile >>> +++ b/xen/arch/x86/x86_64/Makefile >>> @@ -1,4 +1,4 @@ >>> -subdir-y += compat >>> +subdir-$(CONFIG_PV) += compat >>> >>> obj-bin-y += entry.o >>> obj-y += traps.o >>> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S >>> index 9b02899..975ed6a 100644 >>> --- a/xen/arch/x86/x86_64/entry.S >>> +++ b/xen/arch/x86/x86_64/entry.S >>> @@ -15,6 +15,18 @@ >>> #include >>> #include >>> >>> +#ifndef NDEBUG >>> +/* %rsp: struct cpu_user_regs */ >>> +#define ASSERT_INTERRUPTED_XEN_CONTEXT\ >> Could I suggest ASSERT_CONTEXT_IS_XEN which I think fits better in some >> of the cases below? >> >>> +testb $3, UREGS_cs(%rsp); \ >>> +jz1f; \ >>> +ASSERT_FAILED("INTERRUPTED XEN CONTEXT"); \ >> This probably wants to be: >> >> .macro ASSERT_CONTEXT_IS_XEN >> #ifndef NDEBUG >> Â Â Â testb $3, UREGS_cs(%rsp); >> Â Â Â UNLIKELY_START(nz, ASSERT_XEN_\@); >> Â Â Â ASSERT_FAILED("CONTEXT IS XEN"); >> Â Â Â __UNLIKELY_END(ASSERT_XEN_\@); >> #endif >> .endm > This works. I have deleted all semicolons though. Nice! > > (To avoid posting other patches) > > ---8<--- > From 8f3ce0b0a5b6a215d4309d77d2c63229fb4af900 Mon Sep 17 00:00:00 2001 > From: Wei Liu > Date: Fri, 19 Oct 2018 12:32:12 +0100 > Subject: [PATCH] x86: make entry point code build when !CONFIG_PV > > Skip building x86_64/compat/entry.S and put CONFIG_PV in > x86_64/entry.S. > > Signed-off-by: Wei Liu Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/7] x86: make entry point code build when !CONFIG_PV
On Mon, Nov 05, 2018 at 02:33:23PM +, Andrew Cooper wrote: > On 02/11/18 15:55, Wei Liu wrote: > > Skip building x86_64/compat/entry.S and put CONFIG_PV in > > x86_64/entry.S. > > > > Signed-off-by: Wei Liu > > --- > > v3: > > 1. make CR4_PV32_RESTORE expand to nothing when !PV > > 2. use unconditional jmp and add assertions > > > > v2: new > > --- > > xen/arch/x86/x86_64/Makefile| 2 +- > > xen/arch/x86/x86_64/entry.S | 39 +- > > xen/include/asm-x86/asm_defns.h | 4 +++- > > 3 files changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile > > index f336a6a..4bfa148 100644 > > --- a/xen/arch/x86/x86_64/Makefile > > +++ b/xen/arch/x86/x86_64/Makefile > > @@ -1,4 +1,4 @@ > > -subdir-y += compat > > +subdir-$(CONFIG_PV) += compat > > > > obj-bin-y += entry.o > > obj-y += traps.o > > diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S > > index 9b02899..975ed6a 100644 > > --- a/xen/arch/x86/x86_64/entry.S > > +++ b/xen/arch/x86/x86_64/entry.S > > @@ -15,6 +15,18 @@ > > #include > > #include > > > > +#ifndef NDEBUG > > +/* %rsp: struct cpu_user_regs */ > > +#define ASSERT_INTERRUPTED_XEN_CONTEXT\ > > Could I suggest ASSERT_CONTEXT_IS_XEN which I think fits better in some > of the cases below? > > > +testb $3, UREGS_cs(%rsp); \ > > +jz1f; \ > > +ASSERT_FAILED("INTERRUPTED XEN CONTEXT"); \ > > This probably wants to be: > > .macro ASSERT_CONTEXT_IS_XEN > #ifndef NDEBUG > Â Â Â testb $3, UREGS_cs(%rsp); > Â Â Â UNLIKELY_START(nz, ASSERT_XEN_\@); > Â Â Â ASSERT_FAILED("CONTEXT IS XEN"); > Â Â Â __UNLIKELY_END(ASSERT_XEN_\@); > #endif > .endm This works. I have deleted all semicolons though. (To avoid posting other patches) ---8<--- From 8f3ce0b0a5b6a215d4309d77d2c63229fb4af900 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Fri, 19 Oct 2018 12:32:12 +0100 Subject: [PATCH] x86: make entry point code build when !CONFIG_PV Skip building x86_64/compat/entry.S and put CONFIG_PV in x86_64/entry.S. Signed-off-by: Wei Liu --- v4: 1. Change name to ASSERT_CONTEXT_IS_XEN 2. Use UNLIKELY v3: 1. make CR4_PV32_RESTORE expand to nothing when !PV 2. use unconditional jmp and add assertions v2: new --- xen/arch/x86/x86_64/Makefile| 2 +- xen/arch/x86/x86_64/entry.S | 38 +- xen/include/asm-x86/asm_defns.h | 4 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile index f336a6ae65..4bfa1480eb 100644 --- a/xen/arch/x86/x86_64/Makefile +++ b/xen/arch/x86/x86_64/Makefile @@ -1,4 +1,4 @@ -subdir-y += compat +subdir-$(CONFIG_PV) += compat obj-bin-y += entry.o obj-y += traps.o diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 8e12decea8..e8eae3b08d 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -15,6 +15,17 @@ #include #include +/* %rsp: struct cpu_user_regs */ +.macro ASSERT_CONTEXT_IS_XEN +#ifndef NDEBUG +testb $3, UREGS_cs(%rsp) +UNLIKELY_START(nz, ASSERT_XEN_\@) +ASSERT_FAILED("INTERRUPTED XEN CONTEXT") +__UNLIKELY_END(ASSERT_XEN_\@) +#endif +.endm + +#ifdef CONFIG_PV /* %rbx: struct vcpu */ ENTRY(switch_to_kernel) leaq VCPU_trap_bounce(%rbx),%rdx @@ -522,6 +533,7 @@ ENTRY(dom_crash_sync_extable) xorl %edi,%edi jmp asm_domain_crash_synchronous /* Does not return */ .popsection +#endif /* CONFIG_PV */ /* --- CODE BELOW THIS LINE (MOSTLY) NOT GUEST RELATED --- */ @@ -529,6 +541,7 @@ ENTRY(dom_crash_sync_extable) /* No special register assumptions. */ ENTRY(ret_from_intr) +#ifdef CONFIG_PV GET_CURRENT(bx) testb $3, UREGS_cs(%rsp) jzrestore_all_xen @@ -536,6 +549,10 @@ ENTRY(ret_from_intr) cmpb $0, DOMAIN_is_32bit_pv(%rax) jetest_all_events jmp compat_test_all_events +#else +ASSERT_CONTEXT_IS_XEN +jmp restore_all_xen +#endif .section .text.entry, "ax", @progbits @@ -618,6 +635,7 @@ handle_exception_saved: testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp) jzexception_with_ints_disabled +#ifdef CONFIG_PV ALTERNATIVE_2 "jmp .Lcr4_pv32_done", \ __stringify(mov VCPU_domain(%rbx), %rax), X86_FEATURE_XEN_SMEP, \ __stringify(mov VCPU_domain(%rbx), %rax), X86_FEATURE_XEN_SMAP @@ -657,6 +675,9 @@ handle_exception_saved: test $~(PFEC_write_access|PFEC_insn_fetch),%eax jzcompat_test_all_events .Lcr4_pv32_done: +#else +ASSERT_CONTEXT_IS_XEN +#endif /* CONFIG_PV */ sti 1: movq %rsp,%rdi movzbl UREGS_entry_vector(%rsp),%eax @@ -666,12 +687,17 @@ handle_exception_saved: INDIRECT_CALL %rdx mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
Re: [Xen-devel] [PATCH v3 6/7] vpci/msix: carve p2m hole for MSIX MMIO regions
>>> On 30.10.18 at 16:41, wrote: > Make sure the MSIX MMIO regions don't have p2m entries setup, so that > accesses to them trap into the hypervisor and can be handled by vpci. > > This is a side-effect of commit 042678762 for PVH Dom0, which added > mappings for all the reserved regions into the Dom0 p2m. I'm afraid the description is ambiguous or misleading, as I don't suppose you want to state that what the patch here does is a side effect of the mentioned commit. Instead I assume you mean that p2m entries we don't want get set up without the change here. > --- a/xen/drivers/vpci/header.c > +++ b/xen/drivers/vpci/header.c > @@ -88,6 +88,14 @@ static void modify_decoding(const struct pci_dev *pdev, > bool map, bool rom_only) > uint16_t cmd; > unsigned int i; > > +/* > + * Make sure there are no mappings in the MSIX MMIO areas, so that > accesses > + * can be trapped (and emulated) by Xen when the memory decoding bit is > + * enabled. > + */ > +if ( map && !rom_only && vpci_make_msix_hole(pdev) ) > +return; If I'm not mistaken, you punch holes after having set up p2m entries. This may be fine for Dom0, but looks racy for (future) DomU use of this code. If so, please add a respective fixme annotation. > +int vpci_make_msix_hole(const struct pci_dev *pdev) > +{ > +struct domain *d = pdev->domain; > +unsigned int i; > + > +if ( !pdev->vpci->msix ) > +return 0; > + > +/* Make sure there's a hole for the MSIX table/PBA in the p2m. */ > +for ( i = 0; i < ARRAY_SIZE(pdev->vpci->msix->tables); i++ ) > +{ > +unsigned long start = PFN_DOWN(vmsix_table_addr(pdev->vpci, i)); > +unsigned long end = PFN_DOWN(vmsix_table_addr(pdev->vpci, i) + > + vmsix_table_size(pdev->vpci, i) - 1); > + > +for ( ; start <= end; start++ ) > +{ > +p2m_type_t t; > +mfn_t mfn = get_gfn_query(d, start, &t); > + > +if ( t == p2m_mmio_direct && mfn_x(mfn) == start ) > +clear_identity_p2m_entry(d, start); Indentation. > +else if ( t != p2m_mmio_dm ) Can you please also permit p2m_invalid right away, as the long term plan is to default to that type instead of p2m_mmio_dm for unpopulated p2m entries? And perhaps using switch() then produces easier to read code. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/7] vpci: fix execution of long running operations
>>> On 30.10.18 at 16:41, wrote: > BAR map/unmap is a long running operation that needs to be preempted > in order to avoid overrunning the assigned vCPU time (or even > triggering the watchdog). > > Current logic for this preemption is wrong, and won't work at all for > AMD since only Intel makes use of hvm_io_pending (and even in that > case the current code is wrong). I'm having trouble understanding this, both for the AMD aspect (it is only vvmx.c which has a function call not mirrored on the AMD side) and for the supposed general brokenness. Without some clarification I can't judge whether re-implementing via tasklet is actually the best approach. > +void vpci_init_vcpu(struct vcpu *v) > +{ > +tasklet_init(&v->vpci.task, vpci_process_pending, (unsigned long)v); > } Since there's no respective cleanup code added afaics - what if the domain gets cleaned up behind the back of the (long running) tasklet? Don't you want to acquire (and then release) an extra domain reference somewhere? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/7] vpci: fix updating the command register
>>> On 30.10.18 at 16:41, wrote: > When switching the memory decoding bit in the command register the > rest of the changes where dropped, leading to only the memory decoding > bit being updated. > > Fix this by unconditionally writing the guest-requested command except > for the memory decoding bit, which will be updated once the p2m > changes are performed. Are you convinced there are no devices (or drivers) with errata requiring the write to happen in a single step? Remember that the register value immediately becomes visible to other (v)CPUs. > --- a/xen/drivers/vpci/header.c > +++ b/xen/drivers/vpci/header.c > @@ -333,8 +333,10 @@ static void cmd_write(const struct pci_dev *pdev, > unsigned int reg, > * hoping the guest will realize and try again. > */ > modify_bars(pdev, cmd & PCI_COMMAND_MEMORY, false); > -else > -pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd); > + > +/* Write the new command without updating the memory decoding bit. */ > +cmd = (cmd & ~PCI_COMMAND_MEMORY) | (current_cmd & PCI_COMMAND_MEMORY); > +pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd); Furthermore, if the mapping didn't get deferred, aren't you discarding the new decode bit value here, as written by modify_bars() -> apply_map() -> modify_decoding()? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 129426: tolerable FAIL
flight 129426 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/129426/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail pass in 129400 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail in 129400 like 129369 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 129400 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 129400 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 129400 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 129400 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 129400 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 129400 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 129400 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 129400 test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-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-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 2cf113891a38cc05434bc9876ffc107a990887be baseline version: xen 2cf113891a38cc05434bc9876ffc107a990887be Last test of basis 129426 2018-11-05 01:51:49 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf
Re: [Xen-devel] MSR load lists on Harpertown
On 05/11/18 16:25, Ian Jackson wrote: > Andrew Cooper writes ("[Xen-devel] MSR load lists on Harpertown"): >> I realise this is an old CPU, but I've I've encountered a weird failure >> on it. > ... >> However, given this behaviour, I can't think of any way to context >> switch NX properly, and leave 64bit guests in a working state. >> >> Do you have any suggestions? > ISTM that Xen should run on these old CPUs and also that it should > work, ideally, even if the microcode is buggy. > > So presumaly the right answer is to simply disable NX for the guest > completely ? As you've observed, when SCE doesn't work, everything is fully hosed. However, disabling NX on that CPU won't fix the problem, because the problem appears to be using the MSR load list. Without a microcode fix (if indeed this is a microcode issue, but given how much of vmentry is, I'd be surprised if it is something else), the only fix I can see is to effectively revert the change to use MSR load lists. However, I really don't want to do this unilaterally, because it will break NX handling on all Intel hardware, and reintroduce a wart into the context switch code which I thought I'd excised properly. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/hvm: Clean up the rest of bool_t from vm_event
On 11/05/2018 04:28 PM, Jan Beulich wrote: On 05.11.18 at 17:17, wrote: >> On 10/29/2018 03:53 PM, Alexandru Stefan ISAILA wrote: >>> --- a/xen/arch/x86/mm/p2m.c >>> +++ b/xen/arch/x86/mm/p2m.c >>> @@ -448,7 +448,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, >>> unsigned long gfn_l, >>> /* Try to unshare. If we fail, communicate ENOMEM without >>> * sleeping. */ >>> if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) >>> -(void)mem_sharing_notify_enomem(p2m->domain, gfn_l, 0); >>> +mem_sharing_notify_enomem(p2m->domain, gfn_l, false); >> >> Why do you remove the (void) cast here... >> >>> mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); >>> } >>> >>> @@ -839,8 +839,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, >>> mfn_t mfn, >>> * Foreign domains are okay to place an event as they >>> * won't go to sleep. */ >>> (void)mem_sharing_notify_enomem(p2m->domain, >>> -gfn_x(gfn_add(gfn, i)), >>> -0); >>> +gfn_x(gfn_add(gfn, i)), >>> false); >> >> ...but not here? > > I had asked that it strictly be removed from lines touched anyway, > and I left it to their discretion to leave alone neighboring lines. Of > course it would have helped if the description said so. Right. In that case: 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] x86/traps: Misc non-functional cleanup
>>> On 05.11.18 at 17:23, wrote: > On Mon, Nov 05, 2018 at 04:19:05PM +, Andrew Cooper wrote: >> * s/unsigned char/uint8_t/ for clarity >> * Drop redundant return at the end of a void function >> >> Signed-off-by: Andrew Cooper > > Reviewed-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/hvm: Clean up the rest of bool_t from vm_event
>>> On 05.11.18 at 17:17, wrote: > On 10/29/2018 03:53 PM, Alexandru Stefan ISAILA wrote: >> --- a/xen/arch/x86/mm/p2m.c >> +++ b/xen/arch/x86/mm/p2m.c >> @@ -448,7 +448,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, >> unsigned long gfn_l, >> /* Try to unshare. If we fail, communicate ENOMEM without >> * sleeping. */ >> if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) >> -(void)mem_sharing_notify_enomem(p2m->domain, gfn_l, 0); >> +mem_sharing_notify_enomem(p2m->domain, gfn_l, false); > > Why do you remove the (void) cast here... > >> mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); >> } >> >> @@ -839,8 +839,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, >> mfn_t mfn, >> * Foreign domains are okay to place an event as they >> * won't go to sleep. */ >> (void)mem_sharing_notify_enomem(p2m->domain, >> -gfn_x(gfn_add(gfn, i)), >> -0); >> +gfn_x(gfn_add(gfn, i)), >> false); > > ...but not here? I had asked that it strictly be removed from lines touched anyway, and I left it to their discretion to leave alone neighboring lines. Of course it would have helped if the description said so. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/hvm: Clean up the rest of bool_t from vm_event
On 11/5/18 6:17 PM, George Dunlap wrote: > On 10/29/2018 03:53 PM, Alexandru Stefan ISAILA wrote: >> Signed-off-by: Alexandru Isaila >> Acked-by: Tamas K Lengyel >> Acked-by: Jan Beulich >> >> --- >> Changes since V1: >> - Made style corrections suggested by Jan. >> --- >> xen/arch/x86/hvm/hvm.c| 3 ++- >> xen/arch/x86/mm/mem_sharing.c | 2 +- >> xen/arch/x86/mm/p2m.c | 5 ++--- >> xen/common/memory.c | 2 +- >> xen/common/vm_event.c | 4 ++-- >> xen/include/asm-x86/mem_sharing.h | 2 +- >> xen/include/xen/vm_event.h| 8 >> 7 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index a140e60c9c..a8566fb87c 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -1905,7 +1905,8 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned >> long gla, >> if ( sharing_enomem ) >> { >> int rv; >> -if ( (rv = mem_sharing_notify_enomem(currd, gfn, 1)) < 0 ) >> + >> +if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 ) >> { >> gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare " >> "gfn %lx, ENOMEM and no helper (rc %d)\n", >> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c >> index 1dab2c8cc3..be09c8871a 100644 >> --- a/xen/arch/x86/mm/mem_sharing.c >> +++ b/xen/arch/x86/mm/mem_sharing.c >> @@ -546,7 +546,7 @@ static int audit(void) >> } >> >> int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, >> -bool_t allow_sleep) >> + bool allow_sleep) >> { >> struct vcpu *v = current; >> int rc; >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c >> index a00a3c1bff..4bdc5e34e0 100644 >> --- a/xen/arch/x86/mm/p2m.c >> +++ b/xen/arch/x86/mm/p2m.c >> @@ -448,7 +448,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, >> unsigned long gfn_l, >> /* Try to unshare. If we fail, communicate ENOMEM without >> * sleeping. */ >> if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) >> -(void)mem_sharing_notify_enomem(p2m->domain, gfn_l, 0); >> +mem_sharing_notify_enomem(p2m->domain, gfn_l, false); > > Why do you remove the (void) cast here... > >> mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); >> } >> >> @@ -839,8 +839,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, >> mfn_t mfn, >> * Foreign domains are okay to place an event as they >> * won't go to sleep. */ >> (void)mem_sharing_notify_enomem(p2m->domain, >> -gfn_x(gfn_add(gfn, i)), >> -0); >> +gfn_x(gfn_add(gfn, i)), >> false); > > ...but not here? That's what Jan has recommended here: https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg02289.html Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] MSR load lists on Harpertown
Andrew Cooper writes ("[Xen-devel] MSR load lists on Harpertown"): > I realise this is an old CPU, but I've I've encountered a weird failure > on it. ... > However, given this behaviour, I can't think of any way to context > switch NX properly, and leave 64bit guests in a working state. > > Do you have any suggestions? ISTM that Xen should run on these old CPUs and also that it should work, ideally, even if the microcode is buggy. So presumaly the right answer is to simply disable NX for the guest completely ? Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/traps: Misc non-functional cleanup
On Mon, Nov 05, 2018 at 04:19:05PM +, Andrew Cooper wrote: > * s/unsigned char/uint8_t/ for clarity > * Drop redundant return at the end of a void function > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/traps: Misc non-functional cleanup
* s/unsigned char/uint8_t/ for clarity * Drop redundant return at the end of a void function Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu Noticed while reviewing Wei's CONFIG_PV series. --- xen/arch/x86/traps.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index c60c8f5..d8b2130 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1530,9 +1530,9 @@ void do_general_protection(struct cpu_user_regs *regs) if ( regs->error_code & X86_XEC_IDT ) { /* This fault must be due to instruction. */ -const struct trap_info *ti; -unsigned char vector = regs->error_code >> 3; -ti = &v->arch.pv.trap_ctxt[vector]; +uint8_t vector = regs->error_code >> 3; +const struct trap_info *ti = &v->arch.pv.trap_ctxt[vector]; + if ( permit_softint(TI_GET_DPL(ti), v, regs) ) { regs->rip += 2; @@ -1771,8 +1771,6 @@ void do_device_not_available(struct cpu_user_regs *regs) } else TRACE_0D(TRC_PV_MATH_STATE_RESTORE); - -return; } void do_debug(struct cpu_user_regs *regs) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] xen_disk qdevification
> -Original Message- > From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: 05 November 2018 15:58 > To: Paul Durrant > Cc: 'Kevin Wolf' ; Tim Smith ; > Stefano Stabellini ; qemu-bl...@nongnu.org; qemu- > de...@nongnu.org; Max Reitz ; Anthony Perard > ; xen-devel@lists.xenproject.org > Subject: Re: [Qemu-devel] xen_disk qdevification > > Paul Durrant writes: > > >> -Original Message- > >> From: Kevin Wolf [mailto:kw...@redhat.com] > >> Sent: 02 November 2018 11:04 > >> To: Tim Smith > >> Cc: xen-devel@lists.xenproject.org; qemu-de...@nongnu.org; qemu- > >> bl...@nongnu.org; Anthony Perard ; Paul > Durrant > >> ; Stefano Stabellini ; > >> Max Reitz ; arm...@redhat.com > >> Subject: xen_disk qdevification (was: [PATCH 0/3] Performance > improvements > >> for xen_disk v2) > >> > >> Am 02.11.2018 um 11:00 hat Tim Smith geschrieben: > >> > A series of performance improvements for disks using the Xen PV ring. > >> > > >> > These have had fairly extensive testing. > >> > > >> > The batching and latency improvements together boost the throughput > >> > of small reads and writes by two to six percent (measured using fio > >> > in the guest) > >> > > >> > Avoiding repeated calls to posix_memalign() reduced the dirty heap > >> > from 25MB to 5MB in the case of a single datapath process while also > >> > improving performance. > >> > > >> > v2 removes some checkpatch complaints and fixes the CCs > >> > >> Completely unrelated, but since you're the first person touching > >> xen_disk in a while, you're my victim: > >> > >> At KVM Forum we discussed sending a patch to deprecate xen_disk because > >> after all those years, it still hasn't been converted to qdev. Markus > is > >> currently fixing some other not yet qdevified block device, but after > >> that xen_disk will be the only one left. > >> > >> A while ago, a downstream patch review found out that there are some > QMP > >> commands that would immediately crash if a xen_disk device were present > >> because of the lacking qdevification. This is not the code quality > >> standard I envision for QEMU. It's time for non-qdev devices to go. > >> > >> So if you guys are still interested in the device, could someone please > >> finally look into converting it? > >> > > > > I have a patch series to do exactly this. It's somewhat involved as I > > need to convert the whole PV backend infrastructure. I will try to > > rebase and clean up my series a.s.a.p. > > Awesome! Please coordinate with Anthony Prerard to avoid duplicating > work if you haven't done so already. Sure. I have already spoken to Anthony. Thanks, Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/hvm: Clean up the rest of bool_t from vm_event
On 10/29/2018 03:53 PM, Alexandru Stefan ISAILA wrote: > Signed-off-by: Alexandru Isaila > Acked-by: Tamas K Lengyel > Acked-by: Jan Beulich > > --- > Changes since V1: > - Made style corrections suggested by Jan. > --- > xen/arch/x86/hvm/hvm.c| 3 ++- > xen/arch/x86/mm/mem_sharing.c | 2 +- > xen/arch/x86/mm/p2m.c | 5 ++--- > xen/common/memory.c | 2 +- > xen/common/vm_event.c | 4 ++-- > xen/include/asm-x86/mem_sharing.h | 2 +- > xen/include/xen/vm_event.h| 8 > 7 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index a140e60c9c..a8566fb87c 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -1905,7 +1905,8 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned > long gla, > if ( sharing_enomem ) > { > int rv; > -if ( (rv = mem_sharing_notify_enomem(currd, gfn, 1)) < 0 ) > + > +if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 ) > { > gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare " > "gfn %lx, ENOMEM and no helper (rc %d)\n", > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index 1dab2c8cc3..be09c8871a 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -546,7 +546,7 @@ static int audit(void) > } > > int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, > -bool_t allow_sleep) > + bool allow_sleep) > { > struct vcpu *v = current; > int rc; > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index a00a3c1bff..4bdc5e34e0 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -448,7 +448,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, > unsigned long gfn_l, > /* Try to unshare. If we fail, communicate ENOMEM without > * sleeping. */ > if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) > -(void)mem_sharing_notify_enomem(p2m->domain, gfn_l, 0); > +mem_sharing_notify_enomem(p2m->domain, gfn_l, false); Why do you remove the (void) cast here... > mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); > } > > @@ -839,8 +839,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, > mfn_t mfn, > * Foreign domains are okay to place an event as they > * won't go to sleep. */ > (void)mem_sharing_notify_enomem(p2m->domain, > -gfn_x(gfn_add(gfn, i)), > -0); > +gfn_x(gfn_add(gfn, i)), > false); ...but not here? -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
>>> On 05.11.18 at 16:49, wrote: > On 05/11/18 15:48, Wei Liu wrote: >> On Mon, Nov 05, 2018 at 08:04:37AM -0700, Jan Beulich wrote: >> On 02.11.18 at 16:55, wrote: --- a/xen/arch/x86/x86_64/traps.c +++ b/xen/arch/x86/x86_64/traps.c @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( } DEFINE_PER_CPU(struct stubs, stubs); + +#ifdef CONFIG_PV void lstar_enter(void); void cstar_enter(void); +#else +static inline void lstar_enter(void) +{ +panic("%s called", __func__); +} + +static inline void cstar_enter(void) +{ +panic("%s called", __func__); +} +#endif /* CONFIG_PV */ >>> Do we really need two separate stubs (and two separate string literals) >>> here? >> I think it is clearer if we have two distinct messages. But I'm not too >> fussed either way really. If you feel strongly about this, I'm happy to >> change it to only one function. > > This is the correct way to do it. __func__ will already be in the > string table, and the format string (being identical) will be merged. Why would __func__ be in the string table already, for functions containing no other references to it? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] xen_disk qdevification
Paul Durrant writes: >> -Original Message- >> From: Kevin Wolf [mailto:kw...@redhat.com] >> Sent: 02 November 2018 11:04 >> To: Tim Smith >> Cc: xen-devel@lists.xenproject.org; qemu-de...@nongnu.org; qemu- >> bl...@nongnu.org; Anthony Perard ; Paul Durrant >> ; Stefano Stabellini ; >> Max Reitz ; arm...@redhat.com >> Subject: xen_disk qdevification (was: [PATCH 0/3] Performance improvements >> for xen_disk v2) >> >> Am 02.11.2018 um 11:00 hat Tim Smith geschrieben: >> > A series of performance improvements for disks using the Xen PV ring. >> > >> > These have had fairly extensive testing. >> > >> > The batching and latency improvements together boost the throughput >> > of small reads and writes by two to six percent (measured using fio >> > in the guest) >> > >> > Avoiding repeated calls to posix_memalign() reduced the dirty heap >> > from 25MB to 5MB in the case of a single datapath process while also >> > improving performance. >> > >> > v2 removes some checkpatch complaints and fixes the CCs >> >> Completely unrelated, but since you're the first person touching >> xen_disk in a while, you're my victim: >> >> At KVM Forum we discussed sending a patch to deprecate xen_disk because >> after all those years, it still hasn't been converted to qdev. Markus is >> currently fixing some other not yet qdevified block device, but after >> that xen_disk will be the only one left. >> >> A while ago, a downstream patch review found out that there are some QMP >> commands that would immediately crash if a xen_disk device were present >> because of the lacking qdevification. This is not the code quality >> standard I envision for QEMU. It's time for non-qdev devices to go. >> >> So if you guys are still interested in the device, could someone please >> finally look into converting it? >> > > I have a patch series to do exactly this. It's somewhat involved as I > need to convert the whole PV backend infrastructure. I will try to > rebase and clean up my series a.s.a.p. Awesome! Please coordinate with Anthony Prerard to avoid duplicating work if you haven't done so already. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy
On Mon, Nov 05, 2018 at 11:16:40AM +, Andrew Cooper wrote: > From: Sergey Dyasli > > This finally (after literally years of work!) marks the point where the > toolstack can ask the hypervisor for the current CPUID configuration of a > specific domain. > > Introduce a new flask access vector and update the default policies. > > Also extend xen-cpuid's --policy mode to be able to take a domid and dump a > specific domains CPUID and MSR policy. > > Signed-off-by: Andrew Cooper > Signed-off-by: Sergey Dyasli > --- > CC: Jan Beulich > CC: Ian Jackson > CC: Wei Liu > CC: Roger Pau Monné > CC: Sergey Dyasli > CC: Daniel De Graaf > > v3: > * Misc code hygene > * Fix comments to avoid having a SYSCTL/DOMCTL paste error > * Introduce a new flask vector rather than reusing set_cpuid > --- > tools/flask/policy/modules/dom0.te | 2 +- > tools/flask/policy/modules/xen.if | 2 +- > tools/libxc/include/xenctrl.h | 3 ++ > tools/libxc/xc_cpuid_x86.c | 38 > tools/misc/xen-cpuid.c | 58 > - Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/5] x86/sysctl: Implement XEN_SYSCTL_get_cpu_policy
On Mon, Nov 05, 2018 at 11:16:39AM +, Andrew Cooper wrote: > From: Sergey Dyasli > > Provide a SYSCTL for the toolstack to obtain complete system CPUID and MSR > policy information. > > For the flask side of things, this subop is closely related to > {phys,cputopo,numa}info, so shares the physinfo access vector. > > Extend the xen-cpuid utility to be able to dump the system policies. An > example output is: > > Xen reports there are maximum 113 leaves and 3 MSRs > Raw policy: 93 leaves, 3 MSRs >CPUID: > leaf subleaf -> eax ebx ecx edx > : -> 000d:756e6547:6c65746e:49656e69 > 0001: -> 000306c3:00100800:7ffafbff:bfebfbff > 0002: -> 76036301:00f0b5ff::00c1 > 0004: -> 1c004121:01c0003f:003f: > 0004:0001 -> 1c004122:01c0003f:003f: > 0004:0002 -> 1c004143:01c0003f:01ff: > 0004:0003 -> 1c03c163:03c0003f:1fff:0006 > 0005: -> 0040:0040:0003:00042120 > 0006: -> 0077:0002:0009: > 0007: -> :27ab::9c00 > 000a: -> 07300403:::0603 > 000b: -> 0001:0002:0100: > 000b:0001 -> 0004:0008:0201: > 000d: -> 0007:0340:0340: > 000d:0001 -> 0001::: > 000d:0002 -> 0100:0240:: > 8000: -> 8008::: > 8001: -> ::0021:2c100800 > 8002: -> 65746e49:2952286c:6f655820:2952286e > 8003: -> 55504320:2d334520:30343231:20337620 > 8004: -> 2e332040:48473034:007a: > 8006: -> ::01006040: > 8007: -> :::0100 > 8008: -> 3027::: >MSRs: > index-> value > 00ce -> 8000 > > Signed-off-by: Andrew Cooper > Signed-off-by: Sergey Dyasli > Signed-off-by: Roger Pau Monné > --- > CC: Jan Beulich > CC: Ian Jackson > CC: Wei Liu > CC: Roger Pau Monné > CC: Sergey Dyasli > CC: Daniel De Graaf > > v2: > * Avoid introducing a Spectre V1 gadget > v3: > * Reorganise logic > --- > tools/libxc/include/xenctrl.h | 6 +++ > tools/libxc/xc_cpuid_x86.c | 57 > tools/misc/xen-cpuid.c | 86 > +++-- Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
On Mon, Nov 05, 2018 at 03:49:40PM +, Andrew Cooper wrote: > On 05/11/18 15:48, Wei Liu wrote: > > On Mon, Nov 05, 2018 at 08:04:37AM -0700, Jan Beulich wrote: > > On 02.11.18 at 16:55, wrote: > >>> --- a/xen/arch/x86/x86_64/traps.c > >>> +++ b/xen/arch/x86/x86_64/traps.c > >>> @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( > >>> } > >>> > >>> DEFINE_PER_CPU(struct stubs, stubs); > >>> + > >>> +#ifdef CONFIG_PV > >>> void lstar_enter(void); > >>> void cstar_enter(void); > >>> +#else > >>> +static inline void lstar_enter(void) > >>> +{ > >>> +panic("%s called", __func__); > >>> +} > >>> + > >>> +static inline void cstar_enter(void) > >>> +{ > >>> +panic("%s called", __func__); > >>> +} > >>> +#endif /* CONFIG_PV */ > >> Do we really need two separate stubs (and two separate string literals) > >> here? > > I think it is clearer if we have two distinct messages. But I'm not too > > fussed either way really. If you feel strongly about this, I'm happy to > > change it to only one function. > > This is the correct way to do it. __func__ will already be in the > string table, and the format string (being identical) will be merged. I think Jan was complaining two __func__'s (two string literals). Wei. > > ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
On 05/11/18 15:48, Wei Liu wrote: > On Mon, Nov 05, 2018 at 08:04:37AM -0700, Jan Beulich wrote: > On 02.11.18 at 16:55, wrote: >>> --- a/xen/arch/x86/x86_64/traps.c >>> +++ b/xen/arch/x86/x86_64/traps.c >>> @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( >>> } >>> >>> DEFINE_PER_CPU(struct stubs, stubs); >>> + >>> +#ifdef CONFIG_PV >>> void lstar_enter(void); >>> void cstar_enter(void); >>> +#else >>> +static inline void lstar_enter(void) >>> +{ >>> +panic("%s called", __func__); >>> +} >>> + >>> +static inline void cstar_enter(void) >>> +{ >>> +panic("%s called", __func__); >>> +} >>> +#endif /* CONFIG_PV */ >> Do we really need two separate stubs (and two separate string literals) >> here? > I think it is clearer if we have two distinct messages. But I'm not too > fussed either way really. If you feel strongly about this, I'm happy to > change it to only one function. This is the correct way to do it. __func__ will already be in the string table, and the format string (being identical) will be merged. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
On Mon, Nov 05, 2018 at 08:04:37AM -0700, Jan Beulich wrote: > >>> On 02.11.18 at 16:55, wrote: > > --- a/xen/arch/x86/x86_64/traps.c > > +++ b/xen/arch/x86/x86_64/traps.c > > @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( > > } > > > > DEFINE_PER_CPU(struct stubs, stubs); > > + > > +#ifdef CONFIG_PV > > void lstar_enter(void); > > void cstar_enter(void); > > +#else > > +static inline void lstar_enter(void) > > +{ > > +panic("%s called", __func__); > > +} > > + > > +static inline void cstar_enter(void) > > +{ > > +panic("%s called", __func__); > > +} > > +#endif /* CONFIG_PV */ > > Do we really need two separate stubs (and two separate string literals) > here? I think it is clearer if we have two distinct messages. But I'm not too fussed either way really. If you feel strongly about this, I'm happy to change it to only one function. Wei. > > Jan > > > > ___ > 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
Re: [Xen-devel] [PATCH v3 5/5] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy
>>> On 05.11.18 at 12:16, wrote: > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -1528,6 +1528,38 @@ long arch_do_domctl( > recalculate_cpuid_policy(d); > break; > > +case XEN_DOMCTL_get_cpu_policy: > +/* Process the CPUID leaves. */ > +if ( guest_handle_is_null(domctl->u.cpu_policy.cpuid_policy) ) > +domctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES; > +else if ( (ret = x86_cpuid_copy_to_buffer( > + d->arch.cpuid, > + domctl->u.cpu_policy.cpuid_policy, > + &domctl->u.cpu_policy.nr_leaves)) ) > +break; > + > +if ( __copy_field_to_guest(u_domctl, domctl, > + u.cpu_policy.nr_leaves) ) > +{ > +ret = -EFAULT; > +break; > +} > + > +/* Process the MSR entries. */ > +if ( guest_handle_is_null(domctl->u.cpu_policy.msr_policy) ) > +domctl->u.cpu_policy.nr_msrs = MSR_MAX_SERIALISED_ENTRIES; > +else if ( (ret = x86_msr_copy_to_buffer( > + d->arch.msr, > + domctl->u.cpu_policy.msr_policy, > + &domctl->u.cpu_policy.nr_msrs)) ) > +break; > + > +if ( __copy_field_to_guest(u_domctl, domctl, > + u.cpu_policy.nr_msrs) ) > +ret = -EFAULT; Is it really worthwhile having extra code to copy back two fields, rather than just setting copyback to true? Preferably with this changed, hypervisor side Reviewed-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 v3 4/5] x86/sysctl: Implement XEN_SYSCTL_get_cpu_policy
>>> On 05.11.18 at 12:16, wrote: > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -1075,12 +1075,25 @@ struct xen_sysctl_set_parameter { > * - Default_*: Default set of features a PV or HVM guest can use. This is > * the security supported set. > */ > +struct xen_sysctl_cpu_policy { > #define XEN_SYSCTL_cpu_policy_raw 0 > #define XEN_SYSCTL_cpu_policy_host 1 > #define XEN_SYSCTL_cpu_policy_pv_max 2 > #define XEN_SYSCTL_cpu_policy_hvm_max 3 > #define XEN_SYSCTL_cpu_policy_pv_default 4 > #define XEN_SYSCTL_cpu_policy_hvm_default 5 > +uint32_t index; /* IN: Which policy to query? */ > +uint32_t nr_leaves; /* IN/OUT: Number of leaves in/written to > + * 'cpuid_policy', or the maximum number of leaves > + * if the guest handle is NULL. */ > +uint32_t nr_msrs; /* IN/OUT: Number of MSRs in/written to > + * 'msr_policy', or the maximum number of MSRs if > + * the guest handle is NULL. */ > +XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */ I'm sorry for being picky, but I (continue to) think there should be explicit padding added above here, checked to be zero on input. With this hypervisor parts Reviewed-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 v3 3/5] x86: Introduce struct cpu_policy to refer to a group of individual policies
>>> On 05.11.18 at 12:16, wrote: > This is prep work for the following patch - please refer to it as well. > > When auditing and manipulating policies, it is necessary to do so with a > complete set of policies, due to the interdependences of the contents. A > containing structure like this will allow for clearer APIs and code. > > As a first user, this structure is convenient for the mapping used by > XEN_SYSCTL_get_cpu_policy (implemented in the next patch), and for auditing > (later when XEN_DOMCTL_set_cpu_policy is implemented). > > At this point, the distinction between *_max and *_default is introduced into > the ABI. For now, *_default is mapped to *_max, but future development work > will result in *_default being a logical subset of *_max. > > Signed-off-by: Andrew Cooper > Reviewed-by: Roger Pau Monné > Reviewed-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] libx86: Introduce a helper to serialise cpuid_policy objects
>>> On 05.11.18 at 12:16, wrote: > The serialised form is made up of the leaf, subleaf and data tuple. As this > is the architectural form, it is expected not to change going forwards. > > The serialisation of the Xen/Viridian leaves isn't fully implemented yet. > It > is just enough to be bug-compatible with the current DOMCTL_set_cpuid > behaviour, but needs further hypervisor work before the toolstack can > sensibly > control these values. > > x86_cpuid_copy_to_buffer() is implemented using Xen's regular copy_to_guest > primitives, with an API-compatible memcpy() is used for the libxc half of > the > build. > > Signed-off-by: Andrew Cooper > Signed-off-by: Roger Pau Monné > Signed-off-by: Sergey Dyasli > Reviewed-by: Wei Liu Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 7/7] x86: update help string for CONFIG_HVM
>>> On 02.11.18 at 16:55, wrote: > Update text. Change "guest" to "domain" where appropriate because > "guest" doesn't include Domain 0. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
>>> On 02.11.18 at 16:55, wrote: > --- a/xen/arch/x86/x86_64/traps.c > +++ b/xen/arch/x86/x86_64/traps.c > @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( > } > > DEFINE_PER_CPU(struct stubs, stubs); > + > +#ifdef CONFIG_PV > void lstar_enter(void); > void cstar_enter(void); > +#else > +static inline void lstar_enter(void) > +{ > +panic("%s called", __func__); > +} > + > +static inline void cstar_enter(void) > +{ > +panic("%s called", __func__); > +} > +#endif /* CONFIG_PV */ Do we really need two separate stubs (and two separate string literals) here? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 6/9] viridian: separate time related enlightenment implementations...
>>> On 05.11.18 at 14:54, wrote: > Where typedefs are actually given in the spec I prefer to lift them, even > though they do not adhere to our coding style. I could but a document in the > viridian subdirectory stating this if you'd like. I don't think that's worth it. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/7] x86: make entry point code build when !CONFIG_PV
On 02/11/18 15:55, Wei Liu wrote: > Skip building x86_64/compat/entry.S and put CONFIG_PV in > x86_64/entry.S. > > Signed-off-by: Wei Liu > --- > v3: > 1. make CR4_PV32_RESTORE expand to nothing when !PV > 2. use unconditional jmp and add assertions > > v2: new > --- > xen/arch/x86/x86_64/Makefile| 2 +- > xen/arch/x86/x86_64/entry.S | 39 +- > xen/include/asm-x86/asm_defns.h | 4 +++- > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile > index f336a6a..4bfa148 100644 > --- a/xen/arch/x86/x86_64/Makefile > +++ b/xen/arch/x86/x86_64/Makefile > @@ -1,4 +1,4 @@ > -subdir-y += compat > +subdir-$(CONFIG_PV) += compat > > obj-bin-y += entry.o > obj-y += traps.o > diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S > index 9b02899..975ed6a 100644 > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -15,6 +15,18 @@ > #include > #include > > +#ifndef NDEBUG > +/* %rsp: struct cpu_user_regs */ > +#define ASSERT_INTERRUPTED_XEN_CONTEXT\ Could I suggest ASSERT_CONTEXT_IS_XEN which I think fits better in some of the cases below? > +testb $3, UREGS_cs(%rsp); \ > +jz1f; \ > +ASSERT_FAILED("INTERRUPTED XEN CONTEXT"); \ This probably wants to be: .macro ASSERT_CONTEXT_IS_XEN #ifndef NDEBUG    testb $3, UREGS_cs(%rsp);    UNLIKELY_START(nz, ASSERT_XEN_\@);    ASSERT_FAILED("CONTEXT IS XEN");    __UNLIKELY_END(ASSERT_XEN_\@); #endif .endm which will assemble in the fastpath to: testb $3, UREGS_cs(%rsp); jne jmp restore_all_xen And avoid a conditional jump in the expected path. (Code completely untested.) ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/7] x86: rearrange x86_64/entry.S
On 02/11/18 15:55, Wei Liu wrote: > @@ -553,8 +523,43 @@ ENTRY(dom_crash_sync_extable) > jmp asm_domain_crash_synchronous /* Does not return */ > .popsection > > +/* --- CODE BELOW THIS LINE (MOSTLY) NOT GUEST RELATED --- */ > + > +.text > + > +ALIGN No need for an ALIGN here. There is one inside ENTRY(). Otherwise, Reviewed-by: Andrew Cooper > +/* No special register assumptions. */ > +ENTRY(ret_from_intr) > +GET_CURRENT(bx) > +testb $3, UREGS_cs(%rsp) > +jzrestore_all_xen > +movq VCPU_domain(%rbx), %rax > +cmpb $0, DOMAIN_is_32bit_pv(%rax) > +jetest_all_events > +jmp compat_test_all_events > + > .section .text.entry, "ax", @progbits > > +ALIGN > +/* No special register assumptions. */ > +restore_all_xen: > +/* > + * Check whether we need to switch to the per-CPU page tables, in > + * case we return to late PV exit code (from an NMI or #MC). > + */ > +GET_STACK_END(bx) > +cmpb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx) > +UNLIKELY_START(ne, exit_cr3) > +mov STACK_CPUINFO_FIELD(pv_cr3)(%rbx), %rax > +mov %rax, %cr3 > +UNLIKELY_END(exit_cr3) > + > +/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ > +SPEC_CTRL_EXIT_TO_XEN_IST /* Req: %rbx=end, Clob: acd */ > + > +RESTORE_ALL adj=8 > +iretq > + > ENTRY(common_interrupt) > SAVE_ALL CLAC > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV
On 02/11/18 15:55, Wei Liu wrote: > We want Xen to crash if we hit these paths when PV is disabled. > > For syscall, we provide stubs for {l,c}star_enter which end up calling > panic. For sysenter, we initialise CS to 0 so that #GP can be raised. > > Signed-off-by: Wei Liu > --- > v3: rewrite > --- > xen/arch/x86/hvm/vmx/vmcs.c | 5 +++-- > xen/arch/x86/x86_64/traps.c | 19 +-- > 2 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c > index d9747b4..dec21d1 100644 > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -1160,8 +1160,9 @@ static int construct_vmcs(struct vcpu *v) > __vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler); > > /* Host SYSENTER CS:RIP. */ > -__vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS); > -__vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry); > +__vmwrite(HOST_SYSENTER_CS, IS_ENABLED(CONFIG_PV) ? __HYPERVISOR_CS : 0); > +__vmwrite(HOST_SYSENTER_EIP, > + IS_ENABLED(CONFIG_PV) ? (unsigned long)sysenter_entry : 0); Jun/Kevin: Given that the VMCS backing page is zeroed, is it safe to omit these VMWRITE's entirely in the !CONFIG_PV case, rather than explicitly writing 0? It would be more efficient, especially when nested, if it is safe to do so. > > /* MSR intercepts. */ > __vmwrite(VM_EXIT_MSR_LOAD_COUNT, 0); > diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c > index 27154f2..35a60d4 100644 > --- a/xen/arch/x86/x86_64/traps.c > +++ b/xen/arch/x86/x86_64/traps.c > @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline( > } > > DEFINE_PER_CPU(struct stubs, stubs); > + > +#ifdef CONFIG_PV > void lstar_enter(void); > void cstar_enter(void); > +#else > +static inline void lstar_enter(void) > +{ > +panic("%s called", __func__); \n With this fixed, Reviewed-by: Andrew Cooper > +} > + > +static inline void cstar_enter(void) > +{ > +panic("%s called", __func__); > +} > +#endif /* CONFIG_PV */ > > void subarch_percpu_traps_init(void) > { > @@ -329,8 +342,10 @@ void subarch_percpu_traps_init(void) > { > /* SYSENTER entry. */ > wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom); > -wrmsrl(MSR_IA32_SYSENTER_EIP, (unsigned long)sysenter_entry); > -wrmsr(MSR_IA32_SYSENTER_CS, __HYPERVISOR_CS, 0); > +wrmsrl(MSR_IA32_SYSENTER_EIP, > + IS_ENABLED(CONFIG_PV) ? (unsigned long)sysenter_entry : 0); > +wrmsr(MSR_IA32_SYSENTER_CS, > + IS_ENABLED(CONFIG_PV) ? __HYPERVISOR_CS : 0, 0); > } > > /* Trampoline for SYSCALL entry from compatibility mode. */ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/7] x86/domctl: rework XEN_DOMCTL_{set, get}_address_size
On 02/11/18 15:55, Wei Liu wrote: > Going through toolstack code, they are used for PV guests only. > > Tighten their access to PV only. Return -EOPNOTSUPP if they are called > on HVM guests. Rewrite the code in a pattern that makes DCE work. > > Signed-off-by: Wei Liu Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/7] x86: make traps.c build with !CONFIG_PV
On 02/11/18 15:55, Wei Liu wrote: > @@ -2078,6 +2092,7 @@ void activate_debugregs(const struct vcpu *curr) > } > } > > +#ifdef CONFIG_PV > /* > * Used by hypercalls and the emulator. > * -ENODEV => #UD > @@ -2193,6 +2208,7 @@ long set_debugreg(struct vcpu *v, unsigned int reg, > unsigned long value) > > return 0; > } > +#endif /* CONFIG_PV */ set_debugreg() doesn't really belong here. How about moving it to pv/misc-hypercalls.c ? Either way, Reviewed-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 6/9] viridian: separate time related enlightenment implementations...
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 05 November 2018 13:37 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: RE: [PATCH v2 6/9] viridian: separate time related enlightenment > implementations... > > >>> On 05.11.18 at 14:26, wrote: > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 05 November 2018 12:57 > >> > >> >>> On 31.10.18 at 13:43, wrote: > >> > --- /dev/null > >> > +++ b/xen/arch/x86/hvm/viridian/time.c > >> > @@ -0,0 +1,245 @@ > >> > > >> > +/ > >> ** > >> > * > >> > + * time.c > >> > + * > >> > + * An implementation of some time related Viridian enlightenments. > >> > + * See Microsoft's Hypervisor Top Level Functional Specification. > >> > + * for more information. > >> > + */ > >> > + > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +#include > >> > +#include > >> > + > >> > +typedef struct _HV_REFERENCE_TSC_PAGE > >> > +{ > >> > +uint32_t TscSequence; > >> > +uint32_t Reserved1; > >> > +uint64_t TscScale; > >> > +int64_t TscOffset; > >> > +uint64_t Reserved2[509]; > >> > +} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE; > >> > >> And you want to continue to have it all upper case, despite us normally > >> using such identifiers for #define-s only? > >> > > > > It's a definition lifted from the spec. so it's usual Microsoft style... > > i.e. upper case types and camel-case names. I didn't define UINT32, > INT64 and > > UINT64 types - as used by the spec - but I could do so if you'd prefer > the > > consistency. > > Oh, no, I'm not looking forward to see UINT appear. I > was rather asking whether the odd all-upper-case naming > could be changed, to be in line with our style rather than > Microsoft's. > Where typedefs are actually given in the spec I prefer to lift them, even though they do not adhere to our coding style. I could but a document in the viridian subdirectory stating this if you'd like. > >> Apart from that same remark regarding the placement of the function > >> declarations as for the previous patch. > > > > I assume you mean in the proposed new header, which is fine. > > Yes, that's what I mean. > Ok. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 129451: tolerable all pass - PUSHED
flight 129451 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/129451/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass version targeted for testing: xen 3cbecb3c6307a23d5a2227b8f48eb395131e998e baseline version: xen 2cf113891a38cc05434bc9876ffc107a990887be Last test of basis 129330 2018-11-02 15:00:29 Z2 days Testing same since 129451 2018-11-05 11:00:32 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Paul Durrant Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 2cf113891a..3cbecb3c63 3cbecb3c6307a23d5a2227b8f48eb395131e998e -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/vcpu: Remove struct vcpu allocation restriction when possible
Hi, On 05/11/2018 13:17, Andrew Cooper wrote: There is no need for struct vcpu to live below the 4G boundary for PV guests, or for HVM vcpus using HAP. Plumb struct domain into alloc_vcpu_struct() so the x86 version can query the domain's type and paging settings. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: George Dunlap CC: Tim Deegan CC: Stefano Stabellini CC: Julien Grall v2: * Plumb struct domain into alloc_vcpu_struct() --- xen/arch/arm/domain.c| 2 +- Acked-by: Julien Grall -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/vcpu: Remove struct vcpu allocation restriction when possible
On Mon, Nov 05, 2018 at 01:17:30PM +, Andrew Cooper wrote: > There is no need for struct vcpu to live below the 4G boundary for PV guests, > or for HVM vcpus using HAP. > > Plumb struct domain into alloc_vcpu_struct() so the x86 version can query the > domain's type and paging settings. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 6/9] viridian: separate time related enlightenment implementations...
>>> On 05.11.18 at 14:26, wrote: >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: 05 November 2018 12:57 >> >> >>> On 31.10.18 at 13:43, wrote: >> > --- /dev/null >> > +++ b/xen/arch/x86/hvm/viridian/time.c >> > @@ -0,0 +1,245 @@ >> > >> +/ >> ** >> > * >> > + * time.c >> > + * >> > + * An implementation of some time related Viridian enlightenments. >> > + * See Microsoft's Hypervisor Top Level Functional Specification. >> > + * for more information. >> > + */ >> > + >> > +#include >> > +#include >> > +#include >> > +#include >> > + >> > +#include >> > +#include >> > + >> > +typedef struct _HV_REFERENCE_TSC_PAGE >> > +{ >> > +uint32_t TscSequence; >> > +uint32_t Reserved1; >> > +uint64_t TscScale; >> > +int64_t TscOffset; >> > +uint64_t Reserved2[509]; >> > +} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE; >> >> And you want to continue to have it all upper case, despite us normally >> using such identifiers for #define-s only? >> > > It's a definition lifted from the spec. so it's usual Microsoft style... > i.e. upper case types and camel-case names. I didn't define UINT32, INT64 and > UINT64 types - as used by the spec - but I could do so if you'd prefer the > consistency. Oh, no, I'm not looking forward to see UINT appear. I was rather asking whether the odd all-upper-case naming could be changed, to be in line with our style rather than Microsoft's. >> Apart from that same remark regarding the placement of the function >> declarations as for the previous patch. > > I assume you mean in the proposed new header, which is fine. Yes, that's what I mean. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/vcpu: Remove struct vcpu allocation restriction when possible
>>> On 05.11.18 at 14:17, wrote: > There is no need for struct vcpu to live below the 4G boundary for PV guests, > or for HVM vcpus using HAP. > > Plumb struct domain into alloc_vcpu_struct() so the x86 version can query the > domain's type and paging settings. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 129417: regressions - FAIL
flight 129417 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/129417/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 125898 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64 7 xen-bootfail REGR. vs. 125898 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-credit2 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-i386-pvgrub 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-qemuu-nested-amd 7 xen-bootfail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-amd 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 125898 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-examine 8 reboot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-intel
Re: [Xen-devel] [PATCH v2 6/9] viridian: separate time related enlightenment implementations...
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 05 November 2018 12:57 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: Re: [PATCH v2 6/9] viridian: separate time related enlightenment > implementations... > > >>> On 31.10.18 at 13:43, wrote: > > --- /dev/null > > +++ b/xen/arch/x86/hvm/viridian/time.c > > @@ -0,0 +1,245 @@ > > > +/ > ** > > * > > + * time.c > > + * > > + * An implementation of some time related Viridian enlightenments. > > + * See Microsoft's Hypervisor Top Level Functional Specification. > > + * for more information. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +typedef struct _HV_REFERENCE_TSC_PAGE > > +{ > > +uint32_t TscSequence; > > +uint32_t Reserved1; > > +uint64_t TscScale; > > +int64_t TscOffset; > > +uint64_t Reserved2[509]; > > +} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE; > > And you want to continue to have it all upper case, despite us normally > using such identifiers for #define-s only? > It's a definition lifted from the spec. so it's usual Microsoft style... i.e. upper case types and camel-case names. I didn't define UINT32, INT64 and UINT64 types - as used by the spec - but I could do so if you'd prefer the consistency. > Apart from that same remark regarding the placement of the function > declarations as for the previous patch. > I assume you mean in the proposed new header, which is fine. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 5/9] viridian: separate interrupt related enlightenment implementations...
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 05 November 2018 12:51 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: Re: [PATCH v2 5/9] viridian: separate interrupt related > enlightenment implementations... > > >>> On 31.10.18 at 13:43, wrote: > > --- /dev/null > > +++ b/xen/arch/x86/hvm/viridian/synic.c > > @@ -0,0 +1,225 @@ > > > +/ > ** > > * > > + * synic.c > > + * > > + * An implementation of some interrupt related Viridian enlightenments. > > + * See Microsoft's Hypervisor Top Level Functional Specification. > > + * for more information. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +static void dump_vp_assist(const struct vcpu *v) > > With the name change to synic, is the _vp_ infix here and in a few > other places retained intentionally? Yes, it's meant to dump after programming the VP assist MSR, so I think the name is still correct. > > > --- a/xen/include/asm-x86/hvm/viridian.h > > +++ b/xen/include/asm-x86/hvm/viridian.h > > @@ -9,6 +9,74 @@ > > #ifndef __ASM_X86_HVM_VIRIDIAN_H__ > > #define __ASM_X86_HVM_VIRIDIAN_H__ > > > > +#include > > + > > +/* Viridian MSR numbers. */ > > +#define HV_X64_MSR_GUEST_OS_ID 0x4000 > > Are these needed outside of xen/arch/x86/hvm/viridian/ ? If not, > please introduce a local header there rather than polluting > every CU's name space. Other definitions and declaration local > to the Viridian implementation should then go there too, rather > than here. Ok. They are only needed under that sub-dir so I'll add the extra header. Paul > > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 7/9] viridian: define type for the 'virtual VP assist page'
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 05 November 2018 13:00 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: Re: [PATCH v2 7/9] viridian: define type for the 'virtual VP > assist page' > > >>> On 31.10.18 at 13:43, wrote: > > --- a/xen/include/asm-x86/hvm/viridian.h > > +++ b/xen/include/asm-x86/hvm/viridian.h > > @@ -92,7 +92,7 @@ struct viridian_vcpu > > { > > struct { > > union viridian_page_msr msr; > > -void *va; > > +void *ptr; > > Why void rather than the actual type? You can't use the typedef > (for it being local to the .c file), but you can use the union tag > here in a forward declaring manner. > Ok. I'll have a look at that. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/vcpu: Remove struct vcpu allocation restriction when possible
There is no need for struct vcpu to live below the 4G boundary for PV guests, or for HVM vcpus using HAP. Plumb struct domain into alloc_vcpu_struct() so the x86 version can query the domain's type and paging settings. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: George Dunlap CC: Tim Deegan CC: Stefano Stabellini CC: Julien Grall v2: * Plumb struct domain into alloc_vcpu_struct() --- xen/arch/arm/domain.c| 2 +- xen/arch/x86/domain.c| 7 +-- xen/common/domain.c | 2 +- xen/include/xen/domain.h | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index feebbf5..8043287 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -516,7 +516,7 @@ void dump_pageframe_info(struct domain *d) #define MAX_PAGES_PER_VCPU 1 #endif -struct vcpu *alloc_vcpu_struct(void) +struct vcpu *alloc_vcpu_struct(const struct domain *d) { struct vcpu *v; diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 943f95b..f6fe954 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -302,7 +302,7 @@ void free_domain_struct(struct domain *d) free_xenheap_page(d); } -struct vcpu *alloc_vcpu_struct(void) +struct vcpu *alloc_vcpu_struct(const struct domain *d) { struct vcpu *v; /* @@ -311,8 +311,11 @@ struct vcpu *alloc_vcpu_struct(void) * may require that the shadow CR3 points below 4GB, and hence the whole * structure must satisfy this restriction. Thus we specify MEMF_bits(32). */ +unsigned int memflags = +(is_hvm_domain(d) && paging_mode_shadow(d)) ? MEMF_bits(32) : 0; + BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); -v = alloc_xenheap_pages(0, MEMF_bits(32)); +v = alloc_xenheap_pages(0, memflags); if ( v != NULL ) clear_page(v); return v; diff --git a/xen/common/domain.c b/xen/common/domain.c index b8d4848..d6650f0 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -140,7 +140,7 @@ struct vcpu *vcpu_create( BUG_ON((!is_idle_domain(d) || vcpu_id) && d->vcpu[vcpu_id]); -if ( (v = alloc_vcpu_struct()) == NULL ) +if ( (v = alloc_vcpu_struct(d)) == NULL ) return NULL; v->domain = d; diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 5e393fd..33e4148 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -36,7 +36,7 @@ struct domain *alloc_domain_struct(void); void free_domain_struct(struct domain *d); /* Allocate/free a VCPU structure. */ -struct vcpu *alloc_vcpu_struct(void); +struct vcpu *alloc_vcpu_struct(const struct domain *d); void free_vcpu_struct(struct vcpu *v); /* Allocate/free a PIRQ structure. */ -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 9/9] viridian: introduce struct viridian_page
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 05 November 2018 13:06 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; Ian Jackson ; xen-devel > > Subject: Re: [PATCH v2 9/9] viridian: introduce struct viridian_page > > >>> On 31.10.18 at 13:43, wrote: > > The 'vp_assist' page is currently an example of a guest page which needs > to > > be kept mapped throughout the life-time of a guest, but there are other > > such examples in the specifiction [1]. This patch therefore introduces a > > generic 'viridian_page' type and converts the current > vp_assist/apic_assist > > related code to use it. Subsequent patches implementing other > enlightments > > can then also make use of it. > > This sounds generic (as in "not synic specific), yet ... > Actually Microsoft lump pretty much all the interrupt enlightenments under the synic section in the spec. so I thought it better to put them all together. > > --- a/xen/arch/x86/hvm/viridian/synic.c > > +++ b/xen/arch/x86/hvm/viridian/synic.c > > @@ -37,14 +37,13 @@ static void dump_vp_assist(const struct vcpu *v) > > v, (unsigned long)va->fields.pfn); > > } > > > > -static void initialize_vp_assist(struct vcpu *v) > > +static void initialize_guest_page(struct vcpu *v, struct viridian_page > *vp) > > { > > struct domain *d = v->domain; > > -unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn; > > +unsigned long gmfn = vp->msr.fields.pfn; > > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, > P2M_ALLOC); > > -HV_VP_ASSIST_PAGE *ptr; > > > > -ASSERT(!v->arch.hvm.viridian.vp_assist.ptr); > > +ASSERT(!vp->ptr); > > > > if ( !page ) > > goto fail; > > ... you retain the implementation here, when it would now perhaps > more logically live in viridian.c (again). Is this intentional? > Not really, it's down the patch ordering. I agree it makes more sense to put it under viridian.c now. I'll move it. > > @@ -221,9 +218,9 @@ void viridian_synic_load_vcpu_ctxt( > > { > > v->arch.hvm.viridian.vp_assist.msr.raw = ctxt->vp_assist_msr; > > if ( v->arch.hvm.viridian.vp_assist.msr.fields.enabled ) > > -initialize_vp_assist(v); > > +initialize_guest_page(v, &v->arch.hvm.viridian.vp_assist); > > > > -v->arch.hvm.viridian.vp_assist.pending = !!ctxt->vp_assist_pending; > > +v->arch.hvm.viridian.apic_assist_pending = !!ctxt- > >apic_assist_pending; > > No need for !! anymore with ... > > > --- a/xen/include/asm-x86/hvm/viridian.h > > +++ b/xen/include/asm-x86/hvm/viridian.h > > @@ -88,13 +88,16 @@ union viridian_page_msr > > } fields; > > }; > > > > +struct viridian_page > > +{ > > +union viridian_page_msr msr; > > +void *ptr; > > +}; > > + > > struct viridian_vcpu > > { > > -struct { > > -union viridian_page_msr msr; > > -void *ptr; > > -bool pending; > > -} vp_assist; > > +struct viridian_page vp_assist; > > +bool apic_assist_pending; > > ... this being (and having been) bool. > True, I'll get rid of that. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/hvm: Clean up the rest of bool_t from vm_event
Any changes required for this to go in? Regards, Alex On 29.10.2018 17:53, Alexandru Stefan ISAILA wrote: > Signed-off-by: Alexandru Isaila > Acked-by: Tamas K Lengyel > Acked-by: Jan Beulich > > --- > Changes since V1: > - Made style corrections suggested by Jan. > --- > xen/arch/x86/hvm/hvm.c| 3 ++- > xen/arch/x86/mm/mem_sharing.c | 2 +- > xen/arch/x86/mm/p2m.c | 5 ++--- > xen/common/memory.c | 2 +- > xen/common/vm_event.c | 4 ++-- > xen/include/asm-x86/mem_sharing.h | 2 +- > xen/include/xen/vm_event.h| 8 > 7 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index a140e60c9c..a8566fb87c 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -1905,7 +1905,8 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned > long gla, > if ( sharing_enomem ) > { > int rv; > -if ( (rv = mem_sharing_notify_enomem(currd, gfn, 1)) < 0 ) > + > +if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 ) > { > gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare " >"gfn %lx, ENOMEM and no helper (rc %d)\n", > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index 1dab2c8cc3..be09c8871a 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -546,7 +546,7 @@ static int audit(void) > } > > int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, > -bool_t allow_sleep) > + bool allow_sleep) > { > struct vcpu *v = current; > int rc; > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index a00a3c1bff..4bdc5e34e0 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -448,7 +448,7 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, > unsigned long gfn_l, > /* Try to unshare. If we fail, communicate ENOMEM without >* sleeping. */ > if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) > -(void)mem_sharing_notify_enomem(p2m->domain, gfn_l, 0); > +mem_sharing_notify_enomem(p2m->domain, gfn_l, false); > mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); > } > > @@ -839,8 +839,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, > mfn_t mfn, >* Foreign domains are okay to place an event as they >* won't go to sleep. */ > (void)mem_sharing_notify_enomem(p2m->domain, > -gfn_x(gfn_add(gfn, i)), > -0); > +gfn_x(gfn_add(gfn, i)), > false); > return rc; > } > omfn = p2m->get_entry(p2m, gfn_add(gfn, i), > diff --git a/xen/common/memory.c b/xen/common/memory.c > index 987395fbb3..b68efd4d9f 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -356,7 +356,7 @@ int guest_remove_page(struct domain *d, unsigned long > gmfn) > rc = mem_sharing_unshare_page(d, gmfn, 0); > if ( rc ) > { > -(void)mem_sharing_notify_enomem(d, gmfn, 0); > +mem_sharing_notify_enomem(d, gmfn, false); > goto out_put_gfn; > } > /* Maybe the mfn changed */ > diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c > index 6ffd18a448..26cfa2c605 100644 > --- a/xen/common/vm_event.c > +++ b/xen/common/vm_event.c > @@ -496,7 +496,7 @@ static int vm_event_wait_slot(struct vm_event_domain *ved) > return rc; > } > > -bool_t vm_event_check_ring(struct vm_event_domain *ved) > +bool vm_event_check_ring(struct vm_event_domain *ved) > { > return (ved && ved->ring_page); > } > @@ -514,7 +514,7 @@ bool_t vm_event_check_ring(struct vm_event_domain *ved) >* >*/ > int __vm_event_claim_slot(struct domain *d, struct vm_event_domain *ved, > - bool_t allow_sleep) > + bool allow_sleep) > { > if ( !vm_event_check_ring(ved) ) > return -EOPNOTSUPP; > diff --git a/xen/include/asm-x86/mem_sharing.h > b/xen/include/asm-x86/mem_sharing.h > index 91908924df..0e77b7d935 100644 > --- a/xen/include/asm-x86/mem_sharing.h > +++ b/xen/include/asm-x86/mem_sharing.h > @@ -84,7 +84,7 @@ static inline int mem_sharing_unshare_page(struct domain *d, >* then it's the same as a foreign domain. >*/ > int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, > -bool_t allow_sleep); > + bool allow_sleep); > int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg); > int mem_sharing_domctl(struct domain *d, >
Re: [Xen-devel] [PATCH v2 9/9] viridian: introduce struct viridian_page
>>> On 31.10.18 at 13:43, wrote: > The 'vp_assist' page is currently an example of a guest page which needs to > be kept mapped throughout the life-time of a guest, but there are other > such examples in the specifiction [1]. This patch therefore introduces a > generic 'viridian_page' type and converts the current vp_assist/apic_assist > related code to use it. Subsequent patches implementing other enlightments > can then also make use of it. This sounds generic (as in "not synic specific), yet ... > --- a/xen/arch/x86/hvm/viridian/synic.c > +++ b/xen/arch/x86/hvm/viridian/synic.c > @@ -37,14 +37,13 @@ static void dump_vp_assist(const struct vcpu *v) > v, (unsigned long)va->fields.pfn); > } > > -static void initialize_vp_assist(struct vcpu *v) > +static void initialize_guest_page(struct vcpu *v, struct viridian_page *vp) > { > struct domain *d = v->domain; > -unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn; > +unsigned long gmfn = vp->msr.fields.pfn; > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > -HV_VP_ASSIST_PAGE *ptr; > > -ASSERT(!v->arch.hvm.viridian.vp_assist.ptr); > +ASSERT(!vp->ptr); > > if ( !page ) > goto fail; ... you retain the implementation here, when it would now perhaps more logically live in viridian.c (again). Is this intentional? > @@ -221,9 +218,9 @@ void viridian_synic_load_vcpu_ctxt( > { > v->arch.hvm.viridian.vp_assist.msr.raw = ctxt->vp_assist_msr; > if ( v->arch.hvm.viridian.vp_assist.msr.fields.enabled ) > -initialize_vp_assist(v); > +initialize_guest_page(v, &v->arch.hvm.viridian.vp_assist); > > -v->arch.hvm.viridian.vp_assist.pending = !!ctxt->vp_assist_pending; > +v->arch.hvm.viridian.apic_assist_pending = !!ctxt->apic_assist_pending; No need for !! anymore with ... > --- a/xen/include/asm-x86/hvm/viridian.h > +++ b/xen/include/asm-x86/hvm/viridian.h > @@ -88,13 +88,16 @@ union viridian_page_msr > } fields; > }; > > +struct viridian_page > +{ > +union viridian_page_msr msr; > +void *ptr; > +}; > + > struct viridian_vcpu > { > -struct { > -union viridian_page_msr msr; > -void *ptr; > -bool pending; > -} vp_assist; > +struct viridian_page vp_assist; > +bool apic_assist_pending; ... this being (and having been) bool. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
On Mon, Nov 5, 2018 at 6:29 PM Rishi <2rushike...@gmail.com> wrote: > Yes, I'm taking out patches from 4.4 and actually do have a working 4.9 > kernel along with blktap. Tested networking and disk IO in it. > > There are roughly 415 patches to 4.4 out of which some ~210+ are already > applied in 4.9 and ~220+ are already applied in 4.14. I dont have numbers > for 4.19 yet. > > Essentially I'm down to single digit number of patches atm to have a > working setup for kernel 4.9. I know there would be mishaps since I'm not > applying all patches but my experiment is to see how close can we stay near > mainline kernel + what can be the patches that kernel.org can accept. > > > > On Mon, Nov 5, 2018 at 6:19 PM Wei Liu wrote: > >> I forgot to say: please don't top-post. >> >> On Mon, Nov 05, 2018 at 06:00:10PM +0530, Rishi wrote: >> > I'm using a XenServer Host and XCP-NG on it as HVM. I used xencons=tty >> > console=ttyS0 on XCP-NG dom0 kernel line, to obtain serial console. >> > I'm working on to build a more recent dom0 kernel for improved support >> of >> > Ceph in XenServer/XCP-NG. >> >> This is an interesting setup. I don't think you can expect to just drop >> in a new kernel to XenServer/XCP-NG and then it works flawlessly. What >> did you do to the patch queue XenServer carries for 4.4? >> >> Also, have you got a working baseline? I.e. did the stock 4.4 kernel >> work? >> >> Wei. >> >> > >> > >> > >> > On Mon, Nov 5, 2018 at 5:28 PM Wei Liu wrote: >> > >> > > On Mon, Nov 05, 2018 at 05:18:43PM +0530, Rishi wrote: >> > > > Yes, I'm running it in a HVM domU for development purpose. >> > > >> > > What is your exact setup? >> > > >> > > Wei. >> > > >> > > > >> > > > On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: >> > > > >> > > > > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: >> > > > > > Alright, I got the serial console and following is the crash >> log. >> > > Thank >> > > > > you >> > > > > > for pointing that out. >> > > > > > >> > > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! >> > > > > > [ksoftirqd/2:22] >> > > > > > >> > > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung >> tasks >> > > > > > >> > > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G >> > > > > > L4.19.1 >> > > > > > #1 >> > > > > > >> > > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 >> > > > > 12/12/2016 >> > > > > >> > > > > Is this serial log from the host? It says it is running as a HVM >> DomU. >> > > > > Maybe you have mistaken guest serial log with host serial log? >> > > > > >> > > > > This indicates your machine runs XenServer, which has its own >> patch >> > > > > queues on top of upstream Xen. You may also want to report to >> xs-devel >> > > > > mailing list. >> > > > > >> > > > > Wei. >> > > > > >> > > >> > Sorry, I'll take care of top post from onwards. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 7/9] viridian: define type for the 'virtual VP assist page'
>>> On 31.10.18 at 13:43, wrote: > --- a/xen/include/asm-x86/hvm/viridian.h > +++ b/xen/include/asm-x86/hvm/viridian.h > @@ -92,7 +92,7 @@ struct viridian_vcpu > { > struct { > union viridian_page_msr msr; > -void *va; > +void *ptr; Why void rather than the actual type? You can't use the typedef (for it being local to the .c file), but you can use the union tag here in a forward declaring manner. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
Yes, I'm taking out patches from 4.4 and actually do have a working 4.9 kernel along with blktap. Tested networking and disk IO in it. There are roughly 415 patches to 4.4 out of which some ~210+ are already applied in 4.9 and ~220+ are already applied in 4.14. I dont have numbers for 4.19 yet. Essentially I'm down to single digit number of patches atm to have a working setup for kernel 4.9. I know there would be mishaps since I'm not applying all patches but my experiment is to see how close can we stay near mainline kernel + what can be the patches that kernel.org can accept. On Mon, Nov 5, 2018 at 6:19 PM Wei Liu wrote: > I forgot to say: please don't top-post. > > On Mon, Nov 05, 2018 at 06:00:10PM +0530, Rishi wrote: > > I'm using a XenServer Host and XCP-NG on it as HVM. I used xencons=tty > > console=ttyS0 on XCP-NG dom0 kernel line, to obtain serial console. > > I'm working on to build a more recent dom0 kernel for improved support of > > Ceph in XenServer/XCP-NG. > > This is an interesting setup. I don't think you can expect to just drop > in a new kernel to XenServer/XCP-NG and then it works flawlessly. What > did you do to the patch queue XenServer carries for 4.4? > > Also, have you got a working baseline? I.e. did the stock 4.4 kernel > work? > > Wei. > > > > > > > > > On Mon, Nov 5, 2018 at 5:28 PM Wei Liu wrote: > > > > > On Mon, Nov 05, 2018 at 05:18:43PM +0530, Rishi wrote: > > > > Yes, I'm running it in a HVM domU for development purpose. > > > > > > What is your exact setup? > > > > > > Wei. > > > > > > > > > > > On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: > > > > > > > > > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: > > > > > > Alright, I got the serial console and following is the crash log. > > > Thank > > > > > you > > > > > > for pointing that out. > > > > > > > > > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! > > > > > > [ksoftirqd/2:22] > > > > > > > > > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung tasks > > > > > > > > > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G > > > > > > L4.19.1 > > > > > > #1 > > > > > > > > > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 > > > > > 12/12/2016 > > > > > > > > > > Is this serial log from the host? It says it is running as a HVM > DomU. > > > > > Maybe you have mistaken guest serial log with host serial log? > > > > > > > > > > This indicates your machine runs XenServer, which has its own patch > > > > > queues on top of upstream Xen. You may also want to report to > xs-devel > > > > > mailing list. > > > > > > > > > > Wei. > > > > > > > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 6/9] viridian: separate time related enlightenment implementations...
>>> On 31.10.18 at 13:43, wrote: > --- /dev/null > +++ b/xen/arch/x86/hvm/viridian/time.c > @@ -0,0 +1,245 @@ > +/** > * > + * time.c > + * > + * An implementation of some time related Viridian enlightenments. > + * See Microsoft's Hypervisor Top Level Functional Specification. > + * for more information. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +typedef struct _HV_REFERENCE_TSC_PAGE > +{ > +uint32_t TscSequence; > +uint32_t Reserved1; > +uint64_t TscScale; > +int64_t TscOffset; > +uint64_t Reserved2[509]; > +} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE; And you want to continue to have it all upper case, despite us normally using such identifiers for #define-s only? Apart from that same remark regarding the placement of the function declarations as for the previous patch. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 5/9] viridian: separate interrupt related enlightenment implementations...
>>> On 31.10.18 at 13:43, wrote: > --- /dev/null > +++ b/xen/arch/x86/hvm/viridian/synic.c > @@ -0,0 +1,225 @@ > +/** > * > + * synic.c > + * > + * An implementation of some interrupt related Viridian enlightenments. > + * See Microsoft's Hypervisor Top Level Functional Specification. > + * for more information. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +static void dump_vp_assist(const struct vcpu *v) With the name change to synic, is the _vp_ infix here and in a few other places retained intentionally? > --- a/xen/include/asm-x86/hvm/viridian.h > +++ b/xen/include/asm-x86/hvm/viridian.h > @@ -9,6 +9,74 @@ > #ifndef __ASM_X86_HVM_VIRIDIAN_H__ > #define __ASM_X86_HVM_VIRIDIAN_H__ > > +#include > + > +/* Viridian MSR numbers. */ > +#define HV_X64_MSR_GUEST_OS_ID 0x4000 Are these needed outside of xen/arch/x86/hvm/viridian/ ? If not, please introduce a local header there rather than polluting every CU's name space. Other definitions and declaration local to the Viridian implementation should then go there too, rather than here. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
I forgot to say: please don't top-post. On Mon, Nov 05, 2018 at 06:00:10PM +0530, Rishi wrote: > I'm using a XenServer Host and XCP-NG on it as HVM. I used xencons=tty > console=ttyS0 on XCP-NG dom0 kernel line, to obtain serial console. > I'm working on to build a more recent dom0 kernel for improved support of > Ceph in XenServer/XCP-NG. This is an interesting setup. I don't think you can expect to just drop in a new kernel to XenServer/XCP-NG and then it works flawlessly. What did you do to the patch queue XenServer carries for 4.4? Also, have you got a working baseline? I.e. did the stock 4.4 kernel work? Wei. > > > > On Mon, Nov 5, 2018 at 5:28 PM Wei Liu wrote: > > > On Mon, Nov 05, 2018 at 05:18:43PM +0530, Rishi wrote: > > > Yes, I'm running it in a HVM domU for development purpose. > > > > What is your exact setup? > > > > Wei. > > > > > > > > On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: > > > > > > > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: > > > > > Alright, I got the serial console and following is the crash log. > > Thank > > > > you > > > > > for pointing that out. > > > > > > > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! > > > > > [ksoftirqd/2:22] > > > > > > > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung tasks > > > > > > > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G > > > > > L4.19.1 > > > > > #1 > > > > > > > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 > > > > 12/12/2016 > > > > > > > > Is this serial log from the host? It says it is running as a HVM DomU. > > > > Maybe you have mistaken guest serial log with host serial log? > > > > > > > > This indicates your machine runs XenServer, which has its own patch > > > > queues on top of upstream Xen. You may also want to report to xs-devel > > > > mailing list. > > > > > > > > Wei. > > > > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
I'm using a XenServer Host and XCP-NG on it as HVM. I used xencons=tty console=ttyS0 on XCP-NG dom0 kernel line, to obtain serial console. I'm working on to build a more recent dom0 kernel for improved support of Ceph in XenServer/XCP-NG. On Mon, Nov 5, 2018 at 5:28 PM Wei Liu wrote: > On Mon, Nov 05, 2018 at 05:18:43PM +0530, Rishi wrote: > > Yes, I'm running it in a HVM domU for development purpose. > > What is your exact setup? > > Wei. > > > > > On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: > > > > > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: > > > > Alright, I got the serial console and following is the crash log. > Thank > > > you > > > > for pointing that out. > > > > > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! > > > > [ksoftirqd/2:22] > > > > > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung tasks > > > > > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G > > > > L4.19.1 > > > > #1 > > > > > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 > > > 12/12/2016 > > > > > > Is this serial log from the host? It says it is running as a HVM DomU. > > > Maybe you have mistaken guest serial log with host serial log? > > > > > > This indicates your machine runs XenServer, which has its own patch > > > queues on top of upstream Xen. You may also want to report to xs-devel > > > mailing list. > > > > > > Wei. > > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
On Mon, Nov 05, 2018 at 05:18:43PM +0530, Rishi wrote: > Yes, I'm running it in a HVM domU for development purpose. What is your exact setup? Wei. > > On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: > > > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: > > > Alright, I got the serial console and following is the crash log. Thank > > you > > > for pointing that out. > > > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! > > > [ksoftirqd/2:22] > > > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung tasks > > > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G > > > L4.19.1 > > > #1 > > > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 > > 12/12/2016 > > > > Is this serial log from the host? It says it is running as a HVM DomU. > > Maybe you have mistaken guest serial log with host serial log? > > > > This indicates your machine runs XenServer, which has its own patch > > queues on top of upstream Xen. You may also want to report to xs-devel > > mailing list. > > > > Wei. > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC 14/16] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit)
Hi Stefano, On 02/11/2018 23:44, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: With the recent changes, a P2M entry may be populated but may as not valid. In some situation, it would be useful to know whether the entry has been marked available to guest in order to perform a specific action. So extend p2m_get_entry to return the value of bit[0] (valid bit). Signed-off-by: Julien Grall --- xen/arch/arm/mem_access.c | 6 +++--- xen/arch/arm/p2m.c| 20 xen/include/asm-arm/p2m.h | 3 ++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c index 9239bdf323..f434510b2a 100644 --- a/xen/arch/arm/mem_access.c +++ b/xen/arch/arm/mem_access.c @@ -70,7 +70,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, * No setting was found in the Radix tree. Check if the * entry exists in the page-tables. */ -mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL); +mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) return -ESRCH; @@ -199,7 +199,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, * We had a mem_access permission limiting the access, but the page type * could also be limiting, so we need to check that as well. */ -mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL); +mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) goto err; @@ -405,7 +405,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr, gfn = gfn_next_boundary(gfn, order) ) { p2m_type_t t; -mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order); +mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order, NULL); if ( !mfn_eq(mfn, INVALID_MFN) ) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 12b459924b..df6b48d73b 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -306,10 +306,14 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, * * If the entry is not present, INVALID_MFN will be returned and the * page_order will be set according to the order of the invalid range. + * + * valid will contain the value of bit[0] (e.g valid bit) of the + * entry. */ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, p2m_type_t *t, p2m_access_t *a, -unsigned int *page_order) +unsigned int *page_order, +bool *valid) { paddr_t addr = gfn_to_gaddr(gfn); unsigned int level = 0; @@ -317,6 +321,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, int rc; mfn_t mfn = INVALID_MFN; p2m_type_t _t; +bool _valid; /* Convenience aliases */ const unsigned int offsets[4] = { @@ -334,6 +339,10 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, *t = p2m_invalid; +/* Allow valid to be NULL */ +valid = valid?: &_valid; +*valid = false; Why not a simple: if ( valid ) *valid = false; especially given that you do the same if ( valid ) check below. In fact, it doesn' look like we need _valid? I thought I dropped the if ( valid ) below. I would actually prefer to keep _valid and avoid using if ( ... ) everywhere. This makes the code slightly easier to follow. /* XXX: Check if the mapping is lower than the mapped gfn */ /* This gfn is higher than the highest the p2m map currently holds */ @@ -379,6 +388,9 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, * to the GFN. */ mfn = mfn_add(mfn, gfn_x(gfn) & ((1UL << level_orders[level]) - 1)); + +if ( valid ) +*valid = lpae_is_valid(entry); } out_unmap: @@ -397,7 +409,7 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t) struct p2m_domain *p2m = p2m_get_hostp2m(d); p2m_read_lock(p2m); -mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL); +mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL, NULL); p2m_read_unlock(p2m); return mfn; @@ -1464,7 +1476,7 @@ int relinquish_p2m_mapping(struct domain *d) for ( ; gfn_x(start) < gfn_x(end); start = gfn_next_boundary(start, order) ) { -mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order); +mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL); count++; /* @@ -1527,7 +1539,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) for ( ; gfn_x(start) < gfn_x(end); start = next_gfn ) { -mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order); +mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL); next_gfn = gfn_next_boundary(start, order); diff --git a/xen/include/asm-arm/p2m.h
Re: [Xen-devel] [RFC 09/16] xen/arm: p2m: Introduce a function to resolve translation fault
Hi, On 02/11/2018 23:27, Stefano Stabellini wrote: On Mon, 8 Oct 2018, Julien Grall wrote: Currently a Stage-2 translation fault could happen: 1) MMIO emulation 2) When the page-tables is been updated using Break-Before-Make ^ have 3) Page not mapped A follow-up patch will re-purpose the valid bit in an entry to generate translation fault. This would be used to do an action on each entries to ^entry track page used for a given period. ^pages A new function is introduced to try to resolve a translation fault. This will include 2) and the new way to generate fault explained above. I can see the code does what you describe, but I don't understand why we are doing this. What is missing in the commit message is the explanation of the relationship between the future goal of repurposing the valid bit and the introduction of a function to handle Break-Before-Make stage2 faults. Does it fix an issue with Break-Before-Make that we currently have? Or it becomes needed due to the repurposing of valid? If so, why? This does not fix any issue with BBM. The valid bit adds a 4th reasons for translation fault. Both BBM and the valid bit will require to walk the page-tables. For the valid bit, we will need to walk the page-table in order to fixup the entry (i.e set valid bit). We also can't use p2m_lookup(...) has it only tell you the mapping exists, the valid bit may still not be set. So we need to provide a new helper to walk the page-table and fixup an entry. To avoid invalidating all the page-tables entries in one go. It is possible to invalidate the top-level table and then on trap invalidate the table one-level down. This will be repeated until a block/page entry has been reached. At the moment, there are no action done when reaching a block/page entry but setting the valid bit to 1. Signed-off-by: Julien Grall --- xen/arch/arm/p2m.c | 127 +++ xen/arch/arm/traps.c | 7 +-- 2 files changed, 131 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index ec956bc151..af445d3313 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1043,6 +1043,133 @@ int p2m_set_entry(struct p2m_domain *p2m, return rc; } +/* Invalidate all entries in the table. The p2m should be write locked. */ +static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) +{ +lpae_t *table; +unsigned int i; + +ASSERT(p2m_is_write_locked(p2m)); + +table = map_domain_page(mfn); + +for ( i = 0; i < LPAE_ENTRIES; i++ ) +{ +lpae_t pte = table[i]; + +pte.p2m.valid = 0; + +p2m_write_pte(&table[i], pte, p2m->clean_pte); +} + +unmap_domain_page(table); + +p2m->need_flush = true; +} + +bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn) +{ +struct p2m_domain *p2m = p2m_get_hostp2m(d); +unsigned int level = 0; +bool resolved = false; +lpae_t entry, *table; +paddr_t addr = gfn_to_gaddr(gfn); + +/* Convenience aliases */ +const unsigned int offsets[4] = { +zeroeth_table_offset(addr), +first_table_offset(addr), +second_table_offset(addr), +third_table_offset(addr) +}; + +p2m_write_lock(p2m); + +/* This gfn is higher than the highest the p2m map currently holds */ +if ( gfn_x(gfn) > gfn_x(p2m->max_mapped_gfn) ) +goto out; + +table = p2m_get_root_pointer(p2m, gfn); +/* + * The table should always be non-NULL because the gfn is below + * p2m->max_mapped_gfn and the root table pages are always present. + */ +BUG_ON(table == NULL); + +/* + * Go down the page-tables until an entry has the valid bit unset or + * a block/page entry has been hit. + */ +for ( level = P2M_ROOT_LEVEL; level <= 3; level++ ) +{ +int rc; + +entry = table[offsets[level]]; + +if ( level == 3 ) +break; + +/* Stop as soon as we hit an entry with the valid bit unset. */ +if ( !lpae_is_valid(entry) ) +break; + +rc = p2m_next_level(p2m, true, level, &table, offsets[level]); +if ( rc == GUEST_TABLE_MAP_FAILED ) +goto out_unmap; +else if ( rc != GUEST_TABLE_NORMAL_PAGE ) why not rc == GUEST_TABLE_SUPER_PAGE? The logic has been taken from p2m_get_entry(). It makes sense to use != here as you only want to continue the loop if you are on a table. So it is clearer why you continue. +break; +} + +/* + * If the valid bit of the entry is set, it means someone was playing with + * the Stage-2 page table. Nothing to do and mark the fault as resolved. + */ +if ( lpae_is_valid(entry) ) +{ +resolved = true; +goto out_unmap; +} + +/* + * The valid bit is unset. If the entry is still n
Re: [Xen-devel] Dom0 kernel 4.14 with SMP randomly crashing
Yes, I'm running it in a HVM domU for development purpose. On Mon, Nov 5, 2018 at 5:11 PM Wei Liu wrote: > On Mon, Nov 05, 2018 at 04:58:35PM +0530, Rishi wrote: > > Alright, I got the serial console and following is the crash log. Thank > you > > for pointing that out. > > > > [ 133.594852] watchdog: BUG: soft lockup - CPU#2 stuck for 22s! > > [ksoftirqd/2:22] > > > > [ 133.599232] Kernel panic - not syncing: softlockup: hung tasks > > > > [ 133.602275] CPU: 2 PID: 22 Comm: ksoftirqd/2 Tainted: G > > L4.19.1 > > #1 > > > > [ 133.606620] Hardware name: Xen HVM domU, BIOS 4.4.1-xs132257 > 12/12/2016 > > Is this serial log from the host? It says it is running as a HVM DomU. > Maybe you have mistaken guest serial log with host serial log? > > This indicates your machine runs XenServer, which has its own patch > queues on top of upstream Xen. You may also want to report to xs-devel > mailing list. > > Wei. > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel