Re: [Xen-devel] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Mon, Jul 16, 2018 at 04:12:49PM -0700, Andrew Morton wrote: > On Mon, 16 Jul 2018 13:50:58 +0200 Michal Hocko wrote: > > > From: Michal Hocko > > > > There are several blockable mmu notifiers which might sleep in > > mmu_notifier_invalidate_range_start and that is a problem for the > > oom_reaper because it needs to guarantee a forward progress so it cannot > > depend on any sleepable locks. > > > > Currently we simply back off and mark an oom victim with blockable mmu > > notifiers as done after a short sleep. That can result in selecting a > > new oom victim prematurely because the previous one still hasn't torn > > its memory down yet. > > > > We can do much better though. Even if mmu notifiers use sleepable locks > > there is no reason to automatically assume those locks are held. > > Moreover majority of notifiers only care about a portion of the address > > space and there is absolutely zero reason to fail when we are unmapping an > > unrelated range. Many notifiers do really block and wait for HW which is > > harder to handle and we have to bail out though. > > > > This patch handles the low hanging fruid. > > __mmu_notifier_invalidate_range_start > > gets a blockable flag and callbacks are not allowed to sleep if the > > flag is set to false. This is achieved by using trylock instead of the > > sleepable lock for most callbacks and continue as long as we do not > > block down the call chain. > > I assume device driver developers are wondering "what does this mean > for me". As I understand it, the only time they will see > blockable==false is when their driver is being called in response to an > out-of-memory condition, yes? So it is a very rare thing. I can't say for everyone, but at least for me (mlx5), it is not rare event. I'm seeing OOM very often while I'm running my tests in low memory VMs. Thanks > > Any suggestions regarding how the driver developers can test this code > path? I don't think we presently have a way to fake an oom-killing > event? Perhaps we should add such a thing, given the problems we're > having with that feature. signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 125179: regressions - FAIL
flight 125179 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/125179/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-i386-libvirt6 libvirt-buildfail REGR. vs. 123814 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 123814 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a version targeted for testing: libvirt 19e9d92b27abe210d54e617069fc80caa2af013e baseline version: libvirt 076a2b409667dd9f716a2a2085e1ffea9d58fe8b Last test of basis 123814 2018-06-05 04:19:23 Z 42 days Failing since123840 2018-06-06 04:19:28 Z 41 days 30 attempts Testing same since 125179 2018-07-15 10:52:31 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani Anya Harter Bjoern Walk Bobo Du Boris Fiuczynski Brijesh Singh Changkuo Shi Chen Hanxiao Christian Ehrhardt Cole Robinson Daniel Nicoletti Daniel P. Berrangé Daniel Veillard Eric Blake Erik Skultety Fabiano Fidêncio Filip Alac Han Han intrigeri intrigeri Jamie Strandboge Jie Wang Jiri Denemark John Ferlan Julio Faracco Ján Tomko Kashyap Chamarthy Katerina Koukiou Laine Stump Laszlo Ersek Luyao Huang Marc Hartmayer Marcos Paulo de Souza Martin Kletzander Michal Privoznik Michal Prívozník Pavel Hrdina Peter Krempa Pino Toscano Radostin Stoyanov Ramy Elkest ramyelkest Richard W.M. Jones Roman Bogorodskiy Shichangkuo Simon Kobyda Stefan Bader Stefan Berger Sukrit Bhatnagar w00251574 Weilun Zhu jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairblocked test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw blocked
[Xen-devel] [xen-unstable test] 125178: tolerable FAIL - PUSHED
flight 125178 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/125178/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125152 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125152 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125152 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125152 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125152 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125152 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125152 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125152 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 125152 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-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-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-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 e3f667bc5f51d0aa44357a64ca134cd952679c81 baseline version: xen 41cb2db62627a7438d938aae487550c3f4acb1da Last test of basis 125152 2018-07-13 10:02:25 Z3 days Testing same since 125178 2018-07-15 10:20:11 Z1 days1 attempts People who touched revisions under test: Doug Goldstein Jan Beulich Roger Pau Monne Roger Pau Monné Wei Liu jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64
[Xen-devel] [ovmf test] 125255: all pass - PUSHED
flight 125255 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/125255/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 60ee3bd8dbe70189cab18af733c42187c9b317c7 baseline version: ovmf 4c6d0de7bad46fc15fd34d394dffda3766e3a6a1 Last test of basis 125241 2018-07-16 15:11:08 Z0 days Testing same since 125255 2018-07-17 00:10:40 Z0 days1 attempts People who touched revisions under test: Chao Zhang Zhang, Chao B 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 4c6d0de7ba..60ee3bd8db 60ee3bd8dbe70189cab18af733c42187c9b317c7 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/11] x86/svm Fixes and cleanup to svm_inject_event()
On 07/16/2018 09:33 AM, Andrew Cooper wrote: > On 06/06/18 14:37, Jan Beulich wrote: >> >>> On 04.06.18 at 15:59, wrote: >>> * State adjustments (and debug tracing) for #DB/#BP/#PF should not be done >>>for `int $n` instructions. Updates to %cr2 occur even if the exception >>>combines to #DF. >>> * Don't opencode DR_STEP when updating %dr6. >>> * Simplify the logic for calling svm_emul_swint_injection() as in the >>> common >>>case, every condition needs checking. >>> * Fix comments which have become stale as code has moved between >>> components. >>> >>> Signed-off-by: Andrew Cooper >> Reviewed-by: Jan Beulich >> >> > Ping SVM? > > Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.14 test] 125175: tolerable FAIL - PUSHED
flight 125175 linux-4.14 real [real] http://logs.test-lab.xenproject.org/osstest/logs/125175/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-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-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: linux1e92e813554a93741666e9f378a83d70405b9076 baseline version: linux5893f4c3fb784f48c020d2637c129a45da7be39e Last test of basis 125079 2018-07-10 16:42:10 Z6 days Testing same since 125148 2018-07-13 07:11:20 Z3 days2 attempts People who touched revisions under test: Alex Deucher Alex Williamson Andrew Morton Boris Brezillon Brad Love Cannon Matthews Changbin Du Christian Borntraeger Dan Carpenter Dan Williams Daniel Rosenberg Darrick J. Wong Dave Airlie Dave Jiang David Disseldorp Devin Heitmueller Douglas Gilbert Greg Kroah-Hartman Gustavo A. R. Silva Hans Verkuil Ilya Dryomov Ingo Molnar Jaegeuk Kim Jann Horn
Re: [Xen-devel] [PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Mon, 16 Jul 2018 13:50:58 +0200 Michal Hocko wrote:
> From: Michal Hocko
>
> There are several blockable mmu notifiers which might sleep in
> mmu_notifier_invalidate_range_start and that is a problem for the
> oom_reaper because it needs to guarantee a forward progress so it cannot
> depend on any sleepable locks.
>
> Currently we simply back off and mark an oom victim with blockable mmu
> notifiers as done after a short sleep. That can result in selecting a
> new oom victim prematurely because the previous one still hasn't torn
> its memory down yet.
>
> We can do much better though. Even if mmu notifiers use sleepable locks
> there is no reason to automatically assume those locks are held.
> Moreover majority of notifiers only care about a portion of the address
> space and there is absolutely zero reason to fail when we are unmapping an
> unrelated range. Many notifiers do really block and wait for HW which is
> harder to handle and we have to bail out though.
>
> This patch handles the low hanging fruid.
> __mmu_notifier_invalidate_range_start
> gets a blockable flag and callbacks are not allowed to sleep if the
> flag is set to false. This is achieved by using trylock instead of the
> sleepable lock for most callbacks and continue as long as we do not
> block down the call chain.
I assume device driver developers are wondering "what does this mean
for me". As I understand it, the only time they will see
blockable==false is when their driver is being called in response to an
out-of-memory condition, yes? So it is a very rare thing.
Any suggestions regarding how the driver developers can test this code
path? I don't think we presently have a way to fake an oom-killing
event? Perhaps we should add such a thing, given the problems we're
having with that feature.
> I think we can improve that even further because there is a common
> pattern to do a range lookup first and then do something about that.
> The first part can be done without a sleeping lock in most cases AFAICS.
>
> The oom_reaper end then simply retries if there is at least one notifier
> which couldn't make any progress in !blockable mode. A retry loop is
> already implemented to wait for the mmap_sem and this is basically the
> same thing.
>
> ...
>
> +static inline int mmu_notifier_invalidate_range_start_nonblock(struct
> mm_struct *mm,
> + unsigned long start, unsigned long end)
> +{
> + int ret = 0;
> + if (mm_has_notifiers(mm))
> + ret = __mmu_notifier_invalidate_range_start(mm, start, end,
> false);
> +
> + return ret;
> }
nit,
{
if (mm_has_notifiers(mm))
return __mmu_notifier_invalidate_range_start(mm, start, end,
false);
return 0;
}
would suffice.
>
> ...
>
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3074,7 +3074,7 @@ void exit_mmap(struct mm_struct *mm)
>* reliably test it.
>*/
> mutex_lock(_lock);
> - __oom_reap_task_mm(mm);
> + (void)__oom_reap_task_mm(mm);
> mutex_unlock(_lock);
What does this do?
> set_bit(MMF_OOM_SKIP, >flags);
>
> ...
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.9-testing test] 125171: regressions - FAIL
flight 125171 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/125171/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 124328 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail in 125144 REGR. vs. 124248 Tests which are failing intermittently (not blocking): test-arm64-arm64-xl-credit2 7 xen-boot fail pass in 125144 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 125144 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail blocked in 124328 test-amd64-i386-xl-qemuu-ws16-amd64 18 guest-start/win.repeat fail blocked in 124328 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail in 125144 like 124248 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail in 125144 like 124328 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 125144 like 124328 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail in 125144 like 124328 test-arm64-arm64-xl-credit2 13 migrate-support-check fail in 125144 never pass test-arm64-arm64-xl-credit2 14 saverestore-support-check fail in 125144 never pass test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail like 124248 test-amd64-amd64-xl-qemuu-ws16-amd64 14 guest-localmigratefail like 124248 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 124248 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 124328 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 124328 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 124328 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail like 124328 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen f5c692acb81219d817e97ea8499f44f9f2764af5 baseline version: xen 238007d6fae9447bf5e8e73d67ae9fb844e7ff2a Last test of basis 124328
Re: [Xen-devel] [xen-4.8-testing test] 124100: regressions - FAIL
On 06/13/2018 05:18 AM, Ian Jackson wrote: Jim: please read down to where I discuss test-amd64-amd64-libvirt-pair. If you have any insight I'd appreciate it. Let me know if you want me to preserve the logs, which will otherwise expire in a few weeks. Whoa, sorry for the delay. This mail found a dumb bug in my filter for xen-devel mail. test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail pass in 123701 From the log: 2018-06-12 20:59:40 Z executing ssh ... root@172.16.144.61 virsh migrate --live debian.guest.osstest xen+ssh://joubertin0 error: Timed out during operation: cannot acquire state change lock 2018-06-12 21:00:16 Z command nonzero waitstatus 256: [..] The libvirt libxl logs seem to show libxl doing a successful migration. With the long delay, I'm afraid the logs have expired. Do you still see the problem? All the recent runs seem to be plagued with libvirt's change to require GnuTLS https://libvirt.org/git/?p=libvirt.git;a=commit;h=60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24 Looking at the logs I see this: 2018-06-12 21:00:16.784+: 3507: warning : libxlDomainObjBeginJob:151 : Cannot start job (modify) for domain debian.guest.osstest; current job is (modify) owned by (24947) That job number looks like it's about right for a pid, but I think it must be a thread because it doesn't show up in the ps output. Likely a libvirtd worker thread doing something that requires modifying the state of virDomainObj. I did see this: Jun 12 21:00:20 joubertin0 logger: /etc/xen/scripts/vif-bridge: iptables setup failed. This may affect guest networking. but that seems to be after the failure. A wild guess, but is it possible thread 24947 is running a domain create operation, which includes executing vif-bridge, that is taking longer than expected to complete? I don't have an explanation. I don't really know what this lock is. It's a lock that serializes domain state modifications (changing virDomainObj). Wait time for the lock is currently hardcoded to 30sec. The thread emitting the warning surpassed the timeout, waiting for 24947 to finish whatever it was doing. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/21] xen/arm: extend device tree based multiboot protocol
On Mon, 16 Jul 2018, Julien Grall wrote: > Hi Stefano, > > On 13/07/18 23:41, Stefano Stabellini wrote: > > On Mon, 9 Jul 2018, Julien Grall wrote: > > > On 07/07/18 00:12, Stefano Stabellini wrote: > > > > Extend the existing device tree based multiboot protocol to include > > > > information regarding multiple domains to boot. > > > > > > > > Signed-off-by: Stefano Stabellini > > > > > > > > --- > > > > Changes in v2: > > > > - lower case kernel > > > > - rename mem to memory > > > > - mandate cpus and memory > > > > - replace domU-kernel with kernel and domU-ramdisk with ramdisk > > > > - rename xen,domU with xen,domain > > > > - add info about dom0 > > > > - switch memory and cpus to integers > > > > - remove defaults > > > > - add vpl011 > > > > --- > > > >docs/misc/arm/device-tree/booting.txt | 108 > > > > ++ > > > >1 file changed, 108 insertions(+) > > > > > > > > diff --git a/docs/misc/arm/device-tree/booting.txt > > > > b/docs/misc/arm/device-tree/booting.txt > > > > index ce2d0dc..5c3b8da 100644 > > > > --- a/docs/misc/arm/device-tree/booting.txt > > > > +++ b/docs/misc/arm/device-tree/booting.txt > > > > @@ -119,3 +119,111 @@ For those you would hardcode the Xen commandline > > > > in > > > > the DTB under > > > >line by writing bootargs (as for native Linux). > > > >A Xen-aware bootloader would set xen,xen-bootargs for Xen, > > > > xen,dom0-bootargs > > > >for Dom0 and bootargs for native Linux. > > > > + > > > > + > > > > +Creating Multiple Domains directly from Xen > > > > +=== > > > > + > > > > +It is possible to have Xen create other domains, in addition to dom0, > > > > +out of the information provided via device tree. A kernel and initrd > > > > +(optional) need to be specified for each guest. > > > > + > > > > +For each domain to be created there needs to be one node under /chosen > > > > +with the following properties: > > > > + > > > > +- compatible > > > > + > > > > +For domUs: "xen,domain" > > > > +For dom0: "xen,domain", "xen,initial-domain" > > > > > > Looking briefly at the code, I don't see any support of > > > "xen,initial-domain". > > > Did I miss anything? > > > > > > But, it is a bit strange to put that in compatible. Shouldn't this be a > > > property? > > > > I haven't implemened this in this series yet. Let's add > > "xen,initial-domain" to the spec together with the implementation in one > > of the follow-up series. > > > > > > > > + > > > > +- memory > > > > + > > > > +An integer specifying the amount of megabytes of RAM to allocate to > > > > +the guest. > > > > > > I would define this a KB. With Dom0less it would be easy to spawn a guest > > > with > > > less than a MB of memory. What matter is the amount of memory should be > > > page-aligned. > > > > I think it would be good to allow users to specify the memory in KB, you > > are right that we might be able to have <1MB guests. At the same time, > > it is a pain to have to deal with KBs when allocating multi GBs guests. > > It is not very difficult, You just use your wcalc (or any calculator) and do a > shift 30. > > > > > Any suggestion on how to make this more user friendly? Maybe we could > > find a way to support multiple units, for instance we could support > > memory_mb (for MBs) and memory_kb (for KBs). > > That's ugly because you know have to describe clearly what they are for or > otherwise someone may think it would be fine to describe your 1.5GB guest as: > > memory_gb = 1 > memory_mb = 512 Yes, that would be bad > If you want to make user-friendly then provide macros to generate the > device-tree. This is already used for describe GIC controller in Linux. > > > Or we could just suck it up and use KBs only. I mean, if we have to > > support one unit only, it should probably be KBs. I wonder if it makes > > sense to rename memory to memory_ in any case for clarity. > > I would prefer to keep "memory" and encourage users to read the associated > documentation. I'll keep memory and only use kb for now. If somebody comes up with a good idea we can switch to that in follow versions. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 20/21] xen: support console_switching between Dom0 and DomUs on ARM
On Mon, 16 Jul 2018, Jan Beulich wrote:
> >>> On 07.07.18 at 01:12, wrote:
> > @@ -389,29 +392,49 @@ static void dump_console_ring_key(unsigned char key)
> > free_xenheap_pages(buf, order);
> > }
> >
> > -/* CTRL- switches input direction between Xen and DOM0. */
> > +/*
> > + * CTRL- switches input direction between Xen, Dom0 and
> > + * DomUs.
> > + */
> > #define switch_code (opt_conswitch[0]-'a'+1)
> > -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0.
> > */
> > +static int __read_mostly xen_rx = 1; /* 1 => input passed to domain 0. */
>
> I guess this variable wants renaming now.
Yeah. What about `console_rx'?
> > static void switch_serial_input(void)
> > {
> > -static char *input_str[2] = { "DOM0", "Xen" };
> > -xen_rx = !xen_rx;
> > -printk("*** Serial input -> %s", input_str[xen_rx]);
> > +xen_rx++;
> > +if ( xen_rx == max_init_domid + 1 )
> > +xen_rx = 0;
> > +
> > +if ( !xen_rx )
> > +printk("*** Serial input xen_rx=%d -> %s", xen_rx, "Xen");
> > +else
> > +printk("*** Serial input xen_rx=%d -> DOM%d", xen_rx, xen_rx - 1);
>
> What are the xen_rx= doing in the format string? They weren't there before.
Ah yes, we don't want to print "xen_rx" anywhere, it's a leftover from
my debugging. I'll remove it completely:
if ( !xen_rx )
printk("*** Serial input to Xen");
else
printk("*** Serial input to DOM%d", xen_rx - 1);
> > if ( switch_code )
> > -printk(" (type 'CTRL-%c' three times to switch input to %s)",
> > - opt_conswitch[0], input_str[!xen_rx]);
> > +printk(" (type 'CTRL-%c' three times to switch input)",
> > + opt_conswitch[0]);
> > printk("\n");
> > }
> >
> > static void __serial_rx(char c, struct cpu_user_regs *regs)
> > {
> > -if ( xen_rx )
> > +if ( xen_rx == 0 )
> > return handle_keypress(c, regs);
> >
> > -/* Deliver input to guest buffer, unless it is already full. */
> > -if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
> > -serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> > +if ( xen_rx == 1 )
> > +{
> > +/* Deliver input to guest buffer, unless it is already full. */
> > +if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
>
> Please add blanks around the - .
I'll do
> > +serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> > +}
> > +#ifdef CONFIG_ARM
>
> CONFIG_HAS_PL011 ?
I had already spotted this problem. I turned it into:
#if defined(CONFIG_ARM) && defined(CONFIG_SBSA_VUART_CONSOLE)
It's CONFIG_SBSA_VUART_CONSOLE rather than CONFIG_HAS_PL011 because this
has to do with the virtual pl011 implementation rather than the physical
driver in Xen.
> > +else
> > +{
> > +struct domain *d = get_domain_by_id(xen_rx - 1);
> > +if ( !d->arch.vpl011.ring_enable && d->arch.vpl011.inring != NULL )
>
> Blank line between these two lines please.
OK
> > @@ -933,9 +956,6 @@ void __init console_endboot(void)
> > "decrease log level threshold", 0);
> > register_irq_keyhandler('G', _toggle_guest,
> > "toggle host/guest log level adjustment", 0);
> > -
> > -/* Serial input is directed to DOM0 by default. */
> > -switch_serial_input();
>
> This removes an imo helpful boot time message. Is that intentional,
> and if so why?
Yes, it was intentional. switch_serial_input increases xen_rx, I thought
it didn't make too much sense to do that at boot, and would be clearer
to just initialize xen_rx to the wanted value from the get go (the value
would be 1 for dom0). Also, in previous implementations of this patch it
was actually required, but not anymore.
In fact, if you prefer, I could also keep this switch_serial_input()
call as-is and change the initial value of xen_rx to 0. That would also
work, as the increase of xen_rx here would end up selecting still dom0
for input.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 19/21] xen/arm: introduce create_domUs
On Mon, 16 Jul 2018, Jan Beulich wrote: > >>> On 07.07.18 at 01:12, wrote: > > --- a/xen/include/asm-x86/setup.h > > +++ b/xen/include/asm-x86/setup.h > > @@ -73,4 +73,6 @@ extern bool opt_dom0_shadow; > > #endif > > extern bool dom0_pvh; > > > > +#define max_init_domid (1) > > Why is this 1 rather than 0? Or is the name imprecise? Yeah, the name is imprecise. On x86 there is always only dom0 at boot, so this variable is set to 0+1 = 1. I can change the variable to actually match the name, so that on a system with only dom0 at boot, max_init_domid would be (0). It makes more sense that way. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tboot: Avoid recursive fault in early boot panic with tboot
On Mon, Jul 16, 2018 at 8:22 AM, Jan Beulich wrote:
On 16.07.18 at 13:59, wrote:
>> --- a/xen/arch/x86/tboot.c
>> +++ b/xen/arch/x86/tboot.c
>> @@ -391,7 +391,12 @@ void tboot_shutdown(uint32_t shutdown_type)
>> tboot_gen_xenheap_integrity(g_tboot_shared->s3_key, _mac);
>> }
>>
>> -write_ptbase(idle_vcpu[0]);
>> +/* During early boot, we can be called by panic before idle_vcpu[0] is
>> + * setup, but in that case we don't need to change page tables. */
>> +if ( idle_vcpu[0] != INVALID_VCPU )
>> +{
>> +write_ptbase(idle_vcpu[0]);
>> +}
>
> There are two style issues here: The comment wants to be properly
> formatted, and the braces want to be dropped.
>
> Also please honor patch submission rules - they get sent _to_ the
> list, with maintainers _cc_-ed.
Ok. Sorry for all the errors and thanks for the pointers.
Regards,
Jason
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-sid test] 74976: tolerable FAIL
flight 74976 distros-debian-sid real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74976/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-i386-sid-netboot-pvgrub 10 debian-di-install fail like 74948 test-armhf-armhf-armhf-sid-netboot-pygrub 10 debian-di-install fail like 74948 test-amd64-amd64-i386-sid-netboot-pygrub 10 debian-di-install fail like 74948 test-amd64-i386-amd64-sid-netboot-pygrub 10 debian-di-install fail like 74948 test-amd64-amd64-amd64-sid-netboot-pvgrub 10 debian-di-install fail like 74948 baseline version: flight 74948 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-sid-netboot-pvgrubfail test-amd64-i386-i386-sid-netboot-pvgrub fail test-amd64-i386-amd64-sid-netboot-pygrub fail test-armhf-armhf-armhf-sid-netboot-pygrubfail test-amd64-amd64-i386-sid-netboot-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.11-testing baseline-only test] 74975: tolerable FAIL
This run is configured for baseline tests only. flight 74975 xen-4.11-testing real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74975/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 guest-start fail never pass test-armhf-armhf-xl-midway 12 guest-start fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail never pass test-armhf-armhf-xl-credit2 12 guest-start fail never pass test-amd64-amd64-xl-pvshim 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 guest-start fail never pass test-armhf-armhf-xl-rtds 12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl 12 guest-start fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-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-qemuu-nested-intel 14 xen-boot/l1 fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win10-i386 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 10 debian-di-installfail never pass test-armhf-armhf-xl-vhd 10 debian-di-installfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 1fd87ba1cd0312b743a48756a91c6962b1514aac baseline version: xen eb17ff9ce6a99a8761d3f4768703691f34043356 Last test of basis74932 2018-07-03 10:19:05 Z 13 days Testing same since74975 2018-07-16 06:23:35 Z0 days1 attempts People who touched revisions under test: Ian Jackson jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-prev pass build-i386-prev pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass build-amd64-rumprun pass build-i386-rumprun pass test-xtf-amd64-amd64-1 pass test-xtf-amd64-amd64-2 pass test-xtf-amd64-amd64-3 pass test-xtf-amd64-amd64-4 pass test-xtf-amd64-amd64-5 pass test-amd64-amd64-xl pass test-armhf-armhf-xl fail
[Xen-devel] [xen-unstable-smoke test] 125247: tolerable all pass - PUSHED
flight 125247 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125247/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 6f28c09aa96b636ed4027cada570c1f2b8dc590f baseline version: xen 5894c0a2da66243a89088d309c7e1ea212ab28d6 Last test of basis 125237 2018-07-16 14:00:26 Z0 days Testing same since 125247 2018-07-16 17:00:44 Z0 days1 attempts People who touched revisions under test: Ian Jackson 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 5894c0a2da..6f28c09aa9 6f28c09aa96b636ed4027cada570c1f2b8dc590f -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 03/15] xen/arm: Introduce helpers to clear/flags flags in HCR_EL2
A couple of places in the code will need to clear/set flags in HCR_EL2
for a given vCPU and then replicate into the hardware. Introduce
helpers and replace open-coded version.
Signed-off-by: Julien Grall
---
I haven't find a good place for those new helpers so stick to
processor.h at the moment. This require to use macro rather than inline
helpers given that processor.h is usually the root of all headers.
---
xen/arch/arm/traps.c| 3 +--
xen/include/asm-arm/processor.h | 18 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9ae64ae6fc..d1bf69b245 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -681,8 +681,7 @@ static void inject_vabt_exception(struct cpu_user_regs
*regs)
break;
}
-current->arch.hcr_el2 |= HCR_VA;
-WRITE_SYSREG(current->arch.hcr_el2, HCR_EL2);
+vcpu_hcr_set_flags(current, HCR_VA);
}
/*
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 222a02dd99..7e695c2418 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -843,6 +843,24 @@ void abort_guest_exit_end(void);
: : : "memory"); \
} while (0)
+/*
+ * Clear/Set flags in HCR_EL2 for a given vCPU. It only supports the current
+ * vCPU for now.
+ */
+#define vcpu_hcr_clear_flags(v, flags) \
+do {\
+ASSERT((v) == current); \
+(v)->arch.hcr_el2 &= ~(flags); \
+WRITE_SYSREG((v)->arch.hcr_el2, HCR_EL2); \
+} while (0)
+
+#define vcpu_hcr_set_flags(v, flags)\
+do {\
+ASSERT((v) == current); \
+(v)->arch.hcr_el2 |= (flags); \
+WRITE_SYSREG((v)->arch.hcr_el2, HCR_EL2); \
+} while (0)
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_ARM_PROCESSOR_H */
/*
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 14/15] xen/arm: guest_walk_tables: Switch the return to bool
At the moment, guest_walk_tables can either return 0, -EFAULT, -EINVAL.
The use of the last 2 are not clearly defined and used inconsistently in
the code. The current only caller does not care about the return
value and the value of it seems very limited (no way to differentiate
between the 15ish error paths).
So switch to bool to simplify the return and make the developper life a
bit easier.
Signed-off-by: Julien Grall
---
xen/arch/arm/guest_walk.c| 50
xen/arch/arm/mem_access.c| 2 +-
xen/include/asm-arm/guest_walk.h | 8 +++
3 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index 4a1b4cf2c8..7db7a7321b 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -28,9 +28,9 @@
* page table on a different vCPU, the following registers would need to be
* loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1.
*/
-static int guest_walk_sd(const struct vcpu *v,
- vaddr_t gva, paddr_t *ipa,
- unsigned int *perms)
+static bool guest_walk_sd(const struct vcpu *v,
+ vaddr_t gva, paddr_t *ipa,
+ unsigned int *perms)
{
int ret;
bool disabled = true;
@@ -79,7 +79,7 @@ static int guest_walk_sd(const struct vcpu *v,
}
if ( disabled )
-return -EFAULT;
+return false;
/*
* The address of the L1 descriptor for the initial lookup has the
@@ -97,12 +97,12 @@ static int guest_walk_sd(const struct vcpu *v,
/* Access the guest's memory to read only one PTE. */
ret = access_guest_memory_by_ipa(d, paddr, , sizeof(short_desc_t),
false);
if ( ret )
-return -EINVAL;
+return false;
switch ( pte.walk.dt )
{
case L1DESC_INVALID:
-return -EFAULT;
+return false;
case L1DESC_PAGE_TABLE:
/*
@@ -122,10 +122,10 @@ static int guest_walk_sd(const struct vcpu *v,
/* Access the guest's memory to read only one PTE. */
ret = access_guest_memory_by_ipa(d, paddr, , sizeof(short_desc_t),
false);
if ( ret )
-return -EINVAL;
+return false;
if ( pte.walk.dt == L2DESC_INVALID )
-return -EFAULT;
+return false;
if ( pte.pg.page ) /* Small page. */
{
@@ -175,7 +175,7 @@ static int guest_walk_sd(const struct vcpu *v,
*perms |= GV2M_EXEC;
}
-return 0;
+return true;
}
/*
@@ -355,9 +355,9 @@ static bool check_base_size(unsigned int output_size,
uint64_t base)
* page table on a different vCPU, the following registers would need to be
* loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1.
*/
-static int guest_walk_ld(const struct vcpu *v,
- vaddr_t gva, paddr_t *ipa,
- unsigned int *perms)
+static bool guest_walk_ld(const struct vcpu *v,
+ vaddr_t gva, paddr_t *ipa,
+ unsigned int *perms)
{
int ret;
bool disabled = true;
@@ -442,7 +442,7 @@ static int guest_walk_ld(const struct vcpu *v,
*/
if ( (input_size > TCR_EL1_IPS_48_BIT_VAL) ||
(input_size < TCR_EL1_IPS_MIN_VAL) )
-return -EFAULT;
+return false;
}
else
{
@@ -487,7 +487,7 @@ static int guest_walk_ld(const struct vcpu *v,
}
if ( disabled )
-return -EFAULT;
+return false;
/*
* The starting level is the number of strides (grainsizes[gran] - 3)
@@ -498,12 +498,12 @@ static int guest_walk_ld(const struct vcpu *v,
/* Get the IPA output_size. */
ret = get_ipa_output_size(d, tcr, _size);
if ( ret )
-return -EFAULT;
+return false;
/* Make sure the base address does not exceed its configured size. */
ret = check_base_size(output_size, ttbr);
if ( !ret )
-return -EFAULT;
+return false;
/*
* Compute the base address of the first level translation table that is
@@ -523,12 +523,12 @@ static int guest_walk_ld(const struct vcpu *v,
/* Access the guest's memory to read only one PTE. */
ret = access_guest_memory_by_ipa(d, paddr, , sizeof(lpae_t),
false);
if ( ret )
-return -EFAULT;
+return false;
/* Make sure the base address does not exceed its configured size. */
ret = check_base_size(output_size, pfn_to_paddr(pte.walk.base));
if ( !ret )
-return -EFAULT;
+return false;
/*
* If page granularity is 64K, make sure the address is aligned
@@ -537,7 +537,7 @@ static int guest_walk_ld(const struct vcpu *v,
if ( (output_size < TCR_EL1_IPS_52_BIT_VAL) &&
(gran == GRANULE_SIZE_INDEX_64K) &&
(pte.walk.base & 0xf) )
-return -EFAULT;
+
[Xen-devel] [PATCH 00/15] xen/arm: Bunch of clean-up/improvement
Hi all,
This is patch series is a bunch of clean-up/improvement I collected while
working on the P2M and trap subsystems.
Cheers,
Julien Grall (15):
xen/arm: cpregs: Allow HSR_CPREG* to receive more than 1 parameter
xen/arm: cpregs: Fix typo in the documentation of TTBCR
xen/arm: Introduce helpers to clear/flags flags in HCR_EL2
xen/arm: p2m: Reduce the locking section in get_page_from_gva
xen/arm: p2m: Limit call to mem access code use in get_page_from_gva
xen/arm: Rework lpae_mapping
xen/arm: Rework lpae_table
xen/arm: Rename lpae_valid to lpae_is_valid
xen/arm: guest_walk: Use lpae_is_mapping to simplify the code
xen/arm: Introduce helpers to get/set an MFN from/to an LPAE entry
xen/arm: Allow lpae_is_{table, mapping} helpers to work on invalid
entry
xen/arm: p2m: Rename ret to mfn in p2m_lookup
xen/arm: p2m: Introduce a new variable removing_mapping in
__p2m_set_entry
xen/arm: guest_walk_tables: Switch the return to bool
xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
xen/arch/arm/guest_walk.c| 54 +--
xen/arch/arm/mem_access.c| 2 +-
xen/arch/arm/mm.c| 18 +++
xen/arch/arm/p2m.c | 110 ---
xen/arch/arm/traps.c | 27 +-
xen/include/asm-arm/cpregs.h | 6 +--
xen/include/asm-arm/guest_walk.h | 8 +--
xen/include/asm-arm/lpae.h | 27 +-
xen/include/asm-arm/processor.h | 18 +++
xen/include/asm-arm/traps.h | 24 +
10 files changed, 168 insertions(+), 126 deletions(-)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 01/15] xen/arm: cpregs: Allow HSR_CPREG* to receive more than 1 parameter
At the moment, HSR_CPREG is expected to receive only the co-processor register name in parameter. Because the name is actually a define, this may have been expanded by a previous macro. Rather than imposing the use of _HSR_CPREG* in such cases, allow HSR_CPREG to receive more than 1 parameter. Signed-off-by: Julien Grall --- xen/include/asm-arm/cpregs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h index 8db65d5e2a..4c74e8161b 100644 --- a/xen/include/asm-arm/cpregs.h +++ b/xen/include/asm-arm/cpregs.h @@ -47,8 +47,8 @@ ((__HSR_CPREG_##op1) << HSR_CP64_OP1_SHIFT) /* Encode a register as per HSR ISS pattern */ -#define HSR_CPREG32(X) _HSR_CPREG32(X) -#define HSR_CPREG64(X) _HSR_CPREG64(X) +#define HSR_CPREG32(X...) _HSR_CPREG32(X) +#define HSR_CPREG64(X...) _HSR_CPREG64(X) /* * Order registers by Coprocessor-> CRn-> Opcode 1-> CRm-> Opcode 2 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 13/15] xen/arm: p2m: Introduce a new variable removing_mapping in __p2m_set_entry
This is making the code slightly easier to understand.
Signed-off-by: Julien Grall
---
xen/arch/arm/p2m.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 66d58fabd7..e826f57842 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -792,6 +792,8 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
unsigned int target = 3 - (page_order / LPAE_SHIFT);
lpae_t *entry, *table, orig_pte;
int rc;
+/* A mapping is removed if the MFN is invalid. */
+bool removing_mapping = mfn_eq(smfn, INVALID_MFN);
/* Convenience aliases */
const unsigned int offsets[4] = {
@@ -817,9 +819,9 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
{
/*
* Don't try to allocate intermediate page table if the mapping
- * is about to be removed (i.e mfn == INVALID_MFN).
+ * is about to be removed.
*/
-rc = p2m_next_level(p2m, mfn_eq(smfn, INVALID_MFN),
+rc = p2m_next_level(p2m, removing_mapping,
level, , offsets[level]);
if ( rc == GUEST_TABLE_MAP_FAILED )
{
@@ -830,7 +832,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* when removing a mapping as it may not exist in the
* page table. In this case, just ignore it.
*/
-rc = mfn_eq(smfn, INVALID_MFN) ? 0 : -ENOENT;
+rc = removing_mapping ? 0 : -ENOENT;
goto out;
}
else if ( rc != GUEST_TABLE_NORMAL_PAGE )
@@ -925,7 +927,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( lpae_is_valid(orig_pte) )
p2m_remove_pte(entry, p2m->clean_pte);
-if ( mfn_eq(smfn, INVALID_MFN) )
+if ( removing_mapping )
/* Flush can be deferred if the entry is removed */
p2m->need_flush |= !!lpae_is_valid(orig_pte);
else
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 09/15] xen/arm: guest_walk: Use lpae_is_mapping to simplify the code
!lpae_is_page(pte, level) && !lpae_is_superpage(pte, level) is
equivalent to !lpae_is_mapping(pte, level).
At the same time drop lpae_is_page(pte, level) that is now unused.
Signed-off-by: Julien Grall
---
xen/arch/arm/guest_walk.c | 2 +-
xen/include/asm-arm/lpae.h | 5 -
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index a7c7e05603..e3e21bdad3 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -566,7 +566,7 @@ static int guest_walk_ld(const struct vcpu *v,
* PTE is invalid or holds a reserved entry (PTE<1:0> == x0)) or if the PTE
* maps a memory block at level 3 (PTE<1:0> == 01).
*/
-if ( !lpae_is_page(pte, level) && !lpae_is_superpage(pte, level) )
+if ( !lpae_is_mapping(pte, level) )
return -EFAULT;
/* Make sure that the lower bits of the PTE's base address are zero. */
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 1d86020d07..15595cd35c 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -153,11 +153,6 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned
int level)
return (level < 3) && lpae_is_mapping(pte, level);
}
-static inline bool lpae_is_page(lpae_t pte, unsigned int level)
-{
-return (level == 3) && lpae_is_valid(pte) && pte.walk.table;
-}
-
/*
* AArch64 supports pages with different sizes (4K, 16K, and 64K). To enable
* page table walks for various configurations, the following helpers enable
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 08/15] xen/arm: Rename lpae_valid to lpae_is_valid
This will help to keep the naming consistent accross all lpae helpers.
No functional change intended.
Signed-off-by: Julien Grall
---
xen/arch/arm/guest_walk.c | 2 +-
xen/arch/arm/mm.c | 6 +++---
xen/arch/arm/p2m.c | 20 ++--
xen/include/asm-arm/lpae.h | 8
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index 4d1ea0cdc1..a7c7e05603 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -546,7 +546,7 @@ static int guest_walk_ld(const struct vcpu *v,
* - The PTE is not valid.
* - If (level < 3) and the PTE is valid, we found a block descriptor.
*/
-if ( level == 3 || !lpae_valid(pte) || lpae_is_superpage(pte, level) )
+if ( level == 3 || !lpae_is_valid(pte) || lpae_is_superpage(pte,
level) )
break;
/*
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b7f2dabd05..de9b965d2f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1005,7 +1005,7 @@ static int create_xen_entries(enum xenmap_operation op,
}
}
-BUG_ON(!lpae_valid(*entry));
+BUG_ON(!lpae_is_valid(*entry));
third = __mfn_to_virt(entry->pt.base);
entry = [third_table_offset(addr)];
@@ -1013,7 +1013,7 @@ static int create_xen_entries(enum xenmap_operation op,
switch ( op ) {
case INSERT:
case RESERVE:
-if ( lpae_valid(*entry) )
+if ( lpae_is_valid(*entry) )
{
printk("%s: trying to replace an existing mapping addr=%lx
mfn=%"PRI_mfn"\n",
__func__, addr, mfn_x(mfn));
@@ -1030,7 +1030,7 @@ static int create_xen_entries(enum xenmap_operation op,
break;
case MODIFY:
case REMOVE:
-if ( !lpae_valid(*entry) )
+if ( !lpae_is_valid(*entry) )
{
printk("%s: trying to %s a non-existing mapping
addr=%lx\n",
__func__, op == REMOVE ? "remove" : "modify", addr);
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 72a84a33fd..a80ac301c5 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -250,7 +250,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool
read_only,
entry = *table + offset;
-if ( !lpae_valid(*entry) )
+if ( !lpae_is_valid(*entry) )
{
if ( read_only )
return GUEST_TABLE_MAP_FAILED;
@@ -342,7 +342,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
entry = table[offsets[level]];
-if ( lpae_valid(entry) )
+if ( lpae_is_valid(entry) )
{
*t = entry.p2m.type;
@@ -546,7 +546,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t
*entry)
lpae_t *p;
lpae_t pte;
-ASSERT(!lpae_valid(*entry));
+ASSERT(!lpae_is_valid(*entry));
page = alloc_domheap_page(NULL, 0);
if ( page == NULL )
@@ -610,7 +610,7 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m,
gfn_t gfn,
*/
static void p2m_put_l3_page(const lpae_t pte)
{
-ASSERT(lpae_valid(pte));
+ASSERT(lpae_is_valid(pte));
/*
* TODO: Handle other p2m types
@@ -638,7 +638,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
struct page_info *pg;
/* Nothing to do if the entry is invalid. */
-if ( !lpae_valid(entry) )
+if ( !lpae_is_valid(entry) )
return;
/* Nothing to do but updating the stats if the entry is a super-page. */
@@ -908,12 +908,12 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* sequence when updating the translation table (D4.7.1 in ARM DDI
* 0487A.j).
*/
-if ( lpae_valid(orig_pte) )
+if ( lpae_is_valid(orig_pte) )
p2m_remove_pte(entry, p2m->clean_pte);
if ( mfn_eq(smfn, INVALID_MFN) )
/* Flush can be deferred if the entry is removed */
-p2m->need_flush |= !!lpae_valid(orig_pte);
+p2m->need_flush |= !!lpae_is_valid(orig_pte);
else
{
lpae_t pte = mfn_to_p2m_entry(smfn, t, a);
@@ -928,7 +928,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* Although, it could be defered when only the permissions are
* changed (e.g in case of memaccess).
*/
-if ( lpae_valid(orig_pte) )
+if ( lpae_is_valid(orig_pte) )
{
if ( likely(!p2m->mem_access_enabled) ||
P2M_CLEAR_PERM(pte) != P2M_CLEAR_PERM(orig_pte) )
@@ -950,11 +950,11 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* Free the entry only if the original pte was valid and the base
* is different (to avoid freeing when permission is changed).
*/
-if ( lpae_valid(orig_pte) && entry->p2m.base != orig_pte.p2m.base )
+if ( lpae_is_valid(orig_pte) && entry->p2m.base != orig_pte.p2m.base )
[Xen-devel] [PATCH 07/15] xen/arm: Rework lpae_table
Currently, lpae_table can only work on entry from any level other than
3. Make it work with any level by extending the prototype to pass the
level.
At the same time, rename the function to lpae_is_mapping so naming stay
consistent accross all lpae_* helpers.
Signed-off-by: Julien Grall
---
xen/arch/arm/mm.c | 2 +-
xen/include/asm-arm/lpae.h | 9 ++---
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d234c46e41..b7f2dabd05 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -996,7 +996,7 @@ static int create_xen_entries(enum xenmap_operation op,
for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
{
entry = _second[second_linear_offset(addr)];
-if ( !lpae_table(*entry) )
+if ( !lpae_is_table(*entry, 2) )
{
rc = create_xen_table(entry);
if ( rc < 0 ) {
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 4cf188ff82..c803569c2d 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -133,14 +133,9 @@ static inline bool lpae_valid(lpae_t pte)
return pte.walk.valid;
}
-/*
- * This one can only be used on L0..L2 ptes because L3 mappings set
- * the table bit and therefore these would return the opposite to what
- * you would expect.
- */
-static inline bool lpae_table(lpae_t pte)
+static inline bool lpae_is_table(lpae_t pte, unsigned int level)
{
-return lpae_valid(pte) && pte.walk.table;
+return (level < 3) && lpae_valid(pte) && pte.walk.table;
}
static inline bool lpae_is_mapping(lpae_t pte, unsigned int level)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
GUEST_BUG_ON may be used in other files doing guest emulation.
Signed-off-by: Julien Grall
---
xen/arch/arm/traps.c| 24
xen/include/asm-arm/traps.h | 24
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d1bf69b245..6751e4d754 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -68,30 +68,6 @@ static inline void check_stack_alignment_constraints(void) {
#endif
}
-/*
- * GUEST_BUG_ON is intended for checking that the guest state has not been
- * corrupted in hardware and/or that the hardware behaves as we
- * believe it should (i.e. that certain traps can only occur when the
- * guest is in a particular mode).
- *
- * The intention is to limit the damage such h/w bugs (or spec
- * misunderstandings) can do by turning them into Denial of Service
- * attacks instead of e.g. information leaks or privilege escalations.
- *
- * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
- *
- * Compared with regular BUG_ON it dumps the guest vcpu state instead
- * of Xen's state.
- */
-#define guest_bug_on_failed(p) \
-do {\
-show_execution_state(guest_cpu_user_regs());\
-panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
- current, p, __LINE__, __FILE__); \
-} while (0)
-#define GUEST_BUG_ON(p) \
-do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
-
#ifdef CONFIG_ARM_32
static int debug_stack_lines = 20;
#define stack_words_per_line 8
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 70b52d1d16..0acf7de67d 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -9,6 +9,30 @@
# include
#endif
+/*
+ * GUEST_BUG_ON is intended for checking that the guest state has not been
+ * corrupted in hardware and/or that the hardware behaves as we
+ * believe it should (i.e. that certain traps can only occur when the
+ * guest is in a particular mode).
+ *
+ * The intention is to limit the damage such h/w bugs (or spec
+ * misunderstandings) can do by turning them into Denial of Service
+ * attacks instead of e.g. information leaks or privilege escalations.
+ *
+ * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
+ *
+ * Compared with regular BUG_ON it dumps the guest vcpu state instead
+ * of Xen's state.
+ */
+#define guest_bug_on_failed(p) \
+do {\
+show_execution_state(guest_cpu_user_regs());\
+panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
+ current, p, __LINE__, __FILE__); \
+} while (0)
+#define GUEST_BUG_ON(p) \
+do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
+
int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 05/15] xen/arm: p2m: Limit call to mem access code use in get_page_from_gva
Mem access has only an impact on the hardware translation between a
guest virtual address and the machine physical address. So it is not
necessary to fallback to memaccess for all the other case (e.g when it
is not possible to acquire the page behind the MFN).
Signed-off-by: Julien Grall
---
Cc: Razvan Cojocaru
Cc: Tamas K Lengyel
---
xen/arch/arm/p2m.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 5ca7ffe41b..ebf74760fa 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1425,17 +1425,24 @@ struct page_info *get_page_from_gva(struct vcpu *v,
vaddr_t va,
if ( par )
{
+/*
+ * When memaccess is enabled, the translation GVA to MADDR may
+ * have failed because of a permission fault.
+ */
+if ( p2m->mem_access_enabled )
+return p2m_mem_access_check_and_get_page(va, flags, v);
+
dprintk(XENLOG_G_DEBUG,
"%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx
par=%#"PRIx64"\n",
v, va, flags, par);
-goto err;
+return NULL;
}
if ( !mfn_valid(maddr_to_mfn(maddr)) )
{
dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
v, mfn_x(maddr_to_mfn(maddr)));
-goto err;
+return NULL;
}
page = mfn_to_page(maddr_to_mfn(maddr));
@@ -1445,13 +1452,9 @@ struct page_info *get_page_from_gva(struct vcpu *v,
vaddr_t va,
{
dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN
%#"PRI_mfn"\n",
v, mfn_x(maddr_to_mfn(maddr)));
-page = NULL;
+return NULL;
}
-err:
-if ( !page && p2m->mem_access_enabled )
-page = p2m_mem_access_check_and_get_page(va, flags, v);
-
return page;
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 06/15] xen/arm: Rework lpae_mapping
Currently, lpae_mapping can only work on entry from any level other than
3. Make it work with any level by extending the prototype to pass the
level.
At the same time, rename the function to lpae_is_mapping so naming stay
consistent accross lpae_* helpers.
Signed-off-by: Julien Grall
---
xen/arch/arm/p2m.c | 12 +++-
xen/include/asm-arm/lpae.h | 13 +
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ebf74760fa..72a84a33fd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -241,7 +241,8 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t
*entry);
* GUEST_TABLE_SUPER_PAGE: The next entry points to a superpage.
*/
static int p2m_next_level(struct p2m_domain *p2m, bool read_only,
- lpae_t **table, unsigned int offset)
+ unsigned int level, lpae_t **table,
+ unsigned int offset)
{
lpae_t *entry;
int ret;
@@ -260,7 +261,8 @@ static int p2m_next_level(struct p2m_domain *p2m, bool
read_only,
}
/* The function p2m_next_level is never called at the 3rd level */
-if ( lpae_mapping(*entry) )
+ASSERT(level < 3);
+if ( lpae_is_mapping(*entry, level) )
return GUEST_TABLE_SUPER_PAGE;
mfn = _mfn(entry->p2m.base);
@@ -331,7 +333,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
for ( level = P2M_ROOT_LEVEL; level < 3; level++ )
{
-rc = p2m_next_level(p2m, true, , offsets[level]);
+rc = p2m_next_level(p2m, true, level, , offsets[level]);
if ( rc == GUEST_TABLE_MAP_FAILED )
goto out_unmap;
else if ( rc != GUEST_TABLE_NORMAL_PAGE )
@@ -804,7 +806,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* is about to be removed (i.e mfn == INVALID_MFN).
*/
rc = p2m_next_level(p2m, mfn_eq(smfn, INVALID_MFN),
-, offsets[level]);
+level, , offsets[level]);
if ( rc == GUEST_TABLE_MAP_FAILED )
{
/*
@@ -861,7 +863,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
/* then move to the level we want to make real changes */
for ( ; level < target; level++ )
{
-rc = p2m_next_level(p2m, true, , offsets[level]);
+rc = p2m_next_level(p2m, true, level, , offsets[level]);
/*
* The entry should be found and either be a table
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index b30853e79d..4cf188ff82 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -134,7 +134,7 @@ static inline bool lpae_valid(lpae_t pte)
}
/*
- * These two can only be used on L0..L2 ptes because L3 mappings set
+ * This one can only be used on L0..L2 ptes because L3 mappings set
* the table bit and therefore these would return the opposite to what
* you would expect.
*/
@@ -143,14 +143,19 @@ static inline bool lpae_table(lpae_t pte)
return lpae_valid(pte) && pte.walk.table;
}
-static inline bool lpae_mapping(lpae_t pte)
+static inline bool lpae_is_mapping(lpae_t pte, unsigned int level)
{
-return lpae_valid(pte) && !pte.walk.table;
+if ( !lpae_valid(pte) )
+return false;
+else if ( level == 3 )
+return pte.walk.table;
+else
+return !pte.walk.table;
}
static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
{
-return (level < 3) && lpae_mapping(pte);
+return (level < 3) && lpae_is_mapping(pte, level);
}
static inline bool lpae_is_page(lpae_t pte, unsigned int level)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 04/15] xen/arm: p2m: Reduce the locking section in get_page_from_gva
The p2m lock is only necessary to prevent gvirt_to_maddr failing when
break-before-make sequence is used in the P2M update concurrently on
another pCPU. So reduce the locking section.
Signed-off-by: Julien Grall
---
Note a newline has been dropped to keep the block together.
---
xen/arch/arm/p2m.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 14791388ad..5ca7ffe41b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1415,9 +1415,13 @@ struct page_info *get_page_from_gva(struct vcpu *v,
vaddr_t va,
if ( v != current )
return NULL;
+/*
+ * The lock is here to protect us against the break-before-make
+ * sequence used when updating the entry.
+ */
p2m_read_lock(p2m);
-
par = gvirt_to_maddr(va, , flags);
+p2m_read_unlock(p2m);
if ( par )
{
@@ -1445,8 +1449,6 @@ struct page_info *get_page_from_gva(struct vcpu *v,
vaddr_t va,
}
err:
-p2m_read_unlock(p2m);
-
if ( !page && p2m->mem_access_enabled )
page = p2m_mem_access_check_and_get_page(va, flags, v);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 12/15] xen/arm: p2m: Rename ret to mfn in p2m_lookup
Comestic change to make clearer what is the return ('ret' is a bit
too generic).
Signed-off-by: Julien Grall
---
xen/arch/arm/p2m.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 07925a1be4..66d58fabd7 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -383,14 +383,14 @@ out:
mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
-mfn_t ret;
+mfn_t mfn;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
p2m_read_lock(p2m);
-ret = p2m_get_entry(p2m, gfn, t, NULL, NULL);
+mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL);
p2m_read_unlock(p2m);
-return ret;
+return mfn;
}
int guest_physmap_mark_populate_on_demand(struct domain *d,
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 02/15] xen/arm: cpregs: Fix typo in the documentation of TTBCR
Signed-off-by: Julien Grall --- xen/include/asm-arm/cpregs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h index 4c74e8161b..07e5791983 100644 --- a/xen/include/asm-arm/cpregs.h +++ b/xen/include/asm-arm/cpregs.h @@ -141,7 +141,7 @@ #define HSTRp15,4,c1,c1,3 /* Hyp. System Trap Register */ /* CP15 CR2: Translation Table Base and Control Registers */ -#define TTBCR p15,0,c2,c0,2 /* Translatation Table Base Control Register */ +#define TTBCR p15,0,c2,c0,2 /* Translation Table Base Control Register */ #define TTBR0 p15,0,c2/* Translation Table Base Reg. 0 */ #define TTBR1 p15,1,c2/* Translation Table Base Reg. 1 */ #define HTTBR p15,4,c2/* Hyp. Translation Table Base Register */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 11/15] xen/arm: Allow lpae_is_{table, mapping} helpers to work on invalid entry
Currently, lpae_is_{table, mapping} helpers will always return false on
entry with the valid bit unset. However, it would be useful to have them
operating on any entry. For instance to store information in advance but
still request a fault.
With that change, the p2m is now providing an overlay for *_is_{table,
mapping} that will check the valid bit of the entry.
Signed-off-by: Julien Grall
---
xen/arch/arm/guest_walk.c | 2 +-
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/p2m.c | 22 ++
xen/include/asm-arm/lpae.h | 11 +++
4 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index e3e21bdad3..4a1b4cf2c8 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -566,7 +566,7 @@ static int guest_walk_ld(const struct vcpu *v,
* PTE is invalid or holds a reserved entry (PTE<1:0> == x0)) or if the PTE
* maps a memory block at level 3 (PTE<1:0> == 01).
*/
-if ( !lpae_is_mapping(pte, level) )
+if ( !lpae_is_valid(pte) || !lpae_is_mapping(pte, level) )
return -EFAULT;
/* Make sure that the lower bits of the PTE's base address are zero. */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index e3dafe5fd7..52e57fef2f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -996,7 +996,7 @@ static int create_xen_entries(enum xenmap_operation op,
for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
{
entry = _second[second_linear_offset(addr)];
-if ( !lpae_is_table(*entry, 2) )
+if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
{
rc = create_xen_table(entry);
if ( rc < 0 ) {
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ec3fdcb554..07925a1be4 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -219,6 +219,20 @@ static p2m_access_t p2m_mem_access_radix_get(struct
p2m_domain *p2m, gfn_t gfn)
return radix_tree_ptr_to_int(ptr);
}
+/*
+ * lpae_is_* helpers don't check whether the valid bit is set in the
+ * PTE. Provide our own overlay to check the valid bit.
+ */
+static inline bool p2m_is_mapping(lpae_t pte, unsigned int level)
+{
+return lpae_is_valid(pte) && lpae_is_mapping(pte, level);
+}
+
+static inline bool p2m_is_superpage(lpae_t pte, unsigned int level)
+{
+return lpae_is_valid(pte) && lpae_is_superpage(pte, level);
+}
+
#define GUEST_TABLE_MAP_FAILED 0
#define GUEST_TABLE_SUPER_PAGE 1
#define GUEST_TABLE_NORMAL_PAGE 2
@@ -262,7 +276,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool
read_only,
/* The function p2m_next_level is never called at the 3rd level */
ASSERT(level < 3);
-if ( lpae_is_mapping(*entry, level) )
+if ( p2m_is_mapping(*entry, level) )
return GUEST_TABLE_SUPER_PAGE;
mfn = lpae_to_mfn(*entry);
@@ -642,7 +656,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
return;
/* Nothing to do but updating the stats if the entry is a super-page. */
-if ( lpae_is_superpage(entry, level) )
+if ( p2m_is_superpage(entry, level) )
{
p2m->stats.mappings[level]--;
return;
@@ -697,7 +711,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m,
lpae_t *entry,
* a superpage.
*/
ASSERT(level < target);
-ASSERT(lpae_is_superpage(*entry, level));
+ASSERT(p2m_is_superpage(*entry, level));
page = alloc_domheap_page(NULL, 0);
if ( !page )
@@ -834,7 +848,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
/* We need to split the original page. */
lpae_t split_pte = *entry;
-ASSERT(lpae_is_superpage(*entry, level));
+ASSERT(p2m_is_superpage(*entry, level));
if ( !p2m_split_superpage(p2m, _pte, level, target, offsets) )
{
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 05c87a8f48..88f30fc917 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -133,16 +133,19 @@ static inline bool lpae_is_valid(lpae_t pte)
return pte.walk.valid;
}
+/*
+ * lpae_is_* don't check the valid bit. This gives an opportunity for the
+ * callers to operate on the entry even if they are not valid. For
+ * instance to store information in advance.
+ */
static inline bool lpae_is_table(lpae_t pte, unsigned int level)
{
-return (level < 3) && lpae_is_valid(pte) && pte.walk.table;
+return (level < 3) && pte.walk.table;
}
static inline bool lpae_is_mapping(lpae_t pte, unsigned int level)
{
-if ( !lpae_is_valid(pte) )
-return false;
-else if ( level == 3 )
+if ( level == 3 )
return pte.walk.table;
else
return !pte.walk.table;
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 10/15] xen/arm: Introduce helpers to get/set an MFN from/to an LPAE entry
The new helpers make easier to read the code by abstracting the way to
set/get an MFN from/to an LPAE entry. The helpers is using "walk" as the
bits are common for accross different LPAE stage.
At the same time, use the new helpers to replace the various open-coding
place.
Signed-off-by: Julien Grall
---
xen/arch/arm/mm.c | 10 +-
xen/arch/arm/p2m.c | 19 ++-
xen/include/asm-arm/lpae.h | 3 +++
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index de9b965d2f..e3dafe5fd7 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -238,7 +238,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr,
/* For next iteration */
unmap_domain_page(mapping);
-mapping = map_domain_page(_mfn(pte.walk.base));
+mapping = map_domain_page(lpae_to_mfn(pte));
}
unmap_domain_page(mapping);
@@ -323,7 +323,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned
attr)
ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
-e.pt.base = mfn_x(mfn);
+lpae_set_mfn(e, mfn);
return e;
}
@@ -490,7 +490,7 @@ mfn_t domain_page_map_to_mfn(const void *ptr)
ASSERT(slot >= 0 && slot < DOMHEAP_ENTRIES);
ASSERT(map[slot].pt.avail != 0);
-return _mfn(map[slot].pt.base + offset);
+return mfn_add(lpae_to_mfn(map[slot]), offset);
}
#endif
@@ -851,7 +851,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
/* mfn_to_virt is not valid on the 1st 1st mfn, since it
* is not within the xenheap. */
first = slot == xenheap_first_first_slot ?
-xenheap_first_first : __mfn_to_virt(p->pt.base);
+xenheap_first_first : mfn_to_virt(lpae_to_mfn(*p));
}
else if ( xenheap_first_first_slot == -1)
{
@@ -1007,7 +1007,7 @@ static int create_xen_entries(enum xenmap_operation op,
BUG_ON(!lpae_is_valid(*entry));
-third = __mfn_to_virt(entry->pt.base);
+third = mfn_to_virt(lpae_to_mfn(*entry));
entry = [third_table_offset(addr)];
switch ( op ) {
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a80ac301c5..ec3fdcb554 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -265,7 +265,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool
read_only,
if ( lpae_is_mapping(*entry, level) )
return GUEST_TABLE_SUPER_PAGE;
-mfn = _mfn(entry->p2m.base);
+mfn = lpae_to_mfn(*entry);
unmap_domain_page(*table);
*table = map_domain_page(mfn);
@@ -349,7 +349,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
if ( a )
*a = p2m_mem_access_radix_get(p2m, gfn);
-mfn = _mfn(entry.p2m.base);
+mfn = lpae_to_mfn(entry);
/*
* The entry may point to a superpage. Find the MFN associated
* to the GFN.
@@ -519,7 +519,7 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t,
p2m_access_t a)
ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK));
-e.p2m.base = mfn_x(mfn);
+lpae_set_mfn(e, mfn);
return e;
}
@@ -621,7 +621,7 @@ static void p2m_put_l3_page(const lpae_t pte)
*/
if ( p2m_is_foreign(pte.p2m.type) )
{
-mfn_t mfn = _mfn(pte.p2m.base);
+mfn_t mfn = lpae_to_mfn(pte);
ASSERT(mfn_valid(mfn));
put_page(mfn_to_page(mfn));
@@ -655,7 +655,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
return;
}
-table = map_domain_page(_mfn(entry.p2m.base));
+table = map_domain_page(lpae_to_mfn(entry));
for ( i = 0; i < LPAE_ENTRIES; i++ )
p2m_free_entry(p2m, *(table + i), level + 1);
@@ -669,7 +669,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
*/
p2m_tlb_flush_sync(p2m);
-mfn = _mfn(entry.p2m.base);
+mfn = lpae_to_mfn(entry);
ASSERT(mfn_valid(mfn));
pg = mfn_to_page(mfn);
@@ -688,7 +688,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m,
lpae_t *entry,
bool rv = true;
/* Convenience aliases */
-mfn_t mfn = _mfn(entry->p2m.base);
+mfn_t mfn = lpae_to_mfn(*entry);
unsigned int next_level = level + 1;
unsigned int level_order = level_orders[next_level];
@@ -719,7 +719,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m,
lpae_t *entry,
* the necessary fields. So the correct permission are kept.
*/
pte = *entry;
-pte.p2m.base = mfn_x(mfn_add(mfn, i << level_order));
+lpae_set_mfn(pte, mfn_add(mfn, i << level_order));
/*
* First and second level pages set p2m.table = 0, but third
@@ -950,7 +950,8 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
* Free the entry only if the original pte was valid and the base
* is different (to avoid freeing when permission is changed).
*/
-if ( lpae_is_valid(orig_pte) && entry->p2m.base != orig_pte.p2m.base )
+if
[Xen-devel] [ovmf test] 125241: all pass - PUSHED
flight 125241 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/125241/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 4c6d0de7bad46fc15fd34d394dffda3766e3a6a1 baseline version: ovmf ae08ea246fe9b4a4e05b7ee6cdbd5b0fa38f3f69 Last test of basis 125151 2018-07-13 09:40:50 Z3 days Failing since125194 2018-07-16 02:40:42 Z0 days 11 attempts Testing same since 125241 2018-07-16 15:11:08 Z0 days1 attempts People who touched revisions under test: Bob Feng BobCF Dandan Bi Gary Lin 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 ae08ea246f..4c6d0de7ba 4c6d0de7bad46fc15fd34d394dffda3766e3a6a1 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: avoid memory_type_changed() invocations when possible
On 16/07/18 17:45, Jan Beulich wrote: > They're expensive, and nothing changes if MTRRs are disabled and any of > the ranges gets changed, or if fixed range MTRRs are disabled and any of > them gets changed. > > Signed-off-by: Jan Beulich There is still far more room improve here, but this is a start. Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3.2] libxl: Design of an async API to issue QMP commands to QEMU [and 1 more messages]
Anthony PERARD writes ("[PATCH v3.2] libxl: Design of an async API to issue QMP
commands to QEMU"):
> All the functions will be implemented in later patches.
>
> This patch includes the API that libxl can use to send QMP commands to
> QEMU.
...
> + * This facility allows a command to be sent to QEMU, and the response to be
> + * handed to a callback function. Each libxl__ev_qmp handles zero or one
> + * outstanding command; if multiple commands are to be sent concurrently,
> + * multiple libxl__ev_qmp's must be used.
> + *
> + * Possible states:
> + * Undefined
> + *Might contain anything.
> + * Idle
> + *Struct contents are defined enough to pass to any libxl__ev_qmp_*
> + *functions.
> + *The struct does not contain references to any allocated private
> resources
> + *so can be thrown away.
> + * Active
> + *Currently waiting for the callback to be called.
> + *_dispose must be called to reclaim resources (or wait for the callback
> to
> + *be called).
> + *
> + * libxl__ev_qmp_init: Undefined/Idle -> Idle
> + *
> + * libxl__ev_qmp_send: Idle -> Active (on error: Idle)
> + *Sends a command to QEMU.
> + *
> + * libxl__ev_qmp_dispose: Active/Idle -> Idle
> + *
> + * callback:
> + *When called, ev is Idle, so can be reused or thrown away.
> + *When an error occured, it is called with response == NULL and the error
> + *code in rc.
You have removed this text:
+ * When called from within a callback, the same QMP connection will be
+ * reused to execute the new command. This is important in the case
+ * where the first command is "add-fd" and the second command use the
+ * fdset created by QEMU.
That removes the need for a fourth state, as I discussed earlier. But
does this not introduce the problem that this text was addressing: Ie
there is no way for the user of libxl_ev_qmp to ensure that the
commands to add-fd, and use the fdset, occur on the same qmp
connection.
What has changed since you wrote that ? Or have I misunderstood
something ?
NB that I haven't yet read in detail the part about describing the
implementation, ie the qmp_state but I think this problem exists no
matter what that implementation looks like...
Anthony PERARD writes ("Re: [PATCH v3.2] libxl: Design of an async API to issue
QMP commands to QEMU"):
> On Mon, Jul 16, 2018 at 04:28:00PM +0100, Anthony PERARD wrote:
> > + * Implementation of the QMP client.
> > + *
>
> Here, I wanted to add more description, but forgot to commit before to
> sent the patch, it should read:
>
> + * This struct qmp_state is used by the libxl__ev_qmp_* functions, but it is
> + * not visible to users of libxl__ev_qmp_*. It is allocated as needed and
> + * stored in CTX in order to allow reuse of a same QMP connection between
> + * different users.
This makes sense, thanks.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: report use of PCID together with reporting XPTI status
On 16/07/18 17:45, Jan Beulich wrote: > --- a/xen/include/asm-x86/spec_ctrl.h > +++ b/xen/include/asm-x86/spec_ctrl.h > @@ -38,6 +38,8 @@ extern uint8_t opt_xpti; > #define OPT_XPTI_DOM0 0x01 > #define OPT_XPTI_DOMU 0x02 > > +bool xpti_pcid_enabled(void); To be used in the way you want, this needs a stub for the non CONFIG_PV case, and should probably be in pv/domain.h ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] VMX: fix vmx_{find,del}_msr() build
On 16/07/18 17:46, Jan Beulich wrote: > For a reason that I can't explain, it is only the shim build that fails > for me with an older gcc due to the compiler not recognizing that > apparently uninitialized variables aren't really uninitialized. The only thing that comes to mind is some differences in CFLAGS et al. There is nothing in kconfig which would plausibly impact that code. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] VMX: fix vmx_{find,del}_msr() build
For a reason that I can't explain, it is only the shim build that fails
for me with an older gcc due to the compiler not recognizing that
apparently uninitialized variables aren't really uninitialized. Pull out
the assignments used by two of the three case blocks and make them
initializers of the variables, as I think I had suggested during review.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1305,7 +1305,8 @@ struct vmx_msr_entry *vmx_find_msr(const
{
const struct arch_vmx_struct *vmx = >arch.hvm_vmx;
struct vmx_msr_entry *start = NULL, *ent, *end;
-unsigned int substart, subend, total;
+unsigned int substart = 0, subend = vmx->msr_save_count;
+unsigned int total = vmx->msr_load_count;
ASSERT(v == current || !vcpu_runnable(v));
@@ -1313,23 +1314,18 @@ struct vmx_msr_entry *vmx_find_msr(const
{
case VMX_MSR_HOST:
start= vmx->host_msr_area;
-substart = 0;
subend = vmx->host_msr_count;
total= subend;
break;
case VMX_MSR_GUEST:
start= vmx->msr_area;
-substart = 0;
-subend = vmx->msr_save_count;
-total= vmx->msr_load_count;
break;
case VMX_MSR_GUEST_LOADONLY:
start= vmx->msr_area;
-substart = vmx->msr_save_count;
-subend = vmx->msr_load_count;
-total= subend;
+substart = subend;
+subend = total;
break;
default:
@@ -1461,7 +1457,8 @@ int vmx_del_msr(struct vcpu *v, uint32_t
{
struct arch_vmx_struct *vmx = >arch.hvm_vmx;
struct vmx_msr_entry *start = NULL, *ent, *end;
-unsigned int substart, subend, total;
+unsigned int substart = 0, subend = vmx->msr_save_count;
+unsigned int total = vmx->msr_load_count;
ASSERT(v == current || !vcpu_runnable(v));
@@ -1469,23 +1466,18 @@ int vmx_del_msr(struct vcpu *v, uint32_t
{
case VMX_MSR_HOST:
start= vmx->host_msr_area;
-substart = 0;
subend = vmx->host_msr_count;
total= subend;
break;
case VMX_MSR_GUEST:
start= vmx->msr_area;
-substart = 0;
-subend = vmx->msr_save_count;
-total= vmx->msr_load_count;
break;
case VMX_MSR_GUEST_LOADONLY:
start= vmx->msr_area;
-substart = vmx->msr_save_count;
-subend = vmx->msr_load_count;
-total= subend;
+substart = subend;
+subend = total;
break;
default:
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/HVM: avoid memory_type_changed() invocations when possible
They're expensive, and nothing changes if MTRRs are disabled and any of the ranges gets changed, or if fixed range MTRRs are disabled and any of them gets changed. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -472,7 +472,9 @@ bool_t mtrr_fix_range_msr_set(struct dom return 0; fixed_range_base[row] = msr_content; -memory_type_changed(d); + +if ( m->enabled && m->fixed_enabled ) +memory_type_changed(d); } return 1; @@ -515,7 +517,8 @@ bool_t mtrr_var_range_msr_set( m->overlapped = is_var_mtrr_overlapped(m); -memory_type_changed(d); +if ( m->enabled ) +memory_type_changed(d); return 1; } ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86: report use of PCID together with reporting XPTI status
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -288,6 +288,12 @@ int pv_domain_initialise(struct domain *
return rc;
}
+bool __init xpti_pcid_enabled(void)
+{
+return use_invpcid && cpu_has_pcid &&
+ (opt_pcid == PCID_ALL || opt_pcid == PCID_XPTI);
+}
+
static void _toggle_guest_pt(struct vcpu *v)
{
const struct domain *d = v->domain;
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -256,9 +256,10 @@ static void __init print_details(enum in
boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB" : "",
opt_eager_fpu ? " EAGER_FPU" : "");
-printk(" XPTI (64-bit PV only): Dom0 %s, DomU %s\n",
+printk(" XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
opt_xpti & OPT_XPTI_DOM0 ? "enabled" : "disabled",
- opt_xpti & OPT_XPTI_DOMU ? "enabled" : "disabled");
+ opt_xpti & OPT_XPTI_DOMU ? "enabled" : "disabled",
+ xpti_pcid_enabled() ? "" : "out");
}
/* Calculate whether Retpoline is known-safe on this CPU. */
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -38,6 +38,8 @@ extern uint8_t opt_xpti;
#define OPT_XPTI_DOM0 0x01
#define OPT_XPTI_DOMU 0x02
+bool xpti_pcid_enabled(void);
+
static inline void init_shadow_spec_ctrl_state(void)
{
struct cpu_info *info = get_cpu_info();
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3.2] libxl: Design of an async API to issue QMP commands to QEMU
On Mon, Jul 16, 2018 at 04:28:00PM +0100, Anthony PERARD wrote:
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 760f2798c7..1e6fbb64a5 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -1237,6 +1237,108 @@ int libxl__qmp_initializations(libxl__gc *gc,
> uint32_t domid,
> return ret;
> }
>
> +/* Implementation of asynchronous QMP calls */
> +
> +/*
> + * Implementation of the QMP client.
> + *
Here, I wanted to add more description, but forgot to commit before to
sent the patch, it should read:
+ * This struct qmp_state is used by the libxl__ev_qmp_* functions, but it is
+ * not visible to users of libxl__ev_qmp_*. It is allocated as needed and
+ * stored in CTX in order to allow reuse of a same QMP connection between
+ * different users.
+ *
> + * This are different state possible in which the client can be in, with the
> + * list of possible transition listed after.
> + *
> + * States:
> + * Disconnected
> + * Nothing, no allocated ressources.
> + * Connecting
> + * Have allocated ressources, including a connection to a QMP socket.
> + * Waiting for server greeting.
> + * Capability Negociation
> + * QMP server is in Capabilities Negotiation mode.
> + * Waiting for a response to the "qmp_capabilities" command.
> + * Connected
> + * QMP server is in command mode, commands can be issued.
> + * There is outstanding command to be sent and/or there are in-flight
> + * libxl_ev_qmp with callback.
> + * Within A Callback
> + * The QMP client enter this state when a callback is called.
> + * The connection to QEMU is kept open until at least the callback
> + * return.
> + *
> + * Here is the transition from one state to the next:
> + *Disconnected -> Connecting
> + * Connect to the QMP socket.
> + *Connecting -> Capability Negociation
> + * Server greeting received
> + * Send "qmp_capabilities"
> + *Capability Negociation -> Connected
> + * Response to "qmp_capabilities" received"
> + *
> + *Connected -> Within A Callback
> + * When a response is received, if there's a callback associated to it,
> + * it is called.
> + *Connected -> Disconnected
> + * When both list of in-flight event and command to send are empty.
> + *
> + *Within A Callback -> Connected
> + * On return from a libxl__ev_qmp callback, there are more command to
> send
> + * or there are other in-flight event.
> + *Within A Callback -> Disconnected
> + * On return from a libxl__ev_qmp callback, both list of command to send
> + * and list of in-flight event are empty.
> + * QMP socket connection is closed, all ressources are deallocated.
> + *
> + * * -> Disconnected
> + * Whenever a error occure with the QMP socket, all outstanding callback
> + * will be called with an error, all ressource will be deallocated.
> + *
> + *
> + * checkout qemu.git:docs/interop/qmp-spec.txt for more information on the
> + * qmp protocol.
> + */
> +
> +typedef enum qmp_states {
> +qmp_state_disconnected = 1,
> +qmp_state_connecting,
> +qmp_state_capability_negociation,
> +qmp_state_connected,
> +qmp_state_within_callback,
> +} qmp_states;
> +
> +typedef struct qmp_tx_buf qmp_tx_buf;
> +struct qmp_tx_buf {
> +char *buf;
> +size_t len;
> +/* File descriptor to send along the command */
> +libxl__carefd *efd;
> +LIBXL_TAILQ_ENTRY(qmp_tx_buf) entry;
> +};
> +
> +struct qmp_state {
> +libxl__carefd *cfd;
> +libxl__ev_fd efd;
> +uint32_t domid;
> +
> +/* Current state */
> +qmp_states state;
--
Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125237: tolerable all pass - PUSHED
flight 125237 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125237/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 5894c0a2da66243a89088d309c7e1ea212ab28d6 baseline version: xen e3f667bc5f51d0aa44357a64ca134cd952679c81 Last test of basis 125155 2018-07-13 14:00:54 Z3 days Testing same since 125237 2018-07-16 14:00:26 Z0 days1 attempts People who touched revisions under test: Jan Beulich Julien Grall Roger Pau Monné 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 e3f667bc5f..5894c0a2da 5894c0a2da66243a89088d309c7e1ea212ab28d6 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 20/21] xen: support console_switching between Dom0 and DomUs on ARM
>>> On 07.07.18 at 01:12, wrote:
> @@ -389,29 +392,49 @@ static void dump_console_ring_key(unsigned char key)
> free_xenheap_pages(buf, order);
> }
>
> -/* CTRL- switches input direction between Xen and DOM0. */
> +/*
> + * CTRL- switches input direction between Xen, Dom0 and
> + * DomUs.
> + */
> #define switch_code (opt_conswitch[0]-'a'+1)
> -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. */
> +static int __read_mostly xen_rx = 1; /* 1 => input passed to domain 0. */
I guess this variable wants renaming now.
> static void switch_serial_input(void)
> {
> -static char *input_str[2] = { "DOM0", "Xen" };
> -xen_rx = !xen_rx;
> -printk("*** Serial input -> %s", input_str[xen_rx]);
> +xen_rx++;
> +if ( xen_rx == max_init_domid + 1 )
> +xen_rx = 0;
> +
> +if ( !xen_rx )
> +printk("*** Serial input xen_rx=%d -> %s", xen_rx, "Xen");
> +else
> +printk("*** Serial input xen_rx=%d -> DOM%d", xen_rx, xen_rx - 1);
What are the xen_rx= doing in the format string? They weren't there before.
> if ( switch_code )
> -printk(" (type 'CTRL-%c' three times to switch input to %s)",
> - opt_conswitch[0], input_str[!xen_rx]);
> +printk(" (type 'CTRL-%c' three times to switch input)",
> + opt_conswitch[0]);
> printk("\n");
> }
>
> static void __serial_rx(char c, struct cpu_user_regs *regs)
> {
> -if ( xen_rx )
> +if ( xen_rx == 0 )
> return handle_keypress(c, regs);
>
> -/* Deliver input to guest buffer, unless it is already full. */
> -if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
> -serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> +if ( xen_rx == 1 )
> +{
> +/* Deliver input to guest buffer, unless it is already full. */
> +if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
Please add blanks around the - .
> +serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> +}
> +#ifdef CONFIG_ARM
CONFIG_HAS_PL011 ?
> +else
> +{
> +struct domain *d = get_domain_by_id(xen_rx - 1);
> +if ( !d->arch.vpl011.ring_enable && d->arch.vpl011.inring != NULL )
Blank line between these two lines please.
> @@ -933,9 +956,6 @@ void __init console_endboot(void)
> "decrease log level threshold", 0);
> register_irq_keyhandler('G', _toggle_guest,
> "toggle host/guest log level adjustment", 0);
> -
> -/* Serial input is directed to DOM0 by default. */
> -switch_serial_input();
This removes an imo helpful boot time message. Is that intentional,
and if so why?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 19/21] xen/arm: introduce create_domUs
>>> On 07.07.18 at 01:12, wrote: > --- a/xen/include/asm-x86/setup.h > +++ b/xen/include/asm-x86/setup.h > @@ -73,4 +73,6 @@ extern bool opt_dom0_shadow; > #endif > extern bool dom0_pvh; > > +#define max_init_domid (1) Why is this 1 rather than 0? Or is the name imprecise? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 125169: tolerable FAIL - PUSHED
flight 125169 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/125169/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 16 depriv-audit-qemu/create/privcmd blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 17 depriv-audit-qemu/create/gntdev blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 18 depriv-audit-qemu/create/evtchn blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 19 depriv-audit-qemu/create/other blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 20 depriv-audit-qemu/create/xenstore blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 16 depriv-audit-qemu/create/privcmd blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 17 depriv-audit-qemu/create/gntdev blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 18 depriv-audit-qemu/create/evtchn blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 19 depriv-audit-qemu/create/other blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 20 depriv-audit-qemu/create/xenstore blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125134 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125134 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125134 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125134 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125134 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-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 15 depriv-audit-qemu/create fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 15 depriv-audit-qemu/create fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-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-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuu9277d81f5c2c6f4d0b5e47c8476eb7ee7e5c0beb baseline version: qemuuff82d3c73ec20d6f40e8bbfafe8ed5110bba5049 Last test of basis 125134 2018-07-12 14:45:47 Z4 days Testing same since 125169 2018-07-14
Re: [Xen-devel] [PATCH v12 07/11] x86/hvm: Introduce viridian_save_vcpu_ctxt_one() func
> -Original Message-
> From: Alexandru Isaila [mailto:aisa...@bitdefender.com]
> Sent: 16 July 2018 15:56
> To: xen-de...@lists.xen.org
> Cc: Ian Jackson ; Wei Liu ;
> jbeul...@suse.com; Andrew Cooper ; Paul
> Durrant ; Alexandru Isaila
>
> Subject: [PATCH v12 07/11] x86/hvm: Introduce
> viridian_save_vcpu_ctxt_one() func
>
> This is used to save data from a single instance.
>
> Signed-off-by: Alexandru Isaila
> ---
> xen/arch/x86/hvm/viridian.c | 29 +++--
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 694eae6..f3430bb 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -1026,24 +1026,33 @@ static int viridian_load_domain_ctxt(struct
> domain *d, hvm_domain_context_t *h)
> HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_DOMAIN,
> viridian_save_domain_ctxt,
>viridian_load_domain_ctxt, 1, HVMSR_PER_DOM);
>
> -static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> +static int viridian_save_vcpu_ctxt_one(struct vcpu *v,
> hvm_domain_context_t *h)
> {
> -struct vcpu *v;
> +struct hvm_viridian_vcpu_context ctxt = {};
>
> -if ( !is_viridian_domain(d) )
> +if ( !is_viridian_domain(v->domain) )
> return 0;
>
> +ctxt.vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw;
> +ctxt.vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending;
> +
> +return hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, );
> +}
> +
> +static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> +{
> +struct vcpu *v;
> +int err = 0;
> +
> for_each_vcpu( d, v ) {
Sorry for not noticing earlier but, whilst you're not actually changing this
line, it needs a style fix. So if you re-spin this then it would be good to
correct it.
> -struct hvm_viridian_vcpu_context ctxt = {
> -.vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw,
> -.vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending,
> -};
>
> -if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, ) != 0 )
> -return 1;
> +err = viridian_save_vcpu_ctxt_one(v, h);
> +
I think you can lose the blank line here, but that's a small nit so...
Reviewed-by: Paul Durrant
> +if ( err )
> +break;
> }
>
> -return 0;
> +return err;
> }
>
> static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t
> *h)
> --
> 2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v12 03/11] x86/hvm: Introduce hvm_save_cpu_ctxt_one func
> -Original Message-
> From: Alexandru Isaila [mailto:aisa...@bitdefender.com]
> Sent: 16 July 2018 15:55
> To: xen-de...@lists.xen.org
> Cc: Ian Jackson ; Wei Liu ;
> jbeul...@suse.com; Andrew Cooper ; Paul
> Durrant ; Alexandru Isaila
>
> Subject: [PATCH v12 03/11] x86/hvm: Introduce hvm_save_cpu_ctxt_one
> func
>
> This is used to save data from a single instance.
>
> Signed-off-by: Alexandru Isaila
>
> ---
> Changes since V11:
> - hvm_save_cpu_ctxt() now returns err from
> hvm_save_cpu_ctxt_one().
> ---
> xen/arch/x86/hvm/hvm.c | 216 ++
> ---
> 1 file changed, 113 insertions(+), 103 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index dd88751..e20a25c 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -787,119 +787,129 @@ static int hvm_load_tsc_adjust(struct domain *d,
> hvm_domain_context_t *h)
> HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust,
>hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU);
>
> +static int hvm_save_cpu_ctxt_one(struct vcpu *v, hvm_domain_context_t
> *h)
> +{
> +struct segment_register seg;
> +struct hvm_hw_cpu ctxt;
> +
> +memset(, 0, sizeof(ctxt));
Why not use an = {} initializer instead of the memset here like elsewhere?
Paul
> +
> +/* Architecture-specific vmcs/vmcb bits */
> +hvm_funcs.save_cpu_ctxt(v, );
> +
> +ctxt.tsc = hvm_get_guest_tsc_fixed(v, v->domain-
> >arch.hvm_domain.sync_tsc);
> +
> +ctxt.msr_tsc_aux = hvm_msr_tsc_aux(v);
> +
> +hvm_get_segment_register(v, x86_seg_idtr, );
> +ctxt.idtr_limit = seg.limit;
> +ctxt.idtr_base = seg.base;
> +
> +hvm_get_segment_register(v, x86_seg_gdtr, );
> +ctxt.gdtr_limit = seg.limit;
> +ctxt.gdtr_base = seg.base;
> +
> +hvm_get_segment_register(v, x86_seg_cs, );
> +ctxt.cs_sel = seg.sel;
> +ctxt.cs_limit = seg.limit;
> +ctxt.cs_base = seg.base;
> +ctxt.cs_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_ds, );
> +ctxt.ds_sel = seg.sel;
> +ctxt.ds_limit = seg.limit;
> +ctxt.ds_base = seg.base;
> +ctxt.ds_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_es, );
> +ctxt.es_sel = seg.sel;
> +ctxt.es_limit = seg.limit;
> +ctxt.es_base = seg.base;
> +ctxt.es_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_ss, );
> +ctxt.ss_sel = seg.sel;
> +ctxt.ss_limit = seg.limit;
> +ctxt.ss_base = seg.base;
> +ctxt.ss_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_fs, );
> +ctxt.fs_sel = seg.sel;
> +ctxt.fs_limit = seg.limit;
> +ctxt.fs_base = seg.base;
> +ctxt.fs_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_gs, );
> +ctxt.gs_sel = seg.sel;
> +ctxt.gs_limit = seg.limit;
> +ctxt.gs_base = seg.base;
> +ctxt.gs_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_tr, );
> +ctxt.tr_sel = seg.sel;
> +ctxt.tr_limit = seg.limit;
> +ctxt.tr_base = seg.base;
> +ctxt.tr_arbytes = seg.attr;
> +
> +hvm_get_segment_register(v, x86_seg_ldtr, );
> +ctxt.ldtr_sel = seg.sel;
> +ctxt.ldtr_limit = seg.limit;
> +ctxt.ldtr_base = seg.base;
> +ctxt.ldtr_arbytes = seg.attr;
> +
> +if ( v->fpu_initialised )
> +{
> +memcpy(ctxt.fpu_regs, v->arch.fpu_ctxt, sizeof(ctxt.fpu_regs));
> +ctxt.flags = XEN_X86_FPU_INITIALISED;
> +}
> +
> +ctxt.rax = v->arch.user_regs.rax;
> +ctxt.rbx = v->arch.user_regs.rbx;
> +ctxt.rcx = v->arch.user_regs.rcx;
> +ctxt.rdx = v->arch.user_regs.rdx;
> +ctxt.rbp = v->arch.user_regs.rbp;
> +ctxt.rsi = v->arch.user_regs.rsi;
> +ctxt.rdi = v->arch.user_regs.rdi;
> +ctxt.rsp = v->arch.user_regs.rsp;
> +ctxt.rip = v->arch.user_regs.rip;
> +ctxt.rflags = v->arch.user_regs.rflags;
> +ctxt.r8 = v->arch.user_regs.r8;
> +ctxt.r9 = v->arch.user_regs.r9;
> +ctxt.r10 = v->arch.user_regs.r10;
> +ctxt.r11 = v->arch.user_regs.r11;
> +ctxt.r12 = v->arch.user_regs.r12;
> +ctxt.r13 = v->arch.user_regs.r13;
> +ctxt.r14 = v->arch.user_regs.r14;
> +ctxt.r15 = v->arch.user_regs.r15;
> +ctxt.dr0 = v->arch.debugreg[0];
> +ctxt.dr1 = v->arch.debugreg[1];
> +ctxt.dr2 = v->arch.debugreg[2];
> +ctxt.dr3 = v->arch.debugreg[3];
> +ctxt.dr6 = v->arch.debugreg[6];
> +ctxt.dr7 = v->arch.debugreg[7];
> +
> +return hvm_save_entry(CPU, v->vcpu_id, h, );
> +}
> +
> static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> {
> struct vcpu *v;
> -struct hvm_hw_cpu ctxt;
> -struct segment_register seg;
> +int err = 0;
>
> for_each_vcpu ( d, v )
> {
> -/* We don't need to save state for a vcpu that is down; the restore
> - * code will leave it down if there is nothing saved. */
> +/*
> + * We don't need to save state for a vcpu that
[Xen-devel] [PATCH v3.2] libxl: Design of an async API to issue QMP commands to QEMU
All the functions will be implemented in later patches.
This patch includes the API that libxl can use to send QMP commands to
QEMU.
This patch also include a description of the internal states of the QMP
client in charge of connecting to QEMU, sending command and handling
responces.
Signed-off-by: Anthony PERARD
---
tools/libxl/libxl_internal.h | 58
tools/libxl/libxl_qmp.c | 102 +++
tools/libxl/libxl_types.idl | 4 ++
3 files changed, 164 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2a1fb784ec..7c88f4c3b5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -357,6 +357,64 @@ struct libxl__ev_child {
};
+/*
+ * QMP asynchronous calls
+ */
+
+/*
+ * This facility allows a command to be sent to QEMU, and the response to be
+ * handed to a callback function. Each libxl__ev_qmp handles zero or one
+ * outstanding command; if multiple commands are to be sent concurrently,
+ * multiple libxl__ev_qmp's must be used.
+ *
+ * Possible states:
+ * Undefined
+ *Might contain anything.
+ * Idle
+ *Struct contents are defined enough to pass to any libxl__ev_qmp_*
+ *functions.
+ *The struct does not contain references to any allocated private resources
+ *so can be thrown away.
+ * Active
+ *Currently waiting for the callback to be called.
+ *_dispose must be called to reclaim resources (or wait for the callback to
+ *be called).
+ *
+ * libxl__ev_qmp_init: Undefined/Idle -> Idle
+ *
+ * libxl__ev_qmp_send: Idle -> Active (on error: Idle)
+ *Sends a command to QEMU.
+ *
+ * libxl__ev_qmp_dispose: Active/Idle -> Idle
+ *
+ * callback:
+ *When called, ev is Idle, so can be reused or thrown away.
+ *When an error occured, it is called with response == NULL and the error
+ *code in rc.
+ */
+typedef struct libxl__ev_qmp libxl__ev_qmp;
+typedef libxl__ev_qmp_callback(libxl__gc *gc, libxl__ev_qmp *ev,
+ const libxl__json_object *response,
+ int rc);
+
+_hidden void libxl__ev_qmp_init(libxl__ev_qmp *ev);
+_hidden int libxl__ev_qmp_send(libxl__gc *gc, libxl__ev_qmp *ev,
+ const char *cmd, libxl__json_object *args);
+_hidden void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev);
+
+struct libxl__ev_qmp {
+/* caller should include this in their own struct */
+/* caller must fill these in, and they must all remain valid */
+uint32_t domid;
+libxl__ev_qmp_callback *callback;
+libxl__carefd *efd; /* set to send a fd with the command, NULL otherwise */
+
+/* remaining fields are private to libxl_ev_qmp_* */
+int id;
+LIBXL_TAILQ_ENTRY(libxl__ev_qmp) entry;
+};
+
+
/*
* evgen structures, which are the state we use for generating
* events for the caller.
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 760f2798c7..1e6fbb64a5 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1237,6 +1237,108 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t
domid,
return ret;
}
+/* Implementation of asynchronous QMP calls */
+
+/*
+ * Implementation of the QMP client.
+ *
+ * This are different state possible in which the client can be in, with the
+ * list of possible transition listed after.
+ *
+ * States:
+ * Disconnected
+ * Nothing, no allocated ressources.
+ * Connecting
+ * Have allocated ressources, including a connection to a QMP socket.
+ * Waiting for server greeting.
+ * Capability Negociation
+ * QMP server is in Capabilities Negotiation mode.
+ * Waiting for a response to the "qmp_capabilities" command.
+ * Connected
+ * QMP server is in command mode, commands can be issued.
+ * There is outstanding command to be sent and/or there are in-flight
+ * libxl_ev_qmp with callback.
+ * Within A Callback
+ * The QMP client enter this state when a callback is called.
+ * The connection to QEMU is kept open until at least the callback
+ * return.
+ *
+ * Here is the transition from one state to the next:
+ *Disconnected -> Connecting
+ * Connect to the QMP socket.
+ *Connecting -> Capability Negociation
+ * Server greeting received
+ * Send "qmp_capabilities"
+ *Capability Negociation -> Connected
+ * Response to "qmp_capabilities" received"
+ *
+ *Connected -> Within A Callback
+ * When a response is received, if there's a callback associated to it,
+ * it is called.
+ *Connected -> Disconnected
+ * When both list of in-flight event and command to send are empty.
+ *
+ *Within A Callback -> Connected
+ * On return from a libxl__ev_qmp callback, there are more command to send
+ * or there are other in-flight event.
+ *Within A Callback -> Disconnected
+ * On return from a libxl__ev_qmp
Re: [Xen-devel] [PATCH v3.1] libxl: Design of an async API to issue QMP commands to QEMU
On Thu, Jul 12, 2018 at 05:36:00PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("Re: [PATCH v3.1] libxl: Design of an async API to
> issue QMP commands to QEMU"):
> > I don't think the state of a qmp connection can fit into
> > libxl__qmp_cmd_state. The "Active" state doesn't mean that a qmp
> > connection is open or that the command have been sent. It merely mean
> > that the syscall socket(3) and connect(3) have return successfully, and
> > there will be an attempt later to sent the command to qemu.
>
> I think you haven't quite understood my point.
>
> My understanding of your API is that it gives the user of
> libl__qmp_cmd a certain amount of visibility of the existence or
> otherwise of the qmp connection.
>
> You say:
>
> + * When called from within a callback, the same QMP connection will be
> + * reused to execute the new command. This is important in the case
> + * where the first command is "add-fd" and the second command use the
> + * fdset created by QEMU.
>
> This implies that at the top of the callback, the qmp connection is
> actually in some kind of extra state which is not covered by any of
> your Undefined, Idle or Active.
>
> It is not Undefined, obviously.
>
> It is not Active because no response is being awaited any longer.
> (If a response were being awaited, then it would not be sensible for
> the callback to issue another command, because your rule is one
> command at once.)
>
> And it is not Idle because it contains references to allocated
> resources - namely, the qmp connection fd.
>
> So your documented state model is too poor to express what is going
> on. You need to write down at least one additional state, which you
> might call `Connected'.
>
>
> Also, I have just noticed that you say "from within a callback". That
> suggests that the code which makes the callback does something to the
> libxl__qmp_state after after the callback returns.
>
> That is contrary to the way that everything else works in libxl. In
> libxl, such a callback is made as the last thing before returning back
> to the event loop.
>
> The reason is that the state struct (in this case the libxl__qmp_cmd)
> may be part of a larger struct, which is completed and deallocated
> somewhere in the series of (nested) callback functions. So the memory
> may not be valid any more.
>
>
> Another way to look at this is that you actually have a fourth state
> which I will call WithinCallback. On entry to the callback, the cmd
> is in WithinCallback state.
>
> In WithinCallback state, it is allowed to request that another command
> is sent (putting the cmd back into Active).
>
> But in the WithinCallback state, the libxl__qmp_cmd contains
> references to resources and may not be freed. Furthermore, as I read
> your commentary, the WithinCallback state cannot be exited other than
> to Active, or by returning from the callback.
>
> That would make it very awkward for the user of this interface to ever
> free the cmd. After all, they can only free the memory for it when
> the state is Idle or Undefined. So they need to get it into Idle
> which they can only do by returning, but then they have lost
> control...
>
>
> So this is why I say you should have a proper fourth state, which we
> can call Connected or something. On entry to the callback, the cmd is
> in state Connected. The cmd should stay Connected until it is
> explicitly disposed of.
>
> The code which makes the callback then does not need to do anything
> after making the callback: the user has sent another command; or is
> going to send another command; or has called _dispose. In any case,
> the caller has ownership.
I'll reply to that with a new patch which I hope will anwser your
comments.
> > > Also, I don't think this description of the semantics is right. The
> > > caller must always somehow arrange to initialise this value.
> > > Presumably _init clears it ? Certainly this as a parameter to the
> > > operation, this should be up with domid and callback.
> > >
> > > Maybe you want comments like the ones in libxl__datacopier_state etc.,
> > > which say /* caller must fill these in */.
> > >
> > > And, you probably want to make it clear that the fd remains open in
> > > the libxl process. (I assume it does.)
> >
> > Well I was closing the fd once it was sent to QEMU. But we can have the
> > callbacks takes care of closing it instead.
>
> I don't mind what the semantics are, but they should be clear, and all
> the error cases need to work correctly. Eg if asking to send a fd
> causes the fd to become owned by the qmp machiner, then if the attempt
> to send the qmp command fails (maybe becaue the qmp connection fails)
> then it must be closed.
>
> The semantics should probably be whichever are more convenient.
> Personally i would lean towards qmp_cmd not taking ownership, because
> if you do then someone who wants to keep a copy of the fd has to dup
> it and duping a carefd is quite a faff.
Sounds
[Xen-devel] [linux-linus test] 125167: regressions - FAIL
flight 125167 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/125167/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 123554 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 123554 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 123554 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 123554 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-examine 8 reboot fail REGR. vs. 123554 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 123554 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 123554 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 123554 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail baseline untested test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 123554 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 123554 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 123554 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 123554 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 123554 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 123554 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-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-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13
Re: [Xen-devel] [PATCH v4 6/6] tools: --with-system-{ovmf, seabios, ipxe} should provide absolute paths
Wei Liu writes ("[PATCH v4 6/6] tools: --with-system-{ovmf,seabios,ipxe} should
provide absolute paths"):
> The paths shouldn't be set to "yes". We ask the user to set absolute
> paths because Xen's build system doesn't know where to search, and the
> build machine doesn't necessarily have those binaries present in the
> first place.
>
> Reported-by: Anthony Perard
> Signed-off-by: Wei Liu
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 5/6] tools: provide --with-system-ipxe
Wei Liu writes ("[PATCH v4 5/6] tools: provide --with-system-ipxe"):
> This option lets user specify which binary is to be used as ipxe. If
> it is specified, the in-tree ipxe will not be built. This option is in
> line with other --with-system-* options we provide.
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/6] libxc: allow HVM guest to have modules
Wei Liu writes ("[PATCH v4 3/6] libxc: allow HVM guest to have modules"):
> Lift the loading code out of PVH specific branch. Take the chance to
> make the debug message more useful.
>
> Now the code needs to handle virt_base being UNSET_ADDR, which it is
> for HVM guest. In case virt_base is not set, it should be treated as
> zero. In case PVH and PV, virt_base is set by the respective loader
> by parsing the binary.
>
> IPXE will be loaded as a module of Rombios.
>
> Signed-off-by: Wei Liu
> Reviewed-by: Roger Pau Monné
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 1/6] Tools.mk.in: drop unused variables
Wei Liu writes ("[PATCH v4 1/6] Tools.mk.in: drop unused variables"):
> Signed-off-by: Wei Liu
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 03/11] x86/hvm: Introduce hvm_save_cpu_ctxt_one func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
Changes since V11:
- hvm_save_cpu_ctxt() now returns err from
hvm_save_cpu_ctxt_one().
---
xen/arch/x86/hvm/hvm.c | 216 ++---
1 file changed, 113 insertions(+), 103 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index dd88751..e20a25c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -787,119 +787,129 @@ static int hvm_load_tsc_adjust(struct domain *d,
hvm_domain_context_t *h)
HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust,
hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU);
+static int hvm_save_cpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
+{
+struct segment_register seg;
+struct hvm_hw_cpu ctxt;
+
+memset(, 0, sizeof(ctxt));
+
+/* Architecture-specific vmcs/vmcb bits */
+hvm_funcs.save_cpu_ctxt(v, );
+
+ctxt.tsc = hvm_get_guest_tsc_fixed(v, v->domain->arch.hvm_domain.sync_tsc);
+
+ctxt.msr_tsc_aux = hvm_msr_tsc_aux(v);
+
+hvm_get_segment_register(v, x86_seg_idtr, );
+ctxt.idtr_limit = seg.limit;
+ctxt.idtr_base = seg.base;
+
+hvm_get_segment_register(v, x86_seg_gdtr, );
+ctxt.gdtr_limit = seg.limit;
+ctxt.gdtr_base = seg.base;
+
+hvm_get_segment_register(v, x86_seg_cs, );
+ctxt.cs_sel = seg.sel;
+ctxt.cs_limit = seg.limit;
+ctxt.cs_base = seg.base;
+ctxt.cs_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_ds, );
+ctxt.ds_sel = seg.sel;
+ctxt.ds_limit = seg.limit;
+ctxt.ds_base = seg.base;
+ctxt.ds_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_es, );
+ctxt.es_sel = seg.sel;
+ctxt.es_limit = seg.limit;
+ctxt.es_base = seg.base;
+ctxt.es_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_ss, );
+ctxt.ss_sel = seg.sel;
+ctxt.ss_limit = seg.limit;
+ctxt.ss_base = seg.base;
+ctxt.ss_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_fs, );
+ctxt.fs_sel = seg.sel;
+ctxt.fs_limit = seg.limit;
+ctxt.fs_base = seg.base;
+ctxt.fs_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_gs, );
+ctxt.gs_sel = seg.sel;
+ctxt.gs_limit = seg.limit;
+ctxt.gs_base = seg.base;
+ctxt.gs_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_tr, );
+ctxt.tr_sel = seg.sel;
+ctxt.tr_limit = seg.limit;
+ctxt.tr_base = seg.base;
+ctxt.tr_arbytes = seg.attr;
+
+hvm_get_segment_register(v, x86_seg_ldtr, );
+ctxt.ldtr_sel = seg.sel;
+ctxt.ldtr_limit = seg.limit;
+ctxt.ldtr_base = seg.base;
+ctxt.ldtr_arbytes = seg.attr;
+
+if ( v->fpu_initialised )
+{
+memcpy(ctxt.fpu_regs, v->arch.fpu_ctxt, sizeof(ctxt.fpu_regs));
+ctxt.flags = XEN_X86_FPU_INITIALISED;
+}
+
+ctxt.rax = v->arch.user_regs.rax;
+ctxt.rbx = v->arch.user_regs.rbx;
+ctxt.rcx = v->arch.user_regs.rcx;
+ctxt.rdx = v->arch.user_regs.rdx;
+ctxt.rbp = v->arch.user_regs.rbp;
+ctxt.rsi = v->arch.user_regs.rsi;
+ctxt.rdi = v->arch.user_regs.rdi;
+ctxt.rsp = v->arch.user_regs.rsp;
+ctxt.rip = v->arch.user_regs.rip;
+ctxt.rflags = v->arch.user_regs.rflags;
+ctxt.r8 = v->arch.user_regs.r8;
+ctxt.r9 = v->arch.user_regs.r9;
+ctxt.r10 = v->arch.user_regs.r10;
+ctxt.r11 = v->arch.user_regs.r11;
+ctxt.r12 = v->arch.user_regs.r12;
+ctxt.r13 = v->arch.user_regs.r13;
+ctxt.r14 = v->arch.user_regs.r14;
+ctxt.r15 = v->arch.user_regs.r15;
+ctxt.dr0 = v->arch.debugreg[0];
+ctxt.dr1 = v->arch.debugreg[1];
+ctxt.dr2 = v->arch.debugreg[2];
+ctxt.dr3 = v->arch.debugreg[3];
+ctxt.dr6 = v->arch.debugreg[6];
+ctxt.dr7 = v->arch.debugreg[7];
+
+return hvm_save_entry(CPU, v->vcpu_id, h, );
+}
+
static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
{
struct vcpu *v;
-struct hvm_hw_cpu ctxt;
-struct segment_register seg;
+int err = 0;
for_each_vcpu ( d, v )
{
-/* We don't need to save state for a vcpu that is down; the restore
- * code will leave it down if there is nothing saved. */
+/*
+ * We don't need to save state for a vcpu that is down; the restore
+ * code will leave it down if there is nothing saved.
+ */
if ( v->pause_flags & VPF_down )
continue;
-memset(, 0, sizeof(ctxt));
-
-/* Architecture-specific vmcs/vmcb bits */
-hvm_funcs.save_cpu_ctxt(v, );
-
-ctxt.tsc = hvm_get_guest_tsc_fixed(v, d->arch.hvm_domain.sync_tsc);
-
-ctxt.msr_tsc_aux = hvm_msr_tsc_aux(v);
-
-hvm_get_segment_register(v, x86_seg_idtr, );
-ctxt.idtr_limit = seg.limit;
-ctxt.idtr_base = seg.base;
-
-hvm_get_segment_register(v, x86_seg_gdtr, );
-ctxt.gdtr_limit = seg.limit;
-ctxt.gdtr_base = seg.base;
-
-
[Xen-devel] [PATCH v12 09/11] x86/domctl: Don't pause the whole domain if only getting vcpu state
This patch is focused on moving the for loop to the caller so
now we can save info for a single vcpu instance with the save_one
handlers.
Signed-off-by: Alexandru Isaila
---
Changes since V11:
- Changed the CONTINUE return to return 0.
---
xen/arch/x86/hvm/hvm.c | 18 +++
xen/arch/x86/hvm/save.c | 137 +---
2 files changed, 115 insertions(+), 40 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c458dab..ac0b496 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -793,6 +793,13 @@ static int hvm_save_cpu_ctxt_one(struct vcpu *v,
hvm_domain_context_t *h)
struct segment_register seg;
struct hvm_hw_cpu ctxt;
+/*
+ * We don't need to save state for a vcpu that is down; the restore
+ * code will leave it down if there is nothing saved.
+ */
+if ( v->pause_flags & VPF_down )
+return 0;
+
memset(, 0, sizeof(ctxt));
/* Architecture-specific vmcs/vmcb bits */
@@ -899,13 +906,6 @@ static int hvm_save_cpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
for_each_vcpu ( d, v )
{
-/*
- * We don't need to save state for a vcpu that is down; the restore
- * code will leave it down if there is nothing saved.
- */
-if ( v->pause_flags & VPF_down )
-continue;
-
err = hvm_save_cpu_ctxt_one(v, h);
if ( err )
break;
@@ -1198,7 +1198,7 @@ static int hvm_save_cpu_xsave_states_one(struct vcpu *v,
hvm_domain_context_t *h
unsigned int size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
int err = 0;
-if ( !cpu_has_xsave )
+if ( !cpu_has_xsave || !xsave_enabled(v) )
return 0; /* do nothing */
err = _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size);
@@ -1223,8 +1223,6 @@ static int hvm_save_cpu_xsave_states(struct domain *d,
hvm_domain_context_t *h)
for_each_vcpu ( d, v )
{
-if ( !xsave_enabled(v) )
-continue;
err = hvm_save_cpu_xsave_states_one(v, h);
if ( err )
break;
diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
index b674937..d57648d 100644
--- a/xen/arch/x86/hvm/save.c
+++ b/xen/arch/x86/hvm/save.c
@@ -138,9 +138,12 @@ size_t hvm_save_size(struct domain *d)
int hvm_save_one(struct domain *d, unsigned int typecode, unsigned int
instance,
XEN_GUEST_HANDLE_64(uint8) handle, uint64_t *bufsz)
{
-int rv;
+int rv = 0;
hvm_domain_context_t ctxt = { };
const struct hvm_save_descriptor *desc;
+bool is_single_instance = false;
+uint32_t off = 0;
+struct vcpu *v;
if ( d->is_dying ||
typecode > HVM_SAVE_CODE_MAX ||
@@ -148,43 +151,94 @@ int hvm_save_one(struct domain *d, unsigned int typecode,
unsigned int instance,
!hvm_sr_handlers[typecode].save )
return -EINVAL;
+if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU &&
+instance < d->max_vcpus )
+is_single_instance = true;
+
ctxt.size = hvm_sr_handlers[typecode].size;
-if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU )
+if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU &&
+instance == d->max_vcpus )
ctxt.size *= d->max_vcpus;
ctxt.data = xmalloc_bytes(ctxt.size);
if ( !ctxt.data )
return -ENOMEM;
-if ( (rv = hvm_sr_handlers[typecode].save(d, )) != 0 )
-printk(XENLOG_G_ERR "HVM%d save: failed to save type %"PRIu16" (%d)\n",
- d->domain_id, typecode, rv);
-else if ( rv = -ENOENT, ctxt.cur >= sizeof(*desc) )
+if ( is_single_instance )
+vcpu_pause(d->vcpu[instance]);
+else
+domain_pause(d);
+
+if ( is_single_instance )
{
-uint32_t off;
+if ( hvm_sr_handlers[typecode].save_one != NULL )
+rv = hvm_sr_handlers[typecode].save_one(d->vcpu[instance],
+);
+else
+rv = hvm_sr_handlers[typecode].save(d, );
-for ( off = 0; off <= (ctxt.cur - sizeof(*desc)); off += desc->length )
+if ( rv != 0 )
{
-desc = (void *)(ctxt.data + off);
-/* Move past header */
-off += sizeof(*desc);
-if ( ctxt.cur < desc->length ||
- off > ctxt.cur - desc->length )
-break;
-if ( instance == desc->instance )
-{
-rv = 0;
-if ( guest_handle_is_null(handle) )
-*bufsz = desc->length;
-else if ( *bufsz < desc->length )
-rv = -ENOBUFS;
-else if ( copy_to_guest(handle, ctxt.data + off, desc->length)
)
-rv = -EFAULT;
-else
-*bufsz = desc->length;
-break;
-}
+printk(XENLOG_G_ERR "HVM%d save: failed to save type
[Xen-devel] [PATCH v12 05/11] x86/hvm: Introduce hvm_save_cpu_msrs_one func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
Reviewed-by: Paul Durrant
---
Changes since V11:
- hvm_save_cpu_msrs() returns err from hvm_save_cpu_msrs_one().
---
xen/arch/x86/hvm/hvm.c | 105 +++--
1 file changed, 58 insertions(+), 47 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 1da6a26..70d1b20 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1368,69 +1368,80 @@ static const uint32_t msrs_to_send[] = {
};
static unsigned int __read_mostly msr_count_max = ARRAY_SIZE(msrs_to_send);
-static int hvm_save_cpu_msrs(struct domain *d, hvm_domain_context_t *h)
+static int hvm_save_cpu_msrs_one(struct vcpu *v, hvm_domain_context_t *h)
{
-struct vcpu *v;
+struct hvm_save_descriptor *desc = _p(>data[h->cur]);
+struct hvm_msr *ctxt;
+unsigned int i;
+int err = 0;
-for_each_vcpu ( d, v )
+err = _hvm_init_entry(h, CPU_MSR_CODE, v->vcpu_id,
+ HVM_CPU_MSR_SIZE(msr_count_max));
+if ( err )
+return err;
+ctxt = (struct hvm_msr *)>data[h->cur];
+ctxt->count = 0;
+
+for ( i = 0; i < ARRAY_SIZE(msrs_to_send); ++i )
{
-struct hvm_save_descriptor *desc = _p(>data[h->cur]);
-struct hvm_msr *ctxt;
-unsigned int i;
+uint64_t val;
+int rc = guest_rdmsr(v, msrs_to_send[i], );
-if ( _hvm_init_entry(h, CPU_MSR_CODE, v->vcpu_id,
- HVM_CPU_MSR_SIZE(msr_count_max)) )
-return 1;
-ctxt = (struct hvm_msr *)>data[h->cur];
-ctxt->count = 0;
+/*
+ * It is the programmers responsibility to ensure that
+ * msrs_to_send[] contain generally-read/write MSRs.
+ * X86EMUL_EXCEPTION here implies a missing feature, and that the
+ * guest doesn't have access to the MSR.
+ */
+if ( rc == X86EMUL_EXCEPTION )
+continue;
-for ( i = 0; i < ARRAY_SIZE(msrs_to_send); ++i )
+if ( rc != X86EMUL_OKAY )
{
-uint64_t val;
-int rc = guest_rdmsr(v, msrs_to_send[i], );
+ASSERT_UNREACHABLE();
+return -ENXIO;
+}
-/*
- * It is the programmers responsibility to ensure that
- * msrs_to_send[] contain generally-read/write MSRs.
- * X86EMUL_EXCEPTION here implies a missing feature, and that the
- * guest doesn't have access to the MSR.
- */
-if ( rc == X86EMUL_EXCEPTION )
-continue;
+if ( !val )
+continue; /* Skip empty MSRs. */
-if ( rc != X86EMUL_OKAY )
-{
-ASSERT_UNREACHABLE();
-return -ENXIO;
-}
+ctxt->msr[ctxt->count].index = msrs_to_send[i];
+ctxt->msr[ctxt->count++].val = val;
+}
-if ( !val )
-continue; /* Skip empty MSRs. */
+if ( hvm_funcs.save_msr )
+hvm_funcs.save_msr(v, ctxt);
-ctxt->msr[ctxt->count].index = msrs_to_send[i];
-ctxt->msr[ctxt->count++].val = val;
-}
+ASSERT(ctxt->count <= msr_count_max);
-if ( hvm_funcs.save_msr )
-hvm_funcs.save_msr(v, ctxt);
+for ( i = 0; i < ctxt->count; ++i )
+ctxt->msr[i]._rsvd = 0;
-ASSERT(ctxt->count <= msr_count_max);
+if ( ctxt->count )
+{
+/* Rewrite length to indicate how much space we actually used. */
+desc->length = HVM_CPU_MSR_SIZE(ctxt->count);
+h->cur += HVM_CPU_MSR_SIZE(ctxt->count);
+}
+else
+/* or rewind and remove the descriptor from the stream. */
+h->cur -= sizeof(struct hvm_save_descriptor);
+return 0;
+}
-for ( i = 0; i < ctxt->count; ++i )
-ctxt->msr[i]._rsvd = 0;
+static int hvm_save_cpu_msrs(struct domain *d, hvm_domain_context_t *h)
+{
+struct vcpu *v;
+int err = 0;
-if ( ctxt->count )
-{
-/* Rewrite length to indicate how much space we actually used. */
-desc->length = HVM_CPU_MSR_SIZE(ctxt->count);
-h->cur += HVM_CPU_MSR_SIZE(ctxt->count);
-}
-else
-/* or rewind and remove the descriptor from the stream. */
-h->cur -= sizeof(struct hvm_save_descriptor);
+for_each_vcpu ( d, v )
+{
+err = hvm_save_cpu_msrs_one(v, h);
+if ( err )
+break;
}
-return 0;
+return err;
}
static int hvm_load_cpu_msrs(struct domain *d, hvm_domain_context_t *h)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 06/11] x86/hvm: Introduce hvm_save_mtrr_msr_one func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
Changes since v11:
- hvm_save_mtrr_msr() now returns err from hvm_save_mtrr_msr_one().
Note: This patch is based on Roger Pau Monne's series[1]
---
xen/arch/x86/hvm/mtrr.c | 81 +++--
1 file changed, 44 insertions(+), 37 deletions(-)
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index 48facbb..47a5c29 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -718,52 +718,59 @@ int hvm_set_mem_pinned_cacheattr(struct domain *d,
uint64_t gfn_start,
return 0;
}
-static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
+static int hvm_save_mtrr_msr_one(struct vcpu *v, hvm_domain_context_t *h)
{
-struct vcpu *v;
+const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
+struct hvm_hw_mtrr hw_mtrr = {
+.msr_mtrr_def_type = mtrr_state->def_type |
+ MASK_INSR(mtrr_state->fixed_enabled,
+ MTRRdefType_FE) |
+ MASK_INSR(mtrr_state->enabled, MTRRdefType_E),
+.msr_mtrr_cap = mtrr_state->mtrr_cap,
+};
+unsigned int i;
-/* save mtrr */
-for_each_vcpu(d, v)
+if ( MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT) >
+ (ARRAY_SIZE(hw_mtrr.msr_mtrr_var) / 2) )
{
-const struct mtrr_state *mtrr_state = >arch.hvm_vcpu.mtrr;
-struct hvm_hw_mtrr hw_mtrr = {
-.msr_mtrr_def_type = mtrr_state->def_type |
- MASK_INSR(mtrr_state->fixed_enabled,
- MTRRdefType_FE) |
- MASK_INSR(mtrr_state->enabled, MTRRdefType_E),
-.msr_mtrr_cap = mtrr_state->mtrr_cap,
-};
-unsigned int i;
+dprintk(XENLOG_G_ERR,
+"HVM save: %pv: too many (%lu) variable range MTRRs\n",
+v, MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT));
+return -EINVAL;
+}
-if ( MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT) >
- (ARRAY_SIZE(hw_mtrr.msr_mtrr_var) / 2) )
-{
-dprintk(XENLOG_G_ERR,
-"HVM save: %pv: too many (%lu) variable range MTRRs\n",
-v, MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT));
-return -EINVAL;
-}
+hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
-hvm_get_guest_pat(v, _mtrr.msr_pat_cr);
+for ( i = 0; i < MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT); i++ )
+{
+/* save physbase */
+hw_mtrr.msr_mtrr_var[i*2] =
+((uint64_t*)mtrr_state->var_ranges)[i*2];
+/* save physmask */
+hw_mtrr.msr_mtrr_var[i*2+1] =
+((uint64_t*)mtrr_state->var_ranges)[i*2+1];
+}
-for ( i = 0; i < MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT); i++ )
-{
-/* save physbase */
-hw_mtrr.msr_mtrr_var[i*2] =
-((uint64_t*)mtrr_state->var_ranges)[i*2];
-/* save physmask */
-hw_mtrr.msr_mtrr_var[i*2+1] =
-((uint64_t*)mtrr_state->var_ranges)[i*2+1];
-}
+for ( i = 0; i < NUM_FIXED_MSR; i++ )
+hw_mtrr.msr_mtrr_fixed[i] =
+((uint64_t*)mtrr_state->fixed_ranges)[i];
-for ( i = 0; i < NUM_FIXED_MSR; i++ )
-hw_mtrr.msr_mtrr_fixed[i] =
-((uint64_t*)mtrr_state->fixed_ranges)[i];
+return hvm_save_entry(MTRR, v->vcpu_id, h, _mtrr);
+}
-if ( hvm_save_entry(MTRR, v->vcpu_id, h, _mtrr) != 0 )
-return 1;
+static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
+{
+struct vcpu *v;
+int err = 0;
+
+/* save mtrr */
+for_each_vcpu(d, v)
+{
+ err = hvm_save_mtrr_msr_one(v, h);
+ if ( err )
+ break;
}
-return 0;
+return err;
}
static int hvm_load_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 00/11] x86/domctl: Save info for one vcpu instance
Hi all, This patch series addresses the ideea of saving data from a single vcpu instance. First it starts by adding *save_one functions, then it introduces a handler for the new save_one* funcs and makes use of it in the hvm_save and hvm_save_one funcs. The final 2 patches are used for clean up. The first one removes the save* funcs and renames the save_one* to save. The last patch removes the save_one* handler that is no longer used. Cheers, Alexandru Isaila (11): x86/cpu: Introduce vmce_save_vcpu_ctxt_one() func x86/hvm: Introduce hvm_save_tsc_adjust_one() func x86/hvm: Introduce hvm_save_cpu_ctxt_one func x86/hvm: Introduce hvm_save_cpu_xsave_states_one x86/hvm: Introduce hvm_save_cpu_msrs_one func x86/hvm: Introduce hvm_save_mtrr_msr_one func x86/hvm: Introduce viridian_save_vcpu_ctxt_one() x86/hvm: Add handler for save_one funcs x86/domctl: Don't pause the whole domain if only x86/hvm: Remove redundant save functions x86/hvm: Remove save_one handler ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 07/11] x86/hvm: Introduce viridian_save_vcpu_ctxt_one() func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
xen/arch/x86/hvm/viridian.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 694eae6..f3430bb 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1026,24 +1026,33 @@ static int viridian_load_domain_ctxt(struct domain *d,
hvm_domain_context_t *h)
HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_DOMAIN, viridian_save_domain_ctxt,
viridian_load_domain_ctxt, 1, HVMSR_PER_DOM);
-static int viridian_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
+static int viridian_save_vcpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
{
-struct vcpu *v;
+struct hvm_viridian_vcpu_context ctxt = {};
-if ( !is_viridian_domain(d) )
+if ( !is_viridian_domain(v->domain) )
return 0;
+ctxt.vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw;
+ctxt.vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending;
+
+return hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, );
+}
+
+static int viridian_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
+{
+struct vcpu *v;
+int err = 0;
+
for_each_vcpu( d, v ) {
-struct hvm_viridian_vcpu_context ctxt = {
-.vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw,
-.vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending,
-};
-if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, ) != 0 )
-return 1;
+err = viridian_save_vcpu_ctxt_one(v, h);
+
+if ( err )
+break;
}
-return 0;
+return err;
}
static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 01/11] x86/cpu: Introduce vmce_save_vcpu_ctxt_one() func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
Changes since V11:
- Removed the memset and added init with {}.
---
xen/arch/x86/cpu/mcheck/vmce.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index e07cd2f..31e553c 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -349,6 +349,18 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
return ret;
}
+static int vmce_save_vcpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
+{
+struct hvm_vmce_vcpu ctxt = {
+.caps = v->arch.vmce.mcg_cap,
+.mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2,
+.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2,
+.mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl,
+};
+
+return hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, );
+}
+
static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
{
struct vcpu *v;
@@ -356,14 +368,7 @@ static int vmce_save_vcpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
for_each_vcpu ( d, v )
{
-struct hvm_vmce_vcpu ctxt = {
-.caps = v->arch.vmce.mcg_cap,
-.mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2,
-.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2,
-.mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl,
-};
-
-err = hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, );
+err = vmce_save_vcpu_ctxt_one(v, h);
if ( err )
break;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 08/11] x86/hvm: Add handler for save_one funcs
Signed-off-by: Alexandru Isaila
---
Changes since V8:
- Add comment for the handler return values.
---
xen/arch/x86/cpu/mcheck/vmce.c | 1 +
xen/arch/x86/hvm/hpet.c| 2 +-
xen/arch/x86/hvm/hvm.c | 6 +-
xen/arch/x86/hvm/i8254.c | 2 +-
xen/arch/x86/hvm/irq.c | 6 +++---
xen/arch/x86/hvm/mtrr.c| 4 ++--
xen/arch/x86/hvm/pmtimer.c | 2 +-
xen/arch/x86/hvm/rtc.c | 2 +-
xen/arch/x86/hvm/save.c| 5 -
xen/arch/x86/hvm/vioapic.c | 2 +-
xen/arch/x86/hvm/viridian.c| 3 ++-
xen/arch/x86/hvm/vlapic.c | 4 ++--
xen/arch/x86/hvm/vpic.c| 2 +-
xen/include/asm-x86/hvm/save.h | 6 +-
14 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 31e553c..35044d7 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -396,6 +396,7 @@ static int vmce_load_vcpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
+ vmce_save_vcpu_ctxt_one,
vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
/*
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 2837709..aff8613 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -640,7 +640,7 @@ static int hpet_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, hpet_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, NULL, hpet_load, 1, HVMSR_PER_DOM);
static void hpet_set(HPETState *h)
{
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 70d1b20..c458dab 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -785,6 +785,7 @@ static int hvm_load_tsc_adjust(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust,
+ hvm_save_tsc_adjust_one,
hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU);
static int hvm_save_cpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
@@ -1183,7 +1184,8 @@ static int hvm_load_cpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt, hvm_load_cpu_ctxt,
+HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt, hvm_save_cpu_ctxt_one,
+ hvm_load_cpu_ctxt,
1, HVMSR_PER_VCPU);
#define HVM_CPU_XSAVE_SIZE(xcr0) (offsetof(struct hvm_hw_cpu_xsave, \
@@ -1536,6 +1538,7 @@ static int __init hvm_register_CPU_save_and_restore(void)
hvm_register_savevm(CPU_XSAVE_CODE,
"CPU_XSAVE",
hvm_save_cpu_xsave_states,
+hvm_save_cpu_xsave_states_one,
hvm_load_cpu_xsave_states,
HVM_CPU_XSAVE_SIZE(xfeature_mask) +
sizeof(struct hvm_save_descriptor),
@@ -1548,6 +1551,7 @@ static int __init hvm_register_CPU_save_and_restore(void)
hvm_register_savevm(CPU_MSR_CODE,
"CPU_MSR",
hvm_save_cpu_msrs,
+hvm_save_cpu_msrs_one,
hvm_load_cpu_msrs,
HVM_CPU_MSR_SIZE(msr_count_max) +
sizeof(struct hvm_save_descriptor),
diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c
index 992f08d..ec77b23 100644
--- a/xen/arch/x86/hvm/i8254.c
+++ b/xen/arch/x86/hvm/i8254.c
@@ -437,7 +437,7 @@ static int pit_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, NULL, pit_load, 1, HVMSR_PER_DOM);
void pit_reset(struct domain *d)
{
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index c85d004..770eab7 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -764,9 +764,9 @@ static int irq_load_link(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(PCI_IRQ, irq_save_pci, irq_load_pci,
+HVM_REGISTER_SAVE_RESTORE(PCI_IRQ, irq_save_pci, NULL, irq_load_pci,
1, HVMSR_PER_DOM);
-HVM_REGISTER_SAVE_RESTORE(ISA_IRQ, irq_save_isa, irq_load_isa,
+HVM_REGISTER_SAVE_RESTORE(ISA_IRQ, irq_save_isa, NULL, irq_load_isa,
1, HVMSR_PER_DOM);
-HVM_REGISTER_SAVE_RESTORE(PCI_LINK, irq_save_link, irq_load_link,
+HVM_REGISTER_SAVE_RESTORE(PCI_LINK, irq_save_link, NULL, irq_load_link,
1, HVMSR_PER_DOM);
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index 47a5c29..1cb2a2e 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -823,8 +823,8 @@ static int hvm_load_mtrr_msr(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
[Xen-devel] [PATCH v12 02/11] x86/hvm: Introduce hvm_save_tsc_adjust_one() func
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
Changes since V9:
- Change return of the save_one func to return hvm_save_entry.
---
xen/arch/x86/hvm/hvm.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 93092d2..dd88751 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -740,16 +740,23 @@ void hvm_domain_destroy(struct domain *d)
destroy_vpci_mmcfg(d);
}
+static int hvm_save_tsc_adjust_one(struct vcpu *v, hvm_domain_context_t *h)
+{
+struct hvm_tsc_adjust ctxt = {};
+
+ctxt.tsc_adjust = v->arch.hvm_vcpu.msr_tsc_adjust;
+
+return hvm_save_entry(TSC_ADJUST, v->vcpu_id, h, );
+}
+
static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h)
{
struct vcpu *v;
-struct hvm_tsc_adjust ctxt;
int err = 0;
for_each_vcpu ( d, v )
{
-ctxt.tsc_adjust = v->arch.hvm_vcpu.msr_tsc_adjust;
-err = hvm_save_entry(TSC_ADJUST, v->vcpu_id, h, );
+err = hvm_save_tsc_adjust_one(v, h);
if ( err )
break;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v12 11/11] x86/hvm: Remove save_one handler
Signed-off-by: Alexandru Isaila
---
xen/arch/x86/cpu/mcheck/vmce.c | 1 -
xen/arch/x86/hvm/hpet.c| 2 +-
xen/arch/x86/hvm/hvm.c | 5 +
xen/arch/x86/hvm/i8254.c | 2 +-
xen/arch/x86/hvm/irq.c | 6 +++---
xen/arch/x86/hvm/mtrr.c| 2 +-
xen/arch/x86/hvm/pmtimer.c | 2 +-
xen/arch/x86/hvm/rtc.c | 2 +-
xen/arch/x86/hvm/save.c| 14 +++---
xen/arch/x86/hvm/vioapic.c | 2 +-
xen/arch/x86/hvm/viridian.c| 3 +--
xen/arch/x86/hvm/vlapic.c | 4 ++--
xen/arch/x86/hvm/vpic.c| 2 +-
xen/include/asm-x86/hvm/save.h | 6 +-
14 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index b53ad7c..763d56b 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -381,7 +381,6 @@ static int vmce_load_vcpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
- NULL,
vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
/*
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 6e7f744..3ed6547 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -641,7 +641,7 @@ static int hpet_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, NULL, hpet_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, hpet_load, 1, HVMSR_PER_DOM);
static void hpet_set(HPETState *h)
{
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d7bc877..37553d0 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -770,7 +770,6 @@ static int hvm_load_tsc_adjust(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust,
- NULL,
hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU);
static int hvm_save_cpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
@@ -1155,7 +1154,7 @@ static int hvm_load_cpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt, NULL,
+HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt,
hvm_load_cpu_ctxt,
1, HVMSR_PER_VCPU);
@@ -1477,7 +1476,6 @@ static int __init hvm_register_CPU_save_and_restore(void)
hvm_register_savevm(CPU_XSAVE_CODE,
"CPU_XSAVE",
hvm_save_cpu_xsave_states,
-NULL,
hvm_load_cpu_xsave_states,
HVM_CPU_XSAVE_SIZE(xfeature_mask) +
sizeof(struct hvm_save_descriptor),
@@ -1490,7 +1488,6 @@ static int __init hvm_register_CPU_save_and_restore(void)
hvm_register_savevm(CPU_MSR_CODE,
"CPU_MSR",
hvm_save_cpu_msrs,
-NULL,
hvm_load_cpu_msrs,
HVM_CPU_MSR_SIZE(msr_count_max) +
sizeof(struct hvm_save_descriptor),
diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c
index d51463d..e0d2255 100644
--- a/xen/arch/x86/hvm/i8254.c
+++ b/xen/arch/x86/hvm/i8254.c
@@ -438,7 +438,7 @@ static int pit_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, NULL, pit_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load, 1, HVMSR_PER_DOM);
void pit_reset(struct domain *d)
{
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index a405e7f..b37275c 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -767,9 +767,9 @@ static int irq_load_link(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(PCI_IRQ, irq_save_pci, NULL, irq_load_pci,
+HVM_REGISTER_SAVE_RESTORE(PCI_IRQ, irq_save_pci, irq_load_pci,
1, HVMSR_PER_DOM);
-HVM_REGISTER_SAVE_RESTORE(ISA_IRQ, irq_save_isa, NULL, irq_load_isa,
+HVM_REGISTER_SAVE_RESTORE(ISA_IRQ, irq_save_isa, irq_load_isa,
1, HVMSR_PER_DOM);
-HVM_REGISTER_SAVE_RESTORE(PCI_LINK, irq_save_link, NULL, irq_load_link,
+HVM_REGISTER_SAVE_RESTORE(PCI_LINK, irq_save_link, irq_load_link,
1, HVMSR_PER_DOM);
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index b7c9bd6..d9a4532 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -808,7 +808,7 @@ static int hvm_load_mtrr_msr(struct domain *d,
hvm_domain_context_t *h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, NULL,
+HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr,
hvm_load_mtrr_msr, 1, HVMSR_PER_VCPU);
void memory_type_changed(struct domain *d)
diff --git
[Xen-devel] [PATCH v12 10/11] x86/hvm: Remove redundant save functions
This patch removes the redundant save functions and renames the
save_one* to save. It then changes the domain param to vcpu in the
save funcs.
Signed-off-by: Alexandru Isaila
---
Changes since V11:
- Remove enum return type for save funcs.
---
xen/arch/x86/cpu/mcheck/vmce.c | 19 ++-
xen/arch/x86/hvm/hpet.c| 3 +-
xen/arch/x86/hvm/hvm.c | 75 +-
xen/arch/x86/hvm/i8254.c | 3 +-
xen/arch/x86/hvm/irq.c | 9 +++--
xen/arch/x86/hvm/mtrr.c| 19 ++-
xen/arch/x86/hvm/pmtimer.c | 3 +-
xen/arch/x86/hvm/rtc.c | 3 +-
xen/arch/x86/hvm/save.c| 26 +++
xen/arch/x86/hvm/vioapic.c | 3 +-
xen/arch/x86/hvm/viridian.c| 23 +++--
xen/arch/x86/hvm/vlapic.c | 34 ++-
xen/arch/x86/hvm/vpic.c| 3 +-
xen/include/asm-x86/hvm/save.h | 2 +-
14 files changed, 50 insertions(+), 175 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 35044d7..b53ad7c 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -349,7 +349,7 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
return ret;
}
-static int vmce_save_vcpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
+static int vmce_save_vcpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
{
struct hvm_vmce_vcpu ctxt = {
.caps = v->arch.vmce.mcg_cap,
@@ -361,21 +361,6 @@ static int vmce_save_vcpu_ctxt_one(struct vcpu *v,
hvm_domain_context_t *h)
return hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, );
}
-static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
-{
-struct vcpu *v;
-int err = 0;
-
-for_each_vcpu ( d, v )
-{
-err = vmce_save_vcpu_ctxt_one(v, h);
-if ( err )
-break;
-}
-
-return err;
-}
-
static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
{
unsigned int vcpuid = hvm_load_instance(h);
@@ -396,7 +381,7 @@ static int vmce_load_vcpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
- vmce_save_vcpu_ctxt_one,
+ NULL,
vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
/*
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index aff8613..6e7f744 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -516,8 +516,9 @@ static const struct hvm_mmio_ops hpet_mmio_ops = {
};
-static int hpet_save(struct domain *d, hvm_domain_context_t *h)
+static int hpet_save(struct vcpu *vcpu, hvm_domain_context_t *h)
{
+struct domain *d = vcpu->domain;
HPETState *hp = domain_vhpet(d);
struct vcpu *v = pt_global_vcpu_target(d);
int rc;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ac0b496..d7bc877 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -740,7 +740,7 @@ void hvm_domain_destroy(struct domain *d)
destroy_vpci_mmcfg(d);
}
-static int hvm_save_tsc_adjust_one(struct vcpu *v, hvm_domain_context_t *h)
+static int hvm_save_tsc_adjust(struct vcpu *v, hvm_domain_context_t *h)
{
struct hvm_tsc_adjust ctxt = {};
@@ -749,21 +749,6 @@ static int hvm_save_tsc_adjust_one(struct vcpu *v,
hvm_domain_context_t *h)
return hvm_save_entry(TSC_ADJUST, v->vcpu_id, h, );
}
-static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h)
-{
-struct vcpu *v;
-int err = 0;
-
-for_each_vcpu ( d, v )
-{
-err = hvm_save_tsc_adjust_one(v, h);
-if ( err )
-break;
-}
-
-return err;
-}
-
static int hvm_load_tsc_adjust(struct domain *d, hvm_domain_context_t *h)
{
unsigned int vcpuid = hvm_load_instance(h);
@@ -785,10 +770,10 @@ static int hvm_load_tsc_adjust(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(TSC_ADJUST, hvm_save_tsc_adjust,
- hvm_save_tsc_adjust_one,
+ NULL,
hvm_load_tsc_adjust, 1, HVMSR_PER_VCPU);
-static int hvm_save_cpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
+static int hvm_save_cpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
{
struct segment_register seg;
struct hvm_hw_cpu ctxt;
@@ -899,20 +884,6 @@ static int hvm_save_cpu_ctxt_one(struct vcpu *v,
hvm_domain_context_t *h)
return hvm_save_entry(CPU, v->vcpu_id, h, );
}
-static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
-{
-struct vcpu *v;
-int err = 0;
-
-for_each_vcpu ( d, v )
-{
-err = hvm_save_cpu_ctxt_one(v, h);
-if ( err )
-break;
-}
-return err;
-}
-
/* Return a string indicating the error, or NULL for valid. */
const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
signed int cr0_pg)
@@ -1184,7 +1155,7 @@ static int
[Xen-devel] [PATCH v12 04/11] x86/hvm: Introduce hvm_save_cpu_xsave_states_one
This is used to save data from a single instance.
Signed-off-by: Alexandru Isaila
---
Changes since V11:
- hvm_save_cpu_xsave_states_one() returns the err from
_hvm_init_entry().
- hvm_save_cpu_xsave_states() returns err from
hvm_save_cpu_xsave_states_one();
---
xen/arch/x86/hvm/hvm.c | 42 +++---
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e20a25c..1da6a26 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1190,33 +1190,45 @@ HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt,
hvm_load_cpu_ctxt,
save_area) + \
xstate_ctxt_size(xcr0))
-static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
+static int hvm_save_cpu_xsave_states_one(struct vcpu *v, hvm_domain_context_t
*h)
{
-struct vcpu *v;
struct hvm_hw_cpu_xsave *ctxt;
+unsigned int size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
+int err = 0;
if ( !cpu_has_xsave )
return 0; /* do nothing */
+err = _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size);
+if ( err )
+return err;
+
+ctxt = (struct hvm_hw_cpu_xsave *)>data[h->cur];
+h->cur += size;
+ctxt->xfeature_mask = xfeature_mask;
+ctxt->xcr0 = v->arch.xcr0;
+ctxt->xcr0_accum = v->arch.xcr0_accum;
+
+expand_xsave_states(v, >save_area,
+size - offsetof(typeof(*ctxt), save_area));
+return 0;
+}
+
+static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
+{
+struct vcpu *v;
+int err = 0;
+
for_each_vcpu ( d, v )
{
-unsigned int size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
-
if ( !xsave_enabled(v) )
continue;
-if ( _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size) )
-return 1;
-ctxt = (struct hvm_hw_cpu_xsave *)>data[h->cur];
-h->cur += size;
-
-ctxt->xfeature_mask = xfeature_mask;
-ctxt->xcr0 = v->arch.xcr0;
-ctxt->xcr0_accum = v->arch.xcr0_accum;
-expand_xsave_states(v, >save_area,
-size - offsetof(typeof(*ctxt), save_area));
+err = hvm_save_cpu_xsave_states_one(v, h);
+if ( err )
+break;
}
-return 0;
+return err;
}
/*
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/8] cpupools: fix state when downing a CPU failed
On 16/07/18 16:26, Jan Beulich wrote: On 16.07.18 at 16:21, wrote: >> So maybe changing the condition in cpupool_cpu_add() should be split out >> into a patch of its own in order to be able to backport it? > > We'll want/need to backport this change anyway. Okay, so even if its a bugfix on its own I'm fine with keeping it in the patch. In case you like splitting the patches better I'm fine, too. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 125235: regressions - FAIL
flight 125235 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/125235/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 125151 build-amd64 6 xen-buildfail REGR. vs. 125151 build-i386-xsm6 xen-buildfail REGR. vs. 125151 build-i3866 xen-buildfail REGR. vs. 125151 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a version targeted for testing: ovmf 2add3cffb075de8a705c9d887dd25624c89d2349 baseline version: ovmf ae08ea246fe9b4a4e05b7ee6cdbd5b0fa38f3f69 Last test of basis 125151 2018-07-13 09:40:50 Z3 days Failing since125194 2018-07-16 02:40:42 Z0 days 10 attempts Testing same since 125197 2018-07-16 03:40:38 Z0 days9 attempts People who touched revisions under test: Bob Feng BobCF Dandan Bi Gary Lin jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 321 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/8] cpupools: fix state when downing a CPU failed
>>> On 16.07.18 at 16:21, wrote: > So maybe changing the condition in cpupool_cpu_add() should be split out > into a patch of its own in order to be able to backport it? We'll want/need to backport this change anyway. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/8] cpupools: fix state when downing a CPU failed
On 16/07/18 15:01, Jan Beulich wrote:
On 16.07.18 at 14:47, wrote:
>> On 16/07/18 14:19, Jan Beulich wrote:
>> On 16.07.18 at 13:47, wrote:
On 16/07/18 11:17, Jan Beulich wrote:
On 13.07.18 at 11:02, wrote:
>> On 11/07/18 14:04, Jan Beulich wrote:
>>> While I've run into the issue with further patches in place which no
>>> longer guarantee the per-CPU area to start out as all zeros, the
>>> CPU_DOWN_FAILED processing looks to have the same issue: By not zapping
>>> the per-CPU cpupool pointer, cpupool_cpu_add()'s (indirect) invocation
>>> of schedule_cpu_switch() will trigger the "c != old_pool" assertion
>>> there.
>>>
>>> Clearing the field during CPU_DOWN_PREPARE is too early (afaict this
>>> should not happen before cpu_disable_scheduler()). Clearing it in
>>> CPU_DEAD and CPU_DOWN_FAILED would be an option, but would take the same
>>> piece of code twice. Since the field's value shouldn't matter while the
>>> CPU is offline, simply clear it in CPU_ONLINE and CPU_DOWN_FAILED, but
>>> only for other than the suspend/resume case (which gets specially
>>> handled in cpupool_cpu_remove()).
>>>
>>> Signed-off-by: Jan Beulich
>>> ---
>>> TBD: I think this would better call schedule_cpu_switch(cpu, NULL) from
>>> cpupool_cpu_remove(), but besides that - as per above - likely
>>> being too early, that function has further prereqs to be met. It
>>> also doesn't look as if cpupool_unassign_cpu_helper() could be used
>>> there.
>>>
>>> --- a/xen/common/cpupool.c
>>> +++ b/xen/common/cpupool.c
>>> @@ -778,6 +778,8 @@ static int cpu_callback(
>>> {
>>> case CPU_DOWN_FAILED:
>>> case CPU_ONLINE:
>>> +if ( system_state <= SYS_STATE_active )
>>> +per_cpu(cpupool, cpu) = NULL;
>>> rc = cpupool_cpu_add(cpu);
>>
>> Wouldn't it make more sense to clear the field in cpupool_cpu_add()
>> which already is testing system_state?
>
> Hmm, this may be a matter of taste: I consider the change done here
> a prereq to calling the function in the first place. As said in the
> description, I actually think this should come earlier, and it's just that
> I can't see how to cleanly do so.
>>>
>>> You didn't comment on this one at all, yet it matters for how a v2
>>> is supposed to look like.
>>
>> My comment was thought to address this question, too. cpupool_cpu_add()
>> is handling the special case of resuming explicitly, where the old cpu
>> assignment to a cpupool is kept. So I believe setting
>> per_cpu(cpupool, cpu) = NULL
>> in the else clause of cpupool_cpu_add() only is better.
>
> Well, okay then. You're the maintainer.
>
>> Modifying the condition in cpupool_cpu_add() to
>>
>> if ( system_state <= SYS_STATE_active )
>>
>> at the same time would have the benefit to catch problems in case
>> suspending cpus is failing during SYS_STATE_suspend (I'd expect
>> triggering the first ASSERT in schedule_cpu_switch() in this case).
>
> You mean the if() there, not the else? If so - how would the "else"
> body then ever be reached? IOW if anything I could only see the
> "else" to become "else if ( system_state <= SYS_STATE_active )".
Bad wording on my side.
I should have written "the condition in cpupool_cpu_add() should match
if ( system_state <= SYS_STATE_active )."
So: "if ( system_state > SYS_STATE_active )", as the test is for the
other case.
>>>
>>> I'd recommend against this, as someone adding a new SYS_STATE_*
>>> past suspend/resume would quite likely miss this one. The strong
>>> ordering of states imo should only be used for active and lower states.
>>> But yes, I could see the if() there to become suspend || resume to
>>> address the problem you describe.
>>
>> Yes, this would seem to be a better choice here.
>>
>>> Coming back to your DOWN_FAILED consideration: Why are you
>>> thinking this can't happen during suspend? disable_nonboot_cpus()
>>> uses plain cpu_down() after all.
>>
>> Right.
>>
>> DOWN_FAILED is used only once, and that is in cpu_down() after the step
>> CPU_DOWN_PREPARE returned an error. And CPU_DOWN_PREPARE is only used
>> for cpufreq driver where it never returns an error, and for cpupools
>> which don't matter here, as only other components failing at step
>> CPU_DOWN_PREPARE would lead to calling cpupool/DOWN_FAILED.
>
> What about the stop_machine_run() failure case?
Oh. No idea how I missed that.
So maybe changing the condition in cpupool_cpu_add() should be split out
into a patch of its own in order to be able to backport it?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/13] public / x86: introduce __HYPERCALL_iommu_op
> -Original Message- > From: Wei Liu [mailto:wei.l...@citrix.com] > Sent: 16 July 2018 15:14 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > ; George Dunlap > ; Ian Jackson ; Jan > Beulich ; Konrad Rzeszutek Wilk > ; Stefano Stabellini ; Tim > (Xen.org) ; Wei Liu ; Daniel De Graaf > > Subject: Re: [PATCH v2 06/13] public / x86: introduce > __HYPERCALL_iommu_op > > On Sat, Jul 07, 2018 at 12:05:19PM +0100, Paul Durrant wrote: > > This patch introduces the boilerplate for a new hypercall to allow a > > domain to control IOMMU mappings for its own pages. > > Whilst there is duplication of code between the native and compat entry > > points which appears ripe for some form of combination, I think it is > > better to maintain the separation as-is because the compat entry point > > will necessarily gain complexity in subsequent patches. > > > > Since this is a new op, why is there compatibility issue? Can we do away > with the compat entry point all together? > > I haven't read the rest of this series, maybe it will become clearer > later. Look at the patch adding the query op... that's when it becomes necessary. Paul > > Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/21] xen/arm: extend device tree based multiboot protocol
Hi Stefano, On 13/07/18 23:41, Stefano Stabellini wrote: On Mon, 9 Jul 2018, Julien Grall wrote: On 07/07/18 00:12, Stefano Stabellini wrote: Extend the existing device tree based multiboot protocol to include information regarding multiple domains to boot. Signed-off-by: Stefano Stabellini --- Changes in v2: - lower case kernel - rename mem to memory - mandate cpus and memory - replace domU-kernel with kernel and domU-ramdisk with ramdisk - rename xen,domU with xen,domain - add info about dom0 - switch memory and cpus to integers - remove defaults - add vpl011 --- docs/misc/arm/device-tree/booting.txt | 108 ++ 1 file changed, 108 insertions(+) diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index ce2d0dc..5c3b8da 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -119,3 +119,111 @@ For those you would hardcode the Xen commandline in the DTB under line by writing bootargs (as for native Linux). A Xen-aware bootloader would set xen,xen-bootargs for Xen, xen,dom0-bootargs for Dom0 and bootargs for native Linux. + + +Creating Multiple Domains directly from Xen +=== + +It is possible to have Xen create other domains, in addition to dom0, +out of the information provided via device tree. A kernel and initrd +(optional) need to be specified for each guest. + +For each domain to be created there needs to be one node under /chosen +with the following properties: + +- compatible + +For domUs: "xen,domain" +For dom0: "xen,domain", "xen,initial-domain" Looking briefly at the code, I don't see any support of "xen,initial-domain". Did I miss anything? But, it is a bit strange to put that in compatible. Shouldn't this be a property? I haven't implemened this in this series yet. Let's add "xen,initial-domain" to the spec together with the implementation in one of the follow-up series. + +- memory + +An integer specifying the amount of megabytes of RAM to allocate to +the guest. I would define this a KB. With Dom0less it would be easy to spawn a guest with less than a MB of memory. What matter is the amount of memory should be page-aligned. I think it would be good to allow users to specify the memory in KB, you are right that we might be able to have <1MB guests. At the same time, it is a pain to have to deal with KBs when allocating multi GBs guests. It is not very difficult, You just use your wcalc (or any calculator) and do a shift 30. Any suggestion on how to make this more user friendly? Maybe we could find a way to support multiple units, for instance we could support memory_mb (for MBs) and memory_kb (for KBs). That's ugly because you know have to describe clearly what they are for or otherwise someone may think it would be fine to describe your 1.5GB guest as: memory_gb = 1 memory_mb = 512 If you want to make user-friendly then provide macros to generate the device-tree. This is already used for describe GIC controller in Linux. Or we could just suck it up and use KBs only. I mean, if we have to support one unit only, it should probably be KBs. I wonder if it makes sense to rename memory to memory_ in any case for clarity. I would prefer to keep "memory" and encourage users to read the associated documentation. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/13] public / x86: introduce __HYPERCALL_iommu_op
On Sat, Jul 07, 2018 at 12:05:19PM +0100, Paul Durrant wrote: > This patch introduces the boilerplate for a new hypercall to allow a > domain to control IOMMU mappings for its own pages. > Whilst there is duplication of code between the native and compat entry > points which appears ripe for some form of combination, I think it is > better to maintain the separation as-is because the compat entry point > will necessarily gain complexity in subsequent patches. > Since this is a new op, why is there compatibility issue? Can we do away with the compat entry point all together? I haven't read the rest of this series, maybe it will become clearer later. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/13] iommu: don't domain_crash() inside iommu_map/unmap_page()
On Sat, Jul 07, 2018 at 12:05:18PM +0100, Paul Durrant wrote: > Turn iommu_map/unmap_page() into straightforward wrappers that check the > existence of the relevant iommu_op and call through to it. This makes them > usable by PV IOMMU code to be delivered in future patches. > Leave the decision on whether to invoke domain_crash() up to the caller. > This has the added benefit that the (module/line number) message that > domain_crash() spits out will be more indicative of where the problem lies. > > NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry() > replacing use of p2m->domain with the domain pointer passed into the > function. > > Signed-off-by: Paul Durrant 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] x86/HVM: improve a few state load checks
> -Original Message-
> From: Andrew Cooper
> Sent: 16 July 2018 15:08
> To: Paul Durrant ; 'Jan Beulich'
> ; xen-devel
> Subject: Re: [PATCH] x86/HVM: improve a few state load checks
>
> On 16/07/18 15:05, Paul Durrant wrote:
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -976,14 +976,13 @@ unsigned long hvm_cr4_guest_valid_bits(c
> >>
> >> static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t
> *h)
> >> {
> >> -int vcpuid;
> >> +unsigned int vcpuid = hvm_load_instance(h);
> >> struct vcpu *v;
> >> struct hvm_hw_cpu ctxt;
> >> struct segment_register seg;
> >> const char *errstr;
> >>
> >> /* Which vcpu is this? */
> >> -vcpuid = hvm_load_instance(h);
> >> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> > This open coded pattern:
> >
> > vcpuid = hvm_load_instance(h);
> > if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> > { ...
> >
> > seems to be repeated an awful lot. Is it time, perhaps, to introduce a
> helper function that incorporates the check?
>
> In some copious free time when I can post v2 of my "fix max_vcpus"
> series, all of this logic (and much more) will become redundant and get
> culled.
>
> I really wouldn't waste time re-factoring it at this point.
Ok. If that's the case then...
Reviewed-by: Paul Durrant
>
> ~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: improve a few state load checks
On 16/07/18 15:05, Paul Durrant wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -976,14 +976,13 @@ unsigned long hvm_cr4_guest_valid_bits(c
>>
>> static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>> {
>> -int vcpuid;
>> +unsigned int vcpuid = hvm_load_instance(h);
>> struct vcpu *v;
>> struct hvm_hw_cpu ctxt;
>> struct segment_register seg;
>> const char *errstr;
>>
>> /* Which vcpu is this? */
>> -vcpuid = hvm_load_instance(h);
>> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> This open coded pattern:
>
> vcpuid = hvm_load_instance(h);
> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> { ...
>
> seems to be repeated an awful lot. Is it time, perhaps, to introduce a helper
> function that incorporates the check?
In some copious free time when I can post v2 of my "fix max_vcpus"
series, all of this logic (and much more) will become redundant and get
culled.
I really wouldn't waste time re-factoring it at this point.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/2] EFI build improvements
Hello, The following patches started as a workaround to build Xen using lld (the LLVM linker), but now patch #2 is an improvement of the build system, thus allowing to create a multiboot2 capable ELF binary requiring just a compiler that supports the MS ABI. Previously in order to build a multiboot2 capable ELF binary linker PE support was also required. Thanks, Roger. Roger Pau Monne (2): x86/efi: move the logic to detect PE build support x86/efi: split compiler vs linker support .gitignore| 1 - xen/arch/x86/Makefile | 12 ++-- xen/arch/x86/efi/Makefile | 11 +++ xen/arch/x86/xen.lds.S| 4 +++- 4 files changed, 16 insertions(+), 12 deletions(-) -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/2] x86/efi: split compiler vs linker support
So that an ELF binary with support for EFI services will be built when the compiler supports the MS ABI, regardless of the linker support for PE. Signed-off-by: Roger Pau Monné --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Daniel Kiper --- Changes since v1: - New in this version. --- xen/arch/x86/Makefile | 11 ++- xen/arch/x86/efi/Makefile | 6 +++--- xen/arch/x86/xen.lds.S| 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 2172a07071..7736d37d4c 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -163,11 +163,12 @@ EFI_LDFLAGS += --minor-image-version=$(XEN_SUBVERSION) EFI_LDFLAGS += --major-os-version=2 --minor-os-version=0 EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0 -# Check if the build system supports PE. -XEN_BUILD_PE := $(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c efi/check.c -o efi/check.o 2>/dev/null && echo y) -export XEN_BUILD_PE := $(if $(XEN_BUILD_PE),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y)) -ifeq ($(XEN_BUILD_PE),y) -CFLAGS += -DXEN_BUILD_PE +# Check if the compiler supports the MS ABI. +export XEN_BUILD_EFI := $(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c efi/check.c -o efi/check.o 2>/dev/null && echo y) +# Check if the linker supports PE. +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y)) +ifeq ($(XEN_BUILD_EFI),y) +CFLAGS += -DXEN_BUILD_EFI endif $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIRT_START$$,,p') diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile index 918383b325..3816de2738 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -6,6 +6,6 @@ CFLAGS += -fshort-wchar boot.init.o: buildid.o obj-y := stub.o -obj-$(XEN_BUILD_PE) := boot.init.o compat.o relocs-dummy.o runtime.o -extra-$(XEN_BUILD_PE) += buildid.o -nocov-$(XEN_BUILD_PE) += stub.o +obj-$(XEN_BUILD_EFI) := boot.init.o compat.o relocs-dummy.o runtime.o +extra-$(XEN_BUILD_EFI) += buildid.o +nocov-$(XEN_BUILD_EFI) += stub.o diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 4a59467986..6e9bda5109 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -304,7 +304,7 @@ SECTIONS } :text #endif -#ifndef XEN_BUILD_PE +#ifndef XEN_BUILD_EFI efi = .; #endif -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/2] x86/efi: move the logic to detect PE build support
So that it can be used by other components apart from the efi specific
code. By moving the detection code creating a dummy efi/disabled file
can be avoided.
This is required so that the conditional used to define the efi symbol
in the linker script can be removed and instead the definition of the
efi symbol can be guarded using the preprocessor.
The motivation behind this change is to be able to build Xen using lld
(the LLVM linker), that at least on version 6.0.0 doesn't work
properly with a DEFINED being used in a conditional expression:
ld-melf_x86_64_fbsd -T xen.lds -N prelink.o --build-id=sha1 \
/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
ld: error: xen.lds:233: symbol not found: efi
Signed-off-by: Roger Pau Monné
---
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Daniel Kiper
---
Changes since v1:
- Rename variable.
- Remove usage of the efi/disabled file.
---
.gitignore| 1 -
xen/arch/x86/Makefile | 11 +--
xen/arch/x86/efi/Makefile | 11 +++
xen/arch/x86/xen.lds.S| 4 +++-
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/.gitignore b/.gitignore
index 5b8448d8f7..aa6856c2b9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -302,7 +302,6 @@ xen/arch/x86/boot/*.bin
xen/arch/x86/boot/*.lnk
xen/arch/x86/efi.lds
xen/arch/x86/efi/check.efi
-xen/arch/x86/efi/disabled
xen/arch/x86/efi/mkreloc
xen/arch/*/efi/boot.c
xen/arch/*/efi/compat.c
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5563c813dd..2172a07071 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -163,10 +163,17 @@ EFI_LDFLAGS += --minor-image-version=$(XEN_SUBVERSION)
EFI_LDFLAGS += --major-os-version=2 --minor-os-version=0
EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
+# Check if the build system supports PE.
+XEN_BUILD_PE := $(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
efi/check.c -o efi/check.o 2>/dev/null && echo y)
+export XEN_BUILD_PE := $(if $(XEN_BUILD_PE),$(shell $(LD) -mi386pep
--subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))
+ifeq ($(XEN_BUILD_PE),y)
+CFLAGS += -DXEN_BUILD_PE
+endif
+
$(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A
VIRT_START$$,,p')
$(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A
ALT_START$$,,p')
# Don't use $(wildcard ...) here - at least make 3.80 expands this too early!
-$(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:)
+$(TARGET).efi: guard = $(if $(XEN_BUILD_PE),,:)
ifneq ($(build_id_linker),)
ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
@@ -232,6 +239,6 @@ efi/mkreloc: efi/mkreloc.c
clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
- rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
+ rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
rm -f boot/cmdline.S boot/reloc.S boot/*.lnk boot/*.bin
rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 3be9661108..918383b325 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,16 +1,11 @@
CFLAGS += -fshort-wchar
-efi := y$(shell rm -f disabled)
-efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
check.c 2>disabled && echo y))
-efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y)
-
%.o: %.ihex
$(OBJCOPY) -I ihex -O binary $< $@
boot.init.o: buildid.o
obj-y := stub.o
-obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
-extra-$(efi) += buildid.o
-nocov-$(efi) += stub.o
+obj-$(XEN_BUILD_PE) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(XEN_BUILD_PE) += buildid.o
+nocov-$(XEN_BUILD_PE) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 326e885402..4a59467986 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -304,7 +304,9 @@ SECTIONS
} :text
#endif
- efi = DEFINED(efi) ? efi : .;
+#ifndef XEN_BUILD_PE
+ efi = .;
+#endif
/* Sections to be discarded */
/DISCARD/ : {
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: improve a few state load checks
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 16 July 2018 14:58
> To: xen-devel
> Cc: Andrew Cooper ; Paul Durrant
>
> Subject: [PATCH] x86/HVM: improve a few state load checks
>
> Using plain int for instance numbers looks quite dangerous without
> being aware that hvm_load_instance() returns an unsigned quantity. Make
> this more explicit. Also replace uint16_t uses by unsigned int.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -976,14 +976,13 @@ unsigned long hvm_cr4_guest_valid_bits(c
>
> static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> {
> -int vcpuid;
> +unsigned int vcpuid = hvm_load_instance(h);
> struct vcpu *v;
> struct hvm_hw_cpu ctxt;
> struct segment_register seg;
> const char *errstr;
>
> /* Which vcpu is this? */
> -vcpuid = hvm_load_instance(h);
> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
This open coded pattern:
vcpuid = hvm_load_instance(h);
if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
{ ...
seems to be repeated an awful lot. Is it time, perhaps, to introduce a helper
function that incorporates the check?
Paul
> {
> dprintk(XENLOG_G_ERR, "HVM restore: dom%u has no vcpu%u\n",
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -768,7 +768,7 @@ static int hvm_save_mtrr_msr(struct doma
>
> static int hvm_load_mtrr_msr(struct domain *d, hvm_domain_context_t
> *h)
> {
> -int vcpuid, i;
> +unsigned int vcpuid, i;
> struct vcpu *v;
> struct mtrr_state *mtrr_state;
> struct hvm_hw_mtrr hw_mtrr;
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -1048,11 +1048,10 @@ static int viridian_save_vcpu_ctxt(struc
>
> static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t
> *h)
> {
> -int vcpuid;
> +unsigned int vcpuid = hvm_load_instance(h);
> struct vcpu *v;
> struct hvm_viridian_vcpu_context ctxt;
>
> -vcpuid = hvm_load_instance(h);
> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> {
> dprintk(XENLOG_G_ERR, "HVM restore: dom%d has no vcpu%u\n",
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -1507,7 +1507,7 @@ static void lapic_load_fixup(struct vlap
>
> static int lapic_load_hidden(struct domain *d, hvm_domain_context_t *h)
> {
> -uint16_t vcpuid;
> +unsigned int vcpuid = hvm_load_instance(h);
> struct vcpu *v;
> struct vlapic *s;
>
> @@ -1515,7 +1515,6 @@ static int lapic_load_hidden(struct doma
> return -ENODEV;
>
> /* Which vlapic to load? */
> -vcpuid = hvm_load_instance(h);
> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> {
> dprintk(XENLOG_G_ERR, "HVM restore: dom%d has no apic%u\n",
> @@ -1542,7 +1541,7 @@ static int lapic_load_hidden(struct doma
>
> static int lapic_load_regs(struct domain *d, hvm_domain_context_t *h)
> {
> -uint16_t vcpuid;
> +unsigned int vcpuid = hvm_load_instance(h);
> struct vcpu *v;
> struct vlapic *s;
>
> @@ -1550,7 +1549,6 @@ static int lapic_load_regs(struct domain
> return -ENODEV;
>
> /* Which vlapic to load? */
> -vcpuid = hvm_load_instance(h);
> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> {
> dprintk(XENLOG_G_ERR, "HVM restore: dom%d has no apic%u\n",
> --- a/xen/arch/x86/hvm/vpic.c
> +++ b/xen/arch/x86/hvm/vpic.c
> @@ -393,13 +393,12 @@ static int vpic_save(struct domain *d, h
> static int vpic_load(struct domain *d, hvm_domain_context_t *h)
> {
> struct hvm_hw_vpic *s;
> -uint16_t inst;
> +unsigned int inst = hvm_load_instance(h);
>
> if ( !has_vpic(d) )
> return -ENODEV;
>
> /* Which PIC is this? */
> -inst = hvm_load_instance(h);
> if ( inst > 1 )
> return -EINVAL;
> s = >arch.hvm_domain.vpic[inst];
> --- a/xen/include/asm-x86/hvm/save.h
> +++ b/xen/include/asm-x86/hvm/save.h
> @@ -84,10 +84,10 @@ void _hvm_read_entry(struct hvm_domain_c
> _hvm_load_entry(_x, _h, _dst, 0)
>
> /* Unmarshalling: what is the instance ID of the next entry? */
> -static inline uint16_t hvm_load_instance(struct hvm_domain_context *h)
> +static inline unsigned int hvm_load_instance(const struct
> hvm_domain_context *h)
> {
> -struct hvm_save_descriptor *d
> -= (struct hvm_save_descriptor *)>data[h->cur];
> +const struct hvm_save_descriptor *d = (const void *)>data[h->cur];
> +
> return d->instance;
> }
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 1/6] Tools.mk.in: drop unused variables
Signed-off-by: Wei Liu --- Cc: Ian Jackson --- config/Tools.mk.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/Tools.mk.in b/config/Tools.mk.in index 2d6c440324..4cc9f29090 100644 --- a/config/Tools.mk.in +++ b/config/Tools.mk.in @@ -20,8 +20,6 @@ BCC := @BCC@ IASL:= @IASL@ AWK := @AWK@ FETCHER := @FETCHER@ -SEABIOS_PATH:= @seabios_path@ -OVMF_PATH := @ovmf_path@ # Extra folder for libs/includes PREPEND_INCLUDES:= @PREPEND_INCLUDES@ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 5/6] tools: provide --with-system-ipxe
This option lets user specify which binary is to be used as ipxe. If
it is specified, the in-tree ipxe will not be built. This option is in
line with other --with-system-* options we provide.
Signed-off-by: Wei Liu
---
v3: ipxe should require rombios
Cc: Ian Jackson
---
config/Tools.mk.in| 1 +
tools/config.h.in | 3 +++
tools/configure | 58 +++
tools/configure.ac| 23 +++
tools/libxl/libxl_paths.c | 6 -
5 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 4cc9f29090..0964f6f9e9 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -50,6 +50,7 @@ FLASK_POLICY:= @xsmpolicy@
CONFIG_OVMF := @ovmf@
CONFIG_ROMBIOS := @rombios@
CONFIG_SEABIOS := @seabios@
+CONFIG_IPXE := @ipxe@
CONFIG_QEMU_TRAD:= @qemu_traditional@
CONFIG_QEMU_XEN := @qemu_xen@
CONFIG_BLKTAP2 := @blktap2@
diff --git a/tools/config.h.in b/tools/config.h.in
index c66a78c9b3..5987f087b8 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -96,6 +96,9 @@
/* libutil header file name */
#undef INCLUDE_LIBUTIL_H
+/* IPXE path */
+#undef IPXE_PATH
+
/* OVMF path */
#undef OVMF_PATH
diff --git a/tools/configure b/tools/configure
index 4863f28306..4bff2c02fd 100755
--- a/tools/configure
+++ b/tools/configure
@@ -703,6 +703,7 @@ AS86
qemu_traditional
blktap2
LINUX_BACKEND_MODULES
+ipxe
seabios
ovmf
xsmpolicy
@@ -805,6 +806,7 @@ enable_ocamltools
enable_xsmpolicy
enable_ovmf
enable_seabios
+enable_ipxe
with_linux_backend_modules
enable_blktap2
enable_qemu_traditional
@@ -812,6 +814,7 @@ enable_rombios
with_system_qemu
with_system_seabios
with_system_ovmf
+with_system_ipxe
with_extra_qemuu_configure_args
with_xenstored
enable_systemd
@@ -1488,6 +1491,7 @@ Optional Features:
--disable-xsmpolicy Disable XSM policy compilation (default is ENABLED)
--enable-ovmf Enable OVMF (default is DISABLED)
--disable-seabios Disable SeaBIOS (default is ENABLED)
+ --disable-ipxe Disable IPXE (default is ENABLED)
--enable-blktap2Enable blktap2, (DEFAULT is off)
--enable-qemu-traditional
Enable qemu traditional device model, (DEFAULT is on
@@ -1527,6 +1531,9 @@ Optional Packages:
--with-system-ovmf[=PATH]
Use system supplied OVMF PATH instead of building
and installing our own version
+ --with-system-ipxe[=PATH]
+ Use system supplied IPXE PATH instead of building
+ and installing our own version
--with-extra-qemuu-configure-args[="--ARG1 ..."]
List of additional configure options for upstream
qemu
@@ -4184,6 +4191,29 @@ seabios=$ax_cv_seabios
+# Check whether --enable-ipxe was given.
+if test "${enable_ipxe+set}" = set; then :
+ enableval=$enable_ipxe;
+fi
+
+
+if test "x$enable_ipxe" = "xno"; then :
+
+ax_cv_ipxe="n"
+
+elif test "x$enable_ipxe" = "xyes"; then :
+
+ax_cv_ipxe="y"
+
+elif test -z $ax_cv_ipxe; then :
+
+ax_cv_ipxe="y"
+
+fi
+ipxe=$ax_cv_ipxe
+
+
+
# Check whether --with-linux-backend-modules was given.
if test "${with_linux_backend_modules+set}" = set; then :
@@ -4573,6 +4603,34 @@ _ACEOF
fi
+# Check whether --with-system-ipxe was given.
+if test "${with_system_ipxe+set}" = set; then :
+ withval=$with_system_ipxe;
+# Disable compilation of IPXE.
+ipxe=n
+case $withval in
+no) ipxe_path= ;;
+*) ipxe_path=$withval ;;
+esac
+
+# IPXE depends on Rombios
+if test "x$enable_rombios" = "xno"; then
+as_fn_error $? "Rombios is required for using IPXE" "$LINENO" 5
+fi
+
+fi
+
+if test "x$ipxe" = "xy" -o -n "$ipxe_path" ; then :
+
+
+cat >>confdefs.h <<_ACEOF
+#define IPXE_PATH "${ipxe_path:-$XENFIRMWAREDIR/ipxe.bin}"
+_ACEOF
+
+
+fi
+
+
# Check whether --with-extra-qemuu-configure-args was given.
if test "${with_extra_qemuu_configure_args+set}" = set; then :
withval=$with_extra_qemuu_configure_args;
diff --git a/tools/configure.ac b/tools/configure.ac
index 0826af8cbc..2db2356380 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -84,6 +84,7 @@ AX_ARG_DEFAULT_ENABLE([ocamltools], [Disable Ocaml tools])
AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF])
AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS])
+AX_ARG_DEFAULT_ENABLE([ipxe], [Disable IPXE])
AC_ARG_WITH([linux-backend-modules],
AS_HELP_STRING([--with-linux-backend-modules="mod1 mod2"],
@@ -241,6 +242,28 @@ AS_IF([test "x$ovmf" = "xy" -o -n "$ovmf_path" ], [
[OVMF path])
])
+AC_ARG_WITH([system-ipxe],
+AS_HELP_STRING([--with-system-ipxe@<:@=PATH@:>@],
+ [Use system supplied IPXE PATH instead of building and installing
+
[Xen-devel] [PATCH v4 3/6] libxc: allow HVM guest to have modules
Lift the loading code out of PVH specific branch. Take the chance to
make the debug message more useful.
Now the code needs to handle virt_base being UNSET_ADDR, which it is
for HVM guest. In case virt_base is not set, it should be treated as
zero. In case PVH and PV, virt_base is set by the respective loader
by parsing the binary.
IPXE will be loaded as a module of Rombios.
Signed-off-by: Wei Liu
Reviewed-by: Roger Pau Monné
---
Cc: Ian Jackson
---
tools/libxc/xc_dom_x86.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index d28ff4d7e9..d77f2d6f62 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1742,20 +1742,6 @@ static int bootlate_hvm(struct xc_dom_image *dom)
((uintptr_t)cmdline - (uintptr_t)start_info);
}
-for ( i = 0; i < dom->num_modules; i++ )
-{
-struct xc_hvm_firmware_module mod;
-
-DOMPRINTF("Adding module %u", i);
-mod.guest_addr_out =
-dom->modules[i].seg.vstart - dom->parms.virt_base;
-mod.length =
-dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
-
-add_module_to_list(dom, , dom->modules[i].cmdline,
- modlist, start_info);
-}
-
/* ACPI module 0 is the RSDP */
start_info->rsdp_paddr = dom->acpi_modules[0].guest_addr_out ? : 0;
}
@@ -1765,6 +1751,24 @@ static int bootlate_hvm(struct xc_dom_image *dom)
modlist, start_info);
}
+for ( i = 0; i < dom->num_modules; i++ )
+{
+struct xc_hvm_firmware_module mod;
+uint64_t base = dom->parms.virt_base != UNSET_ADDR ?
+dom->parms.virt_base : 0;
+
+mod.guest_addr_out =
+dom->modules[i].seg.vstart - base;
+mod.length =
+dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
+
+DOMPRINTF("Adding module %u guest_addr %"PRIx64" len %u",
+ i, mod.guest_addr_out, mod.length);
+
+add_module_to_list(dom, , dom->modules[i].cmdline,
+ modlist, start_info);
+}
+
if ( start_info->nr_modules )
{
start_info->modlist_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 2/6] ipxe: produce a single binary from its build
And switch hvmloader/Makefile to use that binary. This will help later when we change hvmloader to pick a user provided binary. No functional change. Signed-off-by: Wei Liu Acked-by: Jan Beulich --- v2: use intermediary file Cc: Ian Jackson --- tools/firmware/etherboot/Makefile | 7 ++- tools/firmware/hvmloader/Makefile | 8 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/firmware/etherboot/Makefile b/tools/firmware/etherboot/Makefile index e33458d2fe..4de6d24a13 100644 --- a/tools/firmware/etherboot/Makefile +++ b/tools/firmware/etherboot/Makefile @@ -18,11 +18,16 @@ D=ipxe T=ipxe.tar.gz ROMS = $(addprefix $D/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS))) +ROM = $D/src/bin/ipxe.bin .NOTPARALLEL: .PHONY: all -all: $(ROMS) +all: $(ROM) + +$(ROM): $(ROMS) + cat $^ > $@.tmp + mv -f $@.tmp $@ %.rom: $D/src/arch/i386/Makefile $(MAKE) -C $D/src bin/$(*F).rom diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile index a5b4c32c1a..16255ebddd 100644 --- a/tools/firmware/hvmloader/Makefile +++ b/tools/firmware/hvmloader/Makefile @@ -51,7 +51,7 @@ CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin else CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin endif -ETHERBOOT_ROMS := $(addprefix ../etherboot/ipxe/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS))) +ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin endif ROMS := @@ -60,7 +60,7 @@ ifeq ($(CONFIG_ROMBIOS),y) OBJS += optionroms.o 32bitbios_support.o rombios.o CFLAGS += -DENABLE_ROMBIOS ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest -ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS) +ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROM) endif .PHONY: all @@ -105,9 +105,9 @@ ifneq ($(CIRRUSVGA_ROM),) sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new echo "#endif" >> $@.new endif -ifneq ($(ETHERBOOT_ROMS),) +ifneq ($(ETHERBOOT_ROM),) echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new - sh ../../misc/mkhex etherboot $(ETHERBOOT_ROMS) >> $@.new + sh ../../misc/mkhex etherboot $(ETHERBOOT_ROM) >> $@.new echo "#endif" >> $@.new endif -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 4/6] tools: load IPXE from standalone file
Do not embed IPXE into Rombios anymore. Instead, it is loaded by the
toolstack from a file as a separate module.
Ability to let user specify an IPXE blob will come later.
No user visible change.
Signed-off-by: Wei Liu
Acked-by: Jan Beulich
---
v3: adjust libxl code a bit, addressed Jan's comment and added his
ack.
Cc: Andrew Cooper
Cc: anthony.per...@citrix.com
---
tools/firmware/Makefile | 6 ++
tools/firmware/hvmloader/Makefile| 9 +
tools/firmware/hvmloader/config.h| 3 ++-
tools/firmware/hvmloader/hvmloader.c | 10 --
tools/firmware/hvmloader/ovmf.c | 3 ++-
tools/firmware/hvmloader/rombios.c | 24 +---
tools/firmware/hvmloader/seabios.c | 3 ++-
tools/libxl/libxl_dom.c | 13 +
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_paths.c| 5 +
10 files changed, 57 insertions(+), 20 deletions(-)
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 842b48c3d3..bc84300b69 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -55,6 +55,9 @@ endif
ifeq ($(CONFIG_OVMF),y)
$(INSTALL_DATA) ovmf-dir/ovmf.bin $(INST_DIR)/ovmf.bin
endif
+ifeq ($(CONFIG_IPXE),y)
+ $(INSTALL_DATA) etherboot/ipxe/src/bin/ipxe.bin $(INST_DIR)/ipxe.bin
+endif
ifeq ($(CONFIG_PV_SHIM),y)
$(INSTALL_DATA) xen-dir/xen-shim $(INST_DIR)/xen-shim
$(INSTALL_DATA) xen-dir/xen-shim-syms $(DEBG_DIR)/xen-shim-syms
@@ -69,6 +72,9 @@ endif
ifeq ($(CONFIG_OVMF),y)
rm -f $(INST_DIR)/ovmf.bin
endif
+ifeq ($(CONFIG_IPXE),y)
+ rm -r $(INST_DIR)/ipxe.bin
+endif
ifeq ($(CONFIG_PV_SHIM),y)
rm -f $(INST_DIR)/xen-shim
rm -f $(DEBG_DIR)/xen-shim-syms
diff --git a/tools/firmware/hvmloader/Makefile
b/tools/firmware/hvmloader/Makefile
index 16255ebddd..496ac72b77 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -51,7 +51,6 @@ CIRRUSVGA_ROM :=
../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
else
CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
endif
-ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin
endif
ROMS :=
@@ -60,7 +59,7 @@ ifeq ($(CONFIG_ROMBIOS),y)
OBJS += optionroms.o 32bitbios_support.o rombios.o
CFLAGS += -DENABLE_ROMBIOS
ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
-ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROM)
+ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM)
endif
.PHONY: all
@@ -105,12 +104,6 @@ ifneq ($(CIRRUSVGA_ROM),)
sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
echo "#endif" >> $@.new
endif
-ifneq ($(ETHERBOOT_ROM),)
- echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
- sh ../../misc/mkhex etherboot $(ETHERBOOT_ROM) >> $@.new
- echo "#endif" >> $@.new
-endif
-
mv $@.new $@
.PHONY: clean
diff --git a/tools/firmware/hvmloader/config.h
b/tools/firmware/hvmloader/config.h
index 6e00413f2e..d9b4713d63 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -22,7 +22,8 @@ struct bios_config {
/* ROMS */
void (*load_roms)(void);
-void (*bios_load)(const struct bios_config *config, void *addr, uint32_t
size);
+void (*bios_load)(const struct bios_config *config, void *addr,
+ uint32_t size, void *extra_addr);
void (*bios_info_setup)(void);
void (*bios_info_finish)(void);
diff --git a/tools/firmware/hvmloader/hvmloader.c
b/tools/firmware/hvmloader/hvmloader.c
index f603f68ded..598a226278 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -363,12 +363,18 @@ int main(void)
{
uint32_t paddr = bios_module->paddr;
-bios->bios_load(bios, (void*)paddr, bios_module->size);
+bios->bios_load(bios, (void *)paddr, bios_module->size, NULL);
}
#ifdef ENABLE_ROMBIOS
else if ( bios == _config )
{
-bios->bios_load(bios, NULL, 0);
+const struct hvm_modlist_entry *ipxe;
+uint32_t paddr = 0;
+
+ipxe = get_module_entry(hvm_start_info, "ipxe");
+if ( ipxe )
+paddr = ipxe->paddr;
+bios->bios_load(bios, NULL, 0, (void *)paddr);
}
#endif
else
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index a17a11c2f9..23610a0717 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -85,7 +85,8 @@ static void ovmf_finish_bios_info(void)
}
static void ovmf_load(const struct bios_config *config,
- void *bios_addr, uint32_t bios_length)
+ void *bios_addr, uint32_t bios_length,
+ void *unused_addr)
{
xen_pfn_t mfn;
uint64_t addr = OVMF_END
diff --git a/tools/firmware/hvmloader/rombios.c
b/tools/firmware/hvmloader/rombios.c
index c736fd9dea..46f331e465 100644
--- a/tools/firmware/hvmloader/rombios.c
+++
[Xen-devel] [PATCH v4 0/6] Load ipxe from a standalone file
Update commit messages. No code change in this version.
Wei Liu (6):
Tools.mk.in: drop unused variables
ipxe: produce a single binary from its build
libxc: allow HVM guest to have modules
tools: load IPXE from standalone file
tools: provide --with-system-ipxe
tools: --with-system-{ovmf,seabios,ipxe} should provide absolute paths
config/Tools.mk.in | 3 +-
tools/config.h.in| 3 ++
tools/configure | 65 ++--
tools/configure.ac | 30 +++--
tools/firmware/Makefile | 6
tools/firmware/etherboot/Makefile| 7 +++-
tools/firmware/hvmloader/Makefile| 9 +
tools/firmware/hvmloader/config.h| 3 +-
tools/firmware/hvmloader/hvmloader.c | 10 --
tools/firmware/hvmloader/ovmf.c | 3 +-
tools/firmware/hvmloader/rombios.c | 24 +
tools/firmware/hvmloader/seabios.c | 3 +-
tools/libxc/xc_dom_x86.c | 32 ++
tools/libxl/libxl_dom.c | 13
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_paths.c| 9 +
16 files changed, 180 insertions(+), 41 deletions(-)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: improve a few state load checks
On 16/07/18 14:58, Jan Beulich wrote: > Using plain int for instance numbers looks quite dangerous without > being aware that hvm_load_instance() returns an unsigned quantity. Make > this more explicit. Also replace uint16_t uses by unsigned int. > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 125226: all pass - PUSHED
flight 125226 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/125226/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd 04c77c18ada08a796cfe1d7545c0e76429e1d3dc baseline version: freebsd 414774b7931d98c2bc5f42317ea69c8a32eebc6e Last test of basis 125150 2018-07-13 09:18:49 Z3 days Testing same since 125226 2018-07-16 09:19:15 Z0 days1 attempts People who touched revisions under test: alc asomers brooks cem gonzo ian imp jilles kp marius markj mjg mmacy mw oshogbo pstef rmacklem sbruno stevek tsoome tuexen jobs: build-amd64-freebsd-againpass build-amd64-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 414774b7931..04c77c18ada 04c77c18ada08a796cfe1d7545c0e76429e1d3dc -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/11] x86/svm Fixes and cleanup to svm_inject_event()
On 06/06/18 14:37, Jan Beulich wrote: > >>> On 04.06.18 at 15:59, wrote: >> * State adjustments (and debug tracing) for #DB/#BP/#PF should not be done >>for `int $n` instructions. Updates to %cr2 occur even if the exception >>combines to #DF. >> * Don't opencode DR_STEP when updating %dr6. >> * Simplify the logic for calling svm_emul_swint_injection() as in the common >>case, every condition needs checking. >> * Fix comments which have become stale as code has moved between components. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Jan Beulich > > Ping SVM? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-upstream-4.9-testing baseline-only test] 74974: regressions - FAIL
This run is configured for baseline tests only. flight 74974 qemu-upstream-4.9-testing real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/74974/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 72513 test-armhf-armhf-xl-multivcpu 12 guest-start fail REGR. vs. 72513 test-armhf-armhf-xl-midway 12 guest-start fail REGR. vs. 72513 test-armhf-armhf-xl-credit2 12 guest-start fail REGR. vs. 72513 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail REGR. vs. 72513 test-amd64-amd64-qemuu-nested-intel 14 xen-boot/l1fail REGR. vs. 72513 test-armhf-armhf-xl-vhd 10 debian-di-install fail REGR. vs. 72513 test-armhf-armhf-libvirt-raw 10 debian-di-install fail REGR. vs. 72513 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 12 guest-start fail REGR. vs. 72513 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 12 guest-start fail blocked in 72513 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail like 72513 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 72513 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-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-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-qemuu-win7-amd64 18 guest-start/win.repeat fail never pass test-amd64-amd64-xl-qemuu-win10-i386 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail never pass version targeted for testing: qemuuaad23066e4b27296d219b9123393fbe2a5a885bb baseline version: qemuub397ed6a586b0a93e9a8b47f5b3008fac34f5f37 Last test of basis72513 2017-12-04 03:47:26 Z 224 days Testing same since74974 2018-07-16 05:04:03 Z0 days1 attempts People who touched revisions under test: Dr. David Alan Gilbert Eric Blake Gerd Hoffmann John Thomson Max Reitz Paolo Bonzini Samuel Thibault jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-armhf-armhf-xl fail test-amd64-i386-xl pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm fail test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-xl-xsm pass test-amd64-i386-xl-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64
Re: [Xen-devel] [PATCH v2 11/13] x86: Introduce struct cpu_policy to refer to a group of individual policies
>>> On 16.07.18 at 15:15, wrote:
> On 16/07/18 13:29, Jan Beulich wrote:
> On 16.07.18 at 14:16, wrote:
>>> On 16/07/18 13:04, Jan Beulich wrote:
>>> On 13.07.18 at 22:03, wrote:
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -31,6 +31,33 @@
> #include
> #include
>
> +const struct cpu_policy system_policies[] = {
By the end of the series the array remains unused outside this
source file. I'd appreciate if it was made extern only when actually
needed, not the least because ...
> +[ XEN_SYSCTL_cpu_policy_raw ] = {
> +_cpuid_policy,
> +_msr_policy,
> +},
> +[ XEN_SYSCTL_cpu_policy_host ] = {
> +_cpuid_policy,
> +_msr_policy,
> +},
> +[ XEN_SYSCTL_cpu_policy_pv_max ] = {
> +_max_cpuid_policy,
> +_max_msr_policy,
> +},
> +[ XEN_SYSCTL_cpu_policy_hvm_max ] = {
> +_max_cpuid_policy,
> +_max_msr_policy,
> +},
> +[ XEN_SYSCTL_cpu_policy_pv_default ] = {
> +_max_cpuid_policy,
> +_max_msr_policy,
> +},
> +[ XEN_SYSCTL_cpu_policy_hvm_default ] = {
> +_max_cpuid_policy,
> +_max_msr_policy,
> +},
> +};
... this does not make obvious (without consulting sysctl.h) that
there are now holes (and hence hidden NULL pointers); this is
perhaps already undesirable with the user of this array that the
next patch adds.
>>> What holes? There shouldn't be any, and gdb confirms my expectations:
>>>
>>> (gdb) p/x system_policies
>>> $1 = {{cpuid = 0x82d080474a80, msr = 0x82d080475960}, {cpuid =
>>> 0x82d080474340, msr = 0x82d08047595c}, {cpuid =
>>> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
>>> 0x82d0804734c0, msr = 0x82d080475958}, {cpuid =
>>> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
>>> 0x82d0804734c0, msr = 0x82d080475958}}
>> I didn't say there are holes, I've said "does not make obvious".
>
> You did, but I guess what you meant to write was "... are no holes"
> rather "... are now holes".
Oops.
>> For example, it is not unreasonable to imagine for the
>> XEN_SYSCTL_cpu_policy_* values to start at 1 rather than zero, in
>> which case there would be two hidden NULLs at the start of the
>> array.
>
> Why does this matter? We have similar patterns elsewhere, and the array
> cannot reasonably be used without the symbolic names (as it is really an
> unordered set happening to be layed out in array form).
It was you even more than me who was worried about NULL pointers
sitting in random places, waiting to be de-referenced. Besides the
obvious reference by symbolic, there are clearly other ways to index
into this array, not to mention someone setting up a loop over it.
Anyway - I can live with it being the way it is, and I've given the ack.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 11/13] x86: Introduce struct cpu_policy to refer to a group of individual policies
On 16/07/18 13:29, Jan Beulich wrote:
On 16.07.18 at 14:16, wrote:
>> On 16/07/18 13:04, Jan Beulich wrote:
>> On 13.07.18 at 22:03, wrote:
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -31,6 +31,33 @@
#include
#include
+const struct cpu_policy system_policies[] = {
>>> By the end of the series the array remains unused outside this
>>> source file. I'd appreciate if it was made extern only when actually
>>> needed, not the least because ...
>>>
+[ XEN_SYSCTL_cpu_policy_raw ] = {
+_cpuid_policy,
+_msr_policy,
+},
+[ XEN_SYSCTL_cpu_policy_host ] = {
+_cpuid_policy,
+_msr_policy,
+},
+[ XEN_SYSCTL_cpu_policy_pv_max ] = {
+_max_cpuid_policy,
+_max_msr_policy,
+},
+[ XEN_SYSCTL_cpu_policy_hvm_max ] = {
+_max_cpuid_policy,
+_max_msr_policy,
+},
+[ XEN_SYSCTL_cpu_policy_pv_default ] = {
+_max_cpuid_policy,
+_max_msr_policy,
+},
+[ XEN_SYSCTL_cpu_policy_hvm_default ] = {
+_max_cpuid_policy,
+_max_msr_policy,
+},
+};
>>> ... this does not make obvious (without consulting sysctl.h) that
>>> there are now holes (and hence hidden NULL pointers); this is
>>> perhaps already undesirable with the user of this array that the
>>> next patch adds.
>> What holes? There shouldn't be any, and gdb confirms my expectations:
>>
>> (gdb) p/x system_policies
>> $1 = {{cpuid = 0x82d080474a80, msr = 0x82d080475960}, {cpuid =
>> 0x82d080474340, msr = 0x82d08047595c}, {cpuid =
>> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
>> 0x82d0804734c0, msr = 0x82d080475958}, {cpuid =
>> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
>> 0x82d0804734c0, msr = 0x82d080475958}}
> I didn't say there are holes, I've said "does not make obvious".
You did, but I guess what you meant to write was "... are no holes"
rather "... are now holes".
> For example, it is not unreasonable to imagine for the
> XEN_SYSCTL_cpu_policy_* values to start at 1 rather than zero, in
> which case there would be two hidden NULLs at the start of the
> array.
Why does this matter? We have similar patterns elsewhere, and the array
cannot reasonably be used without the symbolic names (as it is really an
unordered set happening to be layed out in array form).
>
>>> With "static" added and the "extern" dropped from the header
>>> Acked-by: Jan Beulich
>> I'm not going to waste even more time by committing something which is
>> wrong, and having to undo it again in a later patch.
>>
>> The user, DOMCTL_set_cpu_policy, is deferred from this series because it
>> is still under development, but there is absolutely no question that
>> this array needs to be externally accessible.
> Well, maybe I should have phrased this differently: I'm unconvinced
> sysctl.c is the right place for this to live. Granted neither cpuid.c nor
> msr.c are any better.
If you can suggest a better place then I'm all ears, but it has to live
somewhere and here was the least-bad option I could come up with.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/8] x86: (command line option to) avoid use of secondary hyper-threads
On 16/07/18 13:53, Jan Beulich wrote:
On 16.07.18 at 14:37, wrote:
>> On 13/07/18 09:13, Jan Beulich wrote:
>> On 12.07.18 at 17:45, wrote:
On 11/07/18 13:10, Jan Beulich wrote:
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1040,6 +1040,13 @@ identical to the boot CPU will be parked
> ### hpetbroadcast (x86)
> > `= `
>
> +### ht (x86)
I'd suggest smt rather than ht here. SMT is the technical term, while
HT is Intel's marketing name.
>>> Hmm, many BIOSes (if the have such an option) talk about HT, which
>>> to me makes "ht" a closer match. How about we allow both?
>> That's because a BIOS is custom to the hardware it runs on.
>>
>> Have you tried setting up an alias before? (given the specific
>> deficiency of the *_param() infrastructure in this area) I'm don't
>> think an alias is worth the effort.
> This reads as if you were expecting problems. Instead of
>
> +int8_t __read_mostly opt_ht = -1;
> +boolean_param("ht", opt_ht);
>
> what we'd have is simply
>
> +int8_t __read_mostly opt_ht = -1;
> +boolean_param("ht", opt_ht);
> +boolean_param("smt", opt_ht);
>
> (and whether we use opt_ht or opt_smt doesn't matter much
> to me). I don't see any source of possible issues with this.
Try compiling it.
I tried exactly this with bti= and spec-ctrl= originally. The problem
is that the second argument must be (translation unit) unique, because
of the way it is used to form the name of the struct kernel_param.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/8] x86: (command line option to) avoid use of secondary hyper-threads
>>> On 16.07.18 at 14:37, wrote:
> On 13/07/18 09:13, Jan Beulich wrote:
> On 12.07.18 at 17:45, wrote:
>>> On 11/07/18 13:10, Jan Beulich wrote:
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1040,6 +1040,13 @@ identical to the boot CPU will be parked
### hpetbroadcast (x86)
> `= `
+### ht (x86)
>>> I'd suggest smt rather than ht here. SMT is the technical term, while
>>> HT is Intel's marketing name.
>> Hmm, many BIOSes (if the have such an option) talk about HT, which
>> to me makes "ht" a closer match. How about we allow both?
>
> That's because a BIOS is custom to the hardware it runs on.
>
> Have you tried setting up an alias before? (given the specific
> deficiency of the *_param() infrastructure in this area) I'm don't
> think an alias is worth the effort.
This reads as if you were expecting problems. Instead of
+int8_t __read_mostly opt_ht = -1;
+boolean_param("ht", opt_ht);
what we'd have is simply
+int8_t __read_mostly opt_ht = -1;
+boolean_param("ht", opt_ht);
+boolean_param("smt", opt_ht);
(and whether we use opt_ht or opt_smt doesn't matter much
to me). I don't see any source of possible issues with this.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 11/12] xen: specify support for EXPERT and DEBUG Kconfig options
>>> On 07.07.18 at 01:14, wrote: > Add a clear statement about them, reflecting the current security > support status of Kconfig options (no changes to current policies). > > Signed-off-by: Stefano Stabellini 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 1/8] cpupools: fix state when downing a CPU failed
On 16/07/18 14:19, Jan Beulich wrote:
On 16.07.18 at 13:47, wrote:
>> On 16/07/18 11:17, Jan Beulich wrote:
>> On 13.07.18 at 11:02, wrote:
On 11/07/18 14:04, Jan Beulich wrote:
> While I've run into the issue with further patches in place which no
> longer guarantee the per-CPU area to start out as all zeros, the
> CPU_DOWN_FAILED processing looks to have the same issue: By not zapping
> the per-CPU cpupool pointer, cpupool_cpu_add()'s (indirect) invocation
> of schedule_cpu_switch() will trigger the "c != old_pool" assertion
> there.
>
> Clearing the field during CPU_DOWN_PREPARE is too early (afaict this
> should not happen before cpu_disable_scheduler()). Clearing it in
> CPU_DEAD and CPU_DOWN_FAILED would be an option, but would take the same
> piece of code twice. Since the field's value shouldn't matter while the
> CPU is offline, simply clear it in CPU_ONLINE and CPU_DOWN_FAILED, but
> only for other than the suspend/resume case (which gets specially
> handled in cpupool_cpu_remove()).
>
> Signed-off-by: Jan Beulich
> ---
> TBD: I think this would better call schedule_cpu_switch(cpu, NULL) from
> cpupool_cpu_remove(), but besides that - as per above - likely
> being too early, that function has further prereqs to be met. It
> also doesn't look as if cpupool_unassign_cpu_helper() could be used
> there.
>
> --- a/xen/common/cpupool.c
> +++ b/xen/common/cpupool.c
> @@ -778,6 +778,8 @@ static int cpu_callback(
> {
> case CPU_DOWN_FAILED:
> case CPU_ONLINE:
> +if ( system_state <= SYS_STATE_active )
> +per_cpu(cpupool, cpu) = NULL;
> rc = cpupool_cpu_add(cpu);
Wouldn't it make more sense to clear the field in cpupool_cpu_add()
which already is testing system_state?
>>>
>>> Hmm, this may be a matter of taste: I consider the change done here
>>> a prereq to calling the function in the first place. As said in the
>>> description, I actually think this should come earlier, and it's just that
>>> I can't see how to cleanly do so.
>
> You didn't comment on this one at all, yet it matters for how a v2
> is supposed to look like.
My comment was thought to address this question, too. cpupool_cpu_add()
is handling the special case of resuming explicitly, where the old cpu
assignment to a cpupool is kept. So I believe setting
per_cpu(cpupool, cpu) = NULL
in the else clause of cpupool_cpu_add() only is better.
Modifying the condition in cpupool_cpu_add() to
if ( system_state <= SYS_STATE_active )
at the same time would have the benefit to catch problems in case
suspending cpus is failing during SYS_STATE_suspend (I'd expect
triggering the first ASSERT in schedule_cpu_switch() in this case).
>>>
>>> You mean the if() there, not the else? If so - how would the "else"
>>> body then ever be reached? IOW if anything I could only see the
>>> "else" to become "else if ( system_state <= SYS_STATE_active )".
>>
>> Bad wording on my side.
>>
>> I should have written "the condition in cpupool_cpu_add() should match
>> if ( system_state <= SYS_STATE_active )."
>>
>> So: "if ( system_state > SYS_STATE_active )", as the test is for the
>> other case.
>
> I'd recommend against this, as someone adding a new SYS_STATE_*
> past suspend/resume would quite likely miss this one. The strong
> ordering of states imo should only be used for active and lower states.
> But yes, I could see the if() there to become suspend || resume to
> address the problem you describe.
Yes, this would seem to be a better choice here.
> Coming back to your DOWN_FAILED consideration: Why are you
> thinking this can't happen during suspend? disable_nonboot_cpus()
> uses plain cpu_down() after all.
Right.
DOWN_FAILED is used only once, and that is in cpu_down() after the step
CPU_DOWN_PREPARE returned an error. And CPU_DOWN_PREPARE is only used
for cpufreq driver where it never returns an error, and for cpupools
which don't matter here, as only other components failing at step
CPU_DOWN_PREPARE would lead to calling cpupool/DOWN_FAILED.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/8] x86: (command line option to) avoid use of secondary hyper-threads
On 13/07/18 09:13, Jan Beulich wrote: On 12.07.18 at 17:45, wrote: >> On 11/07/18 13:10, Jan Beulich wrote: >>> --- a/docs/misc/xen-command-line.markdown >>> +++ b/docs/misc/xen-command-line.markdown >>> @@ -1040,6 +1040,13 @@ identical to the boot CPU will be parked >>> ### hpetbroadcast (x86) >>> > `= ` >>> >>> +### ht (x86) >> I'd suggest smt rather than ht here. SMT is the technical term, while >> HT is Intel's marketing name. > Hmm, many BIOSes (if the have such an option) talk about HT, which > to me makes "ht" a closer match. How about we allow both? That's because a BIOS is custom to the hardware it runs on. Have you tried setting up an alias before? (given the specific deficiency of the *_param() infrastructure in this area) I'm don't think an alias is worth the effort. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 11/13] x86: Introduce struct cpu_policy to refer to a group of individual policies
>>> On 16.07.18 at 14:16, wrote:
> On 16/07/18 13:04, Jan Beulich wrote:
> On 13.07.18 at 22:03, wrote:
>>> --- a/xen/arch/x86/sysctl.c
>>> +++ b/xen/arch/x86/sysctl.c
>>> @@ -31,6 +31,33 @@
>>> #include
>>> #include
>>>
>>> +const struct cpu_policy system_policies[] = {
>> By the end of the series the array remains unused outside this
>> source file. I'd appreciate if it was made extern only when actually
>> needed, not the least because ...
>>
>>> +[ XEN_SYSCTL_cpu_policy_raw ] = {
>>> +_cpuid_policy,
>>> +_msr_policy,
>>> +},
>>> +[ XEN_SYSCTL_cpu_policy_host ] = {
>>> +_cpuid_policy,
>>> +_msr_policy,
>>> +},
>>> +[ XEN_SYSCTL_cpu_policy_pv_max ] = {
>>> +_max_cpuid_policy,
>>> +_max_msr_policy,
>>> +},
>>> +[ XEN_SYSCTL_cpu_policy_hvm_max ] = {
>>> +_max_cpuid_policy,
>>> +_max_msr_policy,
>>> +},
>>> +[ XEN_SYSCTL_cpu_policy_pv_default ] = {
>>> +_max_cpuid_policy,
>>> +_max_msr_policy,
>>> +},
>>> +[ XEN_SYSCTL_cpu_policy_hvm_default ] = {
>>> +_max_cpuid_policy,
>>> +_max_msr_policy,
>>> +},
>>> +};
>> ... this does not make obvious (without consulting sysctl.h) that
>> there are now holes (and hence hidden NULL pointers); this is
>> perhaps already undesirable with the user of this array that the
>> next patch adds.
>
> What holes? There shouldn't be any, and gdb confirms my expectations:
>
> (gdb) p/x system_policies
> $1 = {{cpuid = 0x82d080474a80, msr = 0x82d080475960}, {cpuid =
> 0x82d080474340, msr = 0x82d08047595c}, {cpuid =
> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
> 0x82d0804734c0, msr = 0x82d080475958}, {cpuid =
> 0x82d080473c00, msr = 0x82d080475954}, {cpuid =
> 0x82d0804734c0, msr = 0x82d080475958}}
I didn't say there are holes, I've said "does not make obvious". For
example, it is not unreasonable to imagine for the
XEN_SYSCTL_cpu_policy_* values to start at 1 rather than zero, in
which case there would be two hidden NULLs at the start of the
array.
>> With "static" added and the "extern" dropped from the header
>> Acked-by: Jan Beulich
>
> I'm not going to waste even more time by committing something which is
> wrong, and having to undo it again in a later patch.
>
> The user, DOMCTL_set_cpu_policy, is deferred from this series because it
> is still under development, but there is absolutely no question that
> this array needs to be externally accessible.
Well, maybe I should have phrased this differently: I'm unconvinced
sysctl.c is the right place for this to live. Granted neither cpuid.c nor
msr.c are any better.
In the end the point of wanting things static for as long as they need
to be so is such that reviewers can notice when something gains
wider use. I fully accept that the "set" operation will want access to
the array, but we can't demand of other contributors to avoid
needlessly making symbols global and at the same time behave
differently ourselves. My ack stands without the requested adjustment
if this goes in together with the patch implementing "set".
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tboot: Avoid recursive fault in early boot panic with tboot
>>> On 16.07.18 at 13:59, wrote:
> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -391,7 +391,12 @@ void tboot_shutdown(uint32_t shutdown_type)
> tboot_gen_xenheap_integrity(g_tboot_shared->s3_key, _mac);
> }
>
> -write_ptbase(idle_vcpu[0]);
> +/* During early boot, we can be called by panic before idle_vcpu[0] is
> + * setup, but in that case we don't need to change page tables. */
> +if ( idle_vcpu[0] != INVALID_VCPU )
> +{
> +write_ptbase(idle_vcpu[0]);
> +}
There are two style issues here: The comment wants to be properly
formatted, and the braces want to be dropped.
Also please honor patch submission rules - they get sent _to_ the
list, with maintainers _cc_-ed.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/8] cpupools: fix state when downing a CPU failed
>>> On 16.07.18 at 13:47, wrote:
> On 16/07/18 11:17, Jan Beulich wrote:
> On 13.07.18 at 11:02, wrote:
>>> On 11/07/18 14:04, Jan Beulich wrote:
While I've run into the issue with further patches in place which no
longer guarantee the per-CPU area to start out as all zeros, the
CPU_DOWN_FAILED processing looks to have the same issue: By not zapping
the per-CPU cpupool pointer, cpupool_cpu_add()'s (indirect) invocation
of schedule_cpu_switch() will trigger the "c != old_pool" assertion
there.
Clearing the field during CPU_DOWN_PREPARE is too early (afaict this
should not happen before cpu_disable_scheduler()). Clearing it in
CPU_DEAD and CPU_DOWN_FAILED would be an option, but would take the same
piece of code twice. Since the field's value shouldn't matter while the
CPU is offline, simply clear it in CPU_ONLINE and CPU_DOWN_FAILED, but
only for other than the suspend/resume case (which gets specially
handled in cpupool_cpu_remove()).
Signed-off-by: Jan Beulich
---
TBD: I think this would better call schedule_cpu_switch(cpu, NULL) from
cpupool_cpu_remove(), but besides that - as per above - likely
being too early, that function has further prereqs to be met. It
also doesn't look as if cpupool_unassign_cpu_helper() could be used
there.
--- a/xen/common/cpupool.c
+++ b/xen/common/cpupool.c
@@ -778,6 +778,8 @@ static int cpu_callback(
{
case CPU_DOWN_FAILED:
case CPU_ONLINE:
+if ( system_state <= SYS_STATE_active )
+per_cpu(cpupool, cpu) = NULL;
rc = cpupool_cpu_add(cpu);
>>>
>>> Wouldn't it make more sense to clear the field in cpupool_cpu_add()
>>> which already is testing system_state?
>>
>> Hmm, this may be a matter of taste: I consider the change done here
>> a prereq to calling the function in the first place. As said in the
>> description, I actually think this should come earlier, and it's just that
>> I can't see how to cleanly do so.
You didn't comment on this one at all, yet it matters for how a v2
is supposed to look like.
>>> Modifying the condition in cpupool_cpu_add() to
>>>
>>> if ( system_state <= SYS_STATE_active )
>>>
>>> at the same time would have the benefit to catch problems in case
>>> suspending cpus is failing during SYS_STATE_suspend (I'd expect
>>> triggering the first ASSERT in schedule_cpu_switch() in this case).
>>
>> You mean the if() there, not the else? If so - how would the "else"
>> body then ever be reached? IOW if anything I could only see the
>> "else" to become "else if ( system_state <= SYS_STATE_active )".
>
> Bad wording on my side.
>
> I should have written "the condition in cpupool_cpu_add() should match
> if ( system_state <= SYS_STATE_active )."
>
> So: "if ( system_state > SYS_STATE_active )", as the test is for the
> other case.
I'd recommend against this, as someone adding a new SYS_STATE_*
past suspend/resume would quite likely miss this one. The strong
ordering of states imo should only be used for active and lower states.
But yes, I could see the if() there to become suspend || resume to
address the problem you describe.
Coming back to your DOWN_FAILED consideration: Why are you
thinking this can't happen during suspend? disable_nonboot_cpus()
uses plain cpu_down() after all.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
