Re: [Xen-devel] ovmf fail to compile
"git clean -fdx" doesn't change the error result with gcc 4.4.7. Gcc version is "gcc-4.4.?" in Debian Jessie of yours? Additional, gcc 4.8.5 in RHEL7.2 works for build ovmf. > -Original Message- > From: Wei Liu [mailto:wei.l...@citrix.com] > Sent: Wednesday, November 4, 2015 12:19 AM > To: Hao, Xudong > Cc: Wei Liu ; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] ovmf fail to compile > > On Tue, Nov 03, 2015 at 12:07:27AM +, Hao, Xudong wrote: > > Wei, > > > > I checked the latest OVMF changeset on Xen staging tree and master tree, > both are af9785a9ed61daea52b47f0bf448f1f228beee1e. This commit is what I > used. > > OVMF_UPSTREAM_REVISION ?= > af9785a9ed61daea52b47f0bf448f1f228beee1e > > > > Yes, I found the Config.mk updated with Xen commit > 04c5efb0a141fa53e805e396970419436e74ce67, the old OVMF works fine for > me. > > diff --git a/Config.mk b/Config.mk > > index 114db9a..5db7ca5 100644 > > --- a/Config.mk > > +++ b/Config.mk > > @@ -253,7 +253,7 @@ QEMU_TRADITIONAL_URL ?= > > git://xenbits.xen.org/qemu-xen-traditional.git > > SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git > > MINIOS_UPSTREAM_URL ?= git://xenbits.xen.org/mini-os.git endif > > -OVMF_UPSTREAM_REVISION ?= > cb9a7ebabcd6b8a49dc0854b2f9592d732b5afbd > > +OVMF_UPSTREAM_REVISION ?= > af9785a9ed61daea52b47f0bf448f1f228beee1e > > QEMU_UPSTREAM_REVISION ?= master > > MINIOS_UPSTREAM_REVISION ?= > 256035e01a1aa5739e34f245f3b1e9e8ee204210 > > # Thu Jul 23 11:08:38 2015 +0100 > > > > > -Original Message- > > > From: Wei Liu [mailto:wei.l...@citrix.com] > > > Sent: Monday, November 2, 2015 5:59 PM > > > To: Hao, Xudong > > > Cc: xen-devel@lists.xen.org; wei.l...@citrix.com > > > Subject: Re: [Xen-devel] ovmf fail to compile > > > > > > On Mon, Nov 02, 2015 at 07:27:32AM +, Hao, Xudong wrote: > > > > Hi, > > > > > > > > Does anyone meet build OVMF failure? The ovmf is xen default repo: > > > http://xenbits.xen.org/git-http/ovmf.git, with latest commit > > > af9785a9ed61daea52b47f0bf448f1f228beee1e, and OS is X86_64 RHEL6.6. > > > > > > > > > > > > ... > > > > make[1]: *** > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923-20151029/tools > > > > /fir > > > > mware/ovmf- > > > dir/Build/OvmfX64/DEBUG_GCC44/X64/OvmfPkg/Sec/SecMain/DEBUG > > > > /SecMain.dll] Error 1 > > > > > > > > > > > > build.py... > > > > : error 7000: Failed to execute command > > > >make tbuild > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923-20151029/tools > > > > /fir > > > > mware/ovmf- > dir/Build/OvmfX64/DEBUG_GCC44/X64/OvmfPkg/Sec/SecMain] > > > > > > > > > > > > build.py... > > > > : error 7000: Failed to execute command > > > >make tbuild > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923-20151029/tools > > > > /fir > > > > mware/ovmf- > > > dir/Build/OvmfX64/DEBUG_GCC44/X64/MdeModulePkg/Universal/PC > > > > D/Pei/Pcd] > > > > > > > > > > > > build.py... > > > > : error 7000: Failed to execute command > > > >make tbuild > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923-20151029/tools > > > > /fir > > > > mware/ovmf- > > I got similar error when using gcc-4.9 when I built an old repository that > tracked > upstream. Then I did "git clean -fdx" to remove all stale files and rebuilt. > It > worked. Note that "git clean -fdx" is very aggressive in cleaning up things. > > After that I also tried building with gcc-4.4 and gcc-4.9 with the changeset > that > distributed with Xen. It worked, too. I use Debian Jessie to build stuff. I > don't > expect the distro plays an important role though. > > Wei. gcc4.4.log Description: gcc4.4.log ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [VOTE] Release cycle scheme
Hi everyone, may I ask you to prioritise this vote, such that 4.7 release planning can start? Right now, it is holding up the release planning process, which normally starts around a month after a release (the release was on the 13th of Oct, well a little earlier of we count branching). Lars > On 2 Nov 2015, at 13:47, Wei Liu wrote: > > Hi committers, > > There doesn't seem to be consensus on how release cycle should be > managed. In the survey [0] about release cycle there were following > proposed schemes: > > #1. 6 months release cycle + current stable release scheme > #2. 6 months release cycle + LTS scheme > #3. 6 months release cycle + extended security support > #4. 9 months release cycle + current stable release scheme (no change >at all) > > And the tally: > > #1 #2 #3 #4 > George+1 +2 -2 > Dario +1 +2 -2 > Stefano +1 +2 -2 > Ian C +1 +1 +1 -1 > Olaf +1 0 +1 0 > Juergen0 -1 +1 > Ian J +2 +1 +1 -2 > Andrew+1 +1 -1 > Jan -1 -1 0 +1 > > > There are comments made by individuals that couldn't be clearly > represent in tally. The most acceptable option to stable tree > maintainers is #1. > > So I propose we use the following scheme: > > - 6 months release cycle from unstable branch. > - 4 months development. > - 2 months freeze. > - Eat into next cycle if doesn't release on time. > - Fixed cut-off date: the Fridays of the week in which the last day of > March and September falls. > - No more freeze exception, but heads-up mails about freeze will be > sent a few weeks before hand. > - Stable branch maintained for 18 months full support plus 18 months > security support. No mixed maintainership for stable trees. > > > Please vote to ack or nack this proposal. > > > Thanks > Wei. > > [0]: <20151012173222.ge2...@zion.uk.xensource.com> > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 63528: regressions - FAIL
flight 63528 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/63528/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 5 libvirt-build fail REGR. vs. 63340 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass version targeted for testing: libvirt ac339206bfe98e78925b183cba058d0e2e7f03e3 baseline version: libvirt 3c7590e0a435d833895fc7b5be489e53e223ad95 Last test of basis63340 2015-10-28 04:19:47 Z7 days Failing since 63352 2015-10-29 04:20:29 Z6 days5 attempts Testing same since63373 2015-10-30 04:21:45 Z5 days4 attempts People who touched revisions under test: Laine Stump Luyao Huang Maxim Perevedentsev Michal Privoznik Roman Bogorodskiy jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt fail build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm blocked test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-armhf-armhf-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw blocked test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit ac339206bfe98e78925b183cba058d0e2e7f03e3 Author: Laine Stump Date: Thu Oct 29 14:09:59 2015 -0400 util: set max wait for IPv6 DAD to 20 seconds This was originally set to 5 seconds, but times of 5.5 to 7 seconds were experienced. Since it's an arbitrary number intended to prevent an infinite hang, having it a bit too high won't hurt anything, and 20 seconds looks to be adequate (i.e. I think/hope we don't need to make it tunable in libvirtd.conf) commit d41a64a1948c88ccec5b4cff34fd04d3aae7a71e Author: Luyao Huang Date: Thu Oct 29 17:47:33 2015 +0800 util: set error if DAD is not finished If DAD not finished in 5 seconds, user will get an unknown error like this: # virsh net-start ipv6
Re: [Xen-devel] ovmf fail to compile
On Wed, 2015-11-04 at 08:27 +, Hao, Xudong wrote: Please don't top post. > "git clean -fdx" doesn't change the error result with gcc 4.4.7. Gcc > version is "gcc-4.4.?" in Debian Jessie of yours? "git clean -fdx" won't clean recursively into any sub git trees (some extra number of -f and/or -d is needed for that). I don't know how many, the man page suggests "git clean -ffdx" does it, I usually just go -dx ;-) Ian. > > Additional, gcc 4.8.5 in RHEL7.2 works for build ovmf. > > > -Original Message- > > From: Wei Liu [mailto:wei.l...@citrix.com] > > Sent: Wednesday, November 4, 2015 12:19 AM > > To: Hao, Xudong > > Cc: Wei Liu ; xen-devel@lists.xen.org > > Subject: Re: [Xen-devel] ovmf fail to compile > > > > On Tue, Nov 03, 2015 at 12:07:27AM +, Hao, Xudong wrote: > > > Wei, > > > > > > I checked the latest OVMF changeset on Xen staging tree and master > > > tree, > > both are af9785a9ed61daea52b47f0bf448f1f228beee1e. This commit is what > > I > > used. > > > OVMF_UPSTREAM_REVISION ?= > > af9785a9ed61daea52b47f0bf448f1f228beee1e > > > > > > Yes, I found the Config.mk updated with Xen commit > > 04c5efb0a141fa53e805e396970419436e74ce67, the old OVMF works fine for > > me. > > > diff --git a/Config.mk b/Config.mk > > > index 114db9a..5db7ca5 100644 > > > --- a/Config.mk > > > +++ b/Config.mk > > > @@ -253,7 +253,7 @@ QEMU_TRADITIONAL_URL ?= > > > git://xenbits.xen.org/qemu-xen-traditional.git > > > SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git > > > MINIOS_UPSTREAM_URL ?= git://xenbits.xen.org/mini-os.git endif > > > -OVMF_UPSTREAM_REVISION ?= > > cb9a7ebabcd6b8a49dc0854b2f9592d732b5afbd > > > +OVMF_UPSTREAM_REVISION ?= > > af9785a9ed61daea52b47f0bf448f1f228beee1e > > > QEMU_UPSTREAM_REVISION ?= master > > > MINIOS_UPSTREAM_REVISION ?= > > 256035e01a1aa5739e34f245f3b1e9e8ee204210 > > > # Thu Jul 23 11:08:38 2015 +0100 > > > > > > > -Original Message- > > > > From: Wei Liu [mailto:wei.l...@citrix.com] > > > > Sent: Monday, November 2, 2015 5:59 PM > > > > To: Hao, Xudong > > > > Cc: xen-devel@lists.xen.org; wei.l...@citrix.com > > > > Subject: Re: [Xen-devel] ovmf fail to compile > > > > > > > > On Mon, Nov 02, 2015 at 07:27:32AM +, Hao, Xudong wrote: > > > > > Hi, > > > > > > > > > > Does anyone meet build OVMF failure? The ovmf is xen default > > > > > repo: > > > > http://xenbits.xen.org/git-http/ovmf.git, with latest commit > > > > af9785a9ed61daea52b47f0bf448f1f228beee1e, and OS is X86_64 RHEL6.6. > > > > > > > > > > > > > > > ... > > > > > make[1]: *** > > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923- > > > > > 20151029/tools > > > > > /fir > > > > > mware/ovmf- > > > > dir/Build/OvmfX64/DEBUG_GCC44/X64/OvmfPkg/Sec/SecMain/DEBUG > > > > > /SecMain.dll] Error 1 > > > > > > > > > > > > > > > build.py... > > > > > : error 7000: Failed to execute command > > > > > make tbuild > > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923- > > > > > 20151029/tools > > > > > /fir > > > > > mware/ovmf- > > dir/Build/OvmfX64/DEBUG_GCC44/X64/OvmfPkg/Sec/SecMain] > > > > > > > > > > > > > > > build.py... > > > > > : error 7000: Failed to execute command > > > > > make tbuild > > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923- > > > > > 20151029/tools > > > > > /fir > > > > > mware/ovmf- > > > > dir/Build/OvmfX64/DEBUG_GCC44/X64/MdeModulePkg/Universal/PC > > > > > D/Pei/Pcd] > > > > > > > > > > > > > > > build.py... > > > > > : error 7000: Failed to execute command > > > > > make tbuild > > > > > [/home/nightly/builds_xen_unstable/xen-src-bf0d4923- > > > > > 20151029/tools > > > > > /fir > > > > > mware/ovmf- > > > > I got similar error when using gcc-4.9 when I built an old repository > > that tracked > > upstream. Then I did "git clean -fdx" to remove all stale files and > > rebuilt. It > > worked. Note that "git clean -fdx" is very aggressive in cleaning up > > things. > > > > After that I also tried building with gcc-4.4 and gcc-4.9 with the > > changeset that > > distributed with Xen. It worked, too. I use Debian Jessie to build > > stuff. I don't > > expect the distro plays an important role though. > > > > Wei. > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 0/5] xl: convert exit codes to EXIT_[SUCCESS|FAILURE]
On Wed, 2015-10-28 at 07:56 +0530, Harmandeep Kaur wrote: All seems to be acked by Wei and Reviewed by Dario => applied, thanks everyone. > *Its my "bite size contribution" that is required for applying to the > Outreachy program. > > *main_foo() is treated somewhat as a regular main(), it is changed to > return EXIT_SUCCESS or EXIT_FAILURE. > > *Functions that are not main_foo(), are changed to do 'return 0' or > 'return 1', and then 0/1 is taken care in the main_foo() functions > that calls them. > > *Functions in xl.c and xl_cmdimpl.c are changed. > > *Functions related to scheduling, vcpu, cpupool and parsing are > currently changed excluding parse_config_data() which is big enough > to deserve its own patch. > > *Some discussions about this patch: > https://www.mail-archive.com/xen-devel@lists.xen.org/msg42850.html > > *v1 of this patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02497.html > > *v2 of this patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02623.html > > *v3 of this patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02872.html > > *v4 of this patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02947.html > > *v5 of this patch: > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02990.html > > Signed-off-by: Harmandeep Kaur > --- ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 0/5] xl: convert exit codes to EXIT_[SUCCESS|FAILURE]
On Wed, 2015-11-04 at 09:40 +, Ian Campbell wrote: > On Wed, 2015-10-28 at 07:56 +0530, Harmandeep Kaur wrote: > > All seems to be acked by Wei and Reviewed by Dario => applied, thanks > everyone. BTW, Harmandeep, I think I have now applied everything you have submitted. There were a couple of other stray patches in my QUEUE folder, but they appeared to be precursors to these patches which had become unthreaded or to have be otherwise superseded somehow. If you think there is something outstanding them please ping me or resend. Thanks, Ian. > > > *Its my "bite size contribution" that is required for applying to the > > Outreachy program. > > > > *main_foo() is treated somewhat as a regular main(), it is changed to > > return EXIT_SUCCESS or EXIT_FAILURE. > > > > *Functions that are not main_foo(), are changed to do 'return 0' or > > 'return 1', and then 0/1 is taken care in the main_foo() functions > > that calls them. > > > > *Functions in xl.c and xl_cmdimpl.c are changed. > > > > *Functions related to scheduling, vcpu, cpupool and parsing are > > currently changed excluding parse_config_data() which is big enough > > to deserve its own patch. > > > > *Some discussions about this patch: > > https://www.mail-archive.com/xen-devel@lists.xen.org/msg42850.html > > > > *v1 of this patch: > > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02497.ht > > ml > > > > *v2 of this patch: > > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02623.ht > > ml > > > > *v3 of this patch: > > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02872.ht > > ml > > > > *v4 of this patch: > > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02947.ht > > ml > > > > *v5 of this patch: > > http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02990.ht > > ml > > > > Signed-off-by: Harmandeep Kaur > > --- > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [distros-debian-squeeze test] 38246: all pass
flight 38246 distros-debian-squeeze real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/38246/ Perfect :-) All tests in this flight passed baseline version: flight 38217 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-squeeze-netboot-pygrubpass test-amd64-i386-amd64-squeeze-netboot-pygrub pass test-amd64-amd64-i386-squeeze-netboot-pygrub pass test-amd64-i386-i386-squeeze-netboot-pygrub pass 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.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [VOTE] Release cycle scheme
On Mon, Nov 2, 2015 at 1:47 PM, Wei Liu wrote: > - 6 months release cycle from unstable branch. > - 4 months development. > - 2 months freeze. > - Eat into next cycle if doesn't release on time. ... > - Stable branch maintained for 18 months full support plus 18 months > security support. No mixed maintainership for stable trees. These are the only things I think we actually need to vote on; and for these I have a +1 from me. > - Fixed cut-off date: the Fridays of the week in which the last day of > March and September falls. > - No more freeze exception, but heads-up mails about freeze will be > sent a few weeks before hand. These should be left at the discretion of the release manager, as they are now. (FWIW they sound like reasonable decisions to me.) -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] ovmf fail to compile
On Wed, Nov 4, 2015 at 9:38 AM, Ian Campbell wrote: > On Wed, 2015-11-04 at 08:27 +, Hao, Xudong wrote: > > Please don't top post. > >> "git clean -fdx" doesn't change the error result with gcc 4.4.7. Gcc >> version is "gcc-4.4.?" in Debian Jessie of yours? > > "git clean -fdx" won't clean recursively into any sub git trees (some extra > number of -f and/or -d is needed for that). I don't know how many, the man > page suggests "git clean -ffdx" does it, I usually just go -dx > ;-) Which translates roughly to, "G%# ^#$@ it, I said to *#$@ delete #%@##^ everything! What part of 'everything' do you not #@#& understand?" :-) (FWIW I always do git -ffdx.) -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] ovmf fail to compile
On Wed, 2015-11-04 at 10:04 +, George Dunlap wrote: > On Wed, Nov 4, 2015 at 9:38 AM, Ian Campbell > wrote: > > On Wed, 2015-11-04 at 08:27 +, Hao, Xudong wrote: > > > > Please don't top post. > > > > > "git clean -fdx" doesn't change the error result with gcc 4.4.7. Gcc > > > version is "gcc-4.4.?" in Debian Jessie of yours? > > > > "git clean -fdx" won't clean recursively into any sub git trees (some > > extra > > number of -f and/or -d is needed for that). I don't know how many, the > > man > > page suggests "git clean -ffdx" does it, I usually just go > > -dx > > ;-) > > Which translates roughly to, "G%# ^#$@ it, I said to *#$@ delete > #%@##^ everything! What part of 'everything' do you not #@#& > understand?" :-) :-) > > (FWIW I always do git -ffdx.) > > -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 63527: regressions - FAIL
flight 63527 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/63527/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 63466 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 9 debian-installfail REGR. vs. 63466 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeatfail like 63466 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 9 debian-di-installfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 9 debian-di-installfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 9 debian-di-installfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-libvirt-raw 9 debian-di-installfail never pass version targeted for testing: qemuu3d861a01093f8eedfac9889746ccafcfd32039b7 baseline version: qemuu3a958f559ecd0511583d27b10011fa7f3cf79b63 Last test of basis63466 2015-11-01 17:23:53 Z2 days Testing same since63527 2015-11-03 04:00:43 Z1 days1 attempts People who touched revisions under test: Chen Gang Daniel P. Berrange Eric Blake Markus Armbruster Peter Maydell Richard Henderson jobs: build-amd64-xsm pass build-armhf-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 pass test-amd64-i386-xl pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass 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-armhf-armhf-libvirt-xsm fail
Re: [Xen-devel] [RFC PATCH v2 00/16] Load BIOS via toolstack instead of been embedded in hvmloader.
On Tue, 2015-11-03 at 17:50 +, Anthony PERARD wrote: > On Tue, Nov 03, 2015 at 05:30:32PM +, Ian Campbell wrote: > > On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > > > Hi all, > > > > > > I've start to look at loading the BIOS and the ACPI tables via the > > > toolstack instead of having them embedded in the hvmloader binary. > > > After > > > this patch series, hvmloader compilation would be indenpendant from > > > SeaBIOS > > > and OVMF compilation. > > > > > > Compare to the V1, this is now done via the struct hvm_start_info > > > from > > > Roger's patch series "Introduce HVM without dm and new boot ABI". > > > > Just to be clear, does this therefore requires the rest of (or some > > more > > of) Roger's series to be applied or just the initial dozen patches > > which > > are already in? > > The struct in introduced in this patch: > <1443800943-17668-30-git-send-email-roger@citrix.com> > [PATCH v7 29/32] libxc/xen: introduce a start info structure for HVMlite > guests > > And is not yet applied, so yes, it does require the rest of the patch > series, I have not look at which patches in particular. OK, I'll keep it in mind and not try to apply this one if it ends up ready first. > > > Here is a general view of the few step to load the BIOS: > > > - libxl load the BIOS blob into it's memory and add it to struct > > > xc_hvm_build_args.bios_module > > > - libxc load the blob into the guest memory and fill the struct > > > hvm_start_info, with a cmdline which would contain the order in > > > which the > > > modules (bios, acpi_table) are loaded into the modlist. > > > - hvmloader read the addresses from the hvm_start_info, find out > > > which > > > module are which by reading the cmdline and copy the blob to the > > > right > > > place. > > > > Hrm, it's a shame that the mod list doesn't contain this information, > > laundering it via a string cmdline seems a bit sub optimal. I haven't > > looked yet but my intuition is that this will involve hvmloader doing > > some > > string parsing, which I'm not keen on. > > > > Is the modlist a stable API (yet?) or can we extend it if we want? > > I'm not sure what could be added to it. An extra string that describe the > module maybe? Or ... I should have CCd Roger, now done. I think this new interface (is going to) form part of the PVH boot ABI, which is currently not formally stabilised but it means any semantics we choose to give it need to be considered in that light and not just in the "internal between tools and hvmloader" one. I'm not sure what this means for adding a type to it, but on first glance that seems like an "internal between tools and hvmloader" thing not a public ABI thing. > > Failing that perhaps hvmloader and libx? could collude such that the > > first > > module is always some data structure (a private interface between > > hvmloader > > and the tools) which describes the contents of the remaining modules? > > ... or we could have the modules been allocated in the same order, on a > specific position. So BIOS always first, ACPI table always second ..., and > if one is missing or not needed, replace it by a module of size 0. Since this would be an "internal interface" I think we could do, although it might be more convenient for developers trying to change this in the future to include a little more flexibility? (e.g. co-bisection of tools and hvmloader over a change to the list ordering) In the end if this is an "internal interface" I suppose it doesn't matter too much what colour we paint it to start with. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] ovmf fail to compile
On Wed, Nov 04, 2015 at 08:27:56AM +, Hao, Xudong wrote: > "git clean -fdx" doesn't change the error result with gcc 4.4.7. Gcc version > is "gcc-4.4.?" in Debian Jessie of yours? > Debian Jessie's gcc-4.4 has the same version 4.4.7. As the other sub-thread suggests, can you try passing more f's to git? A somewhat related question, are you only interested in xen-unstable branch? Have you tried latest OVMF from upstream? If that builds for you I can easily send another patch to update Config.mk again. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 03/16] configure: #define SEABIOS_PATH and OVMF_PATH
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > Those paths are to be used by libxl, in order to load the firmware in > memory. If a system path is not define, then this default to the Xen > firmware directory. AIUI these are both existing variables which are right now exposed only to the Makefiles and not to the C code, and this adds the latter. That only matters because I then don't have to think about the semantics of a "new" option, what the matching command line option behaviour is, docs etc. > Signed-off-by: Anthony PERARD > > --- > Please, run ./autogen.sh on this patch. > --- > tools/configure.ac | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/configure.ac b/tools/configure.ac > index 6c70040..6929006 100644 > --- a/tools/configure.ac > +++ b/tools/configure.ac > @@ -212,6 +212,9 @@ AC_ARG_WITH([system-seabios], > esac > ],[]) > AC_SUBST(seabios_path) > +AC_DEFINE_UNQUOTED([SEABIOS_PATH], > + ["${seabios_path:-$XENFIRMWAREDIR/seabios.bin}"], > + [SeaBIOS path]) I thought ${foo:-bar} might not be POSIXly-correct, but it turns out it is http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html , so good! ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 04/16] firmware/makefile: install BIOS and ACPI blob ...
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > ... into the firmware directory, along with hvmloader. > > Signed-off-by: Anthony PERARD > --- > .gitignore | 1 + > tools/firmware/Makefile| 15 +++ > tools/firmware/hvmloader/acpi/Makefile | 2 +- > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/.gitignore b/.gitignore > index 9ead7c4..7c7bb56 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -117,6 +117,7 @@ tools/firmware/hvmloader/acpi/mk_dsdt > tools/firmware/hvmloader/acpi/dsdt*.c > tools/firmware/hvmloader/acpi/dsdt_*.asl > tools/firmware/hvmloader/acpi/ssdt_*.h > +tools/firmware/hvmloader/acpi/dsdt_anycpu_qemu_xen.aml > tools/firmware/hvmloader/hvmloader > tools/firmware/hvmloader/roms.h > tools/firmware/hvmloader/roms.inc > diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile > index 6cc86ce..9c63991 100644 > --- a/tools/firmware/Makefile > +++ b/tools/firmware/Makefile > @@ -19,6 +19,10 @@ SUBDIRS-y += hvmloader > > LD32BIT-$(CONFIG_FreeBSD) := LD32BIT_FLAG=-melf_i386_fbsd > > +SEABIOS_ROM := seabios-dir/out/bios.bin > +OVMF_ROM := ovmf-dir/ovmf.bin > +ACPI_TABLE_QEMU_PC_I440FX = hvmloader/acpi/dsdt_anycpu_qemu_xen.aml > + > ovmf-dir: > GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh > $(OVMF_UPSTREAM_URL) $(OVMF_UPSTREAM_REVISION) ovmf-dir > cp ovmf-makefile ovmf-dir/Makefile; > @@ -45,6 +49,17 @@ endif > install: all > [ -d $(INST_DIR) ] || $(INSTALL_DIR) $(INST_DIR) > [ ! -e $(TARGET) ] || $(INSTALL_DATA) $(TARGET) $(INST_DIR) > +ifeq ($(CONFIG_SEABIOS),y) > +ifeq ($(SEABIOS_PATH),) > + [ ! -e $(SEABIOS_ROM) ] || $(INSTALL_DATA) $(SEABIOS_ROM) > $(INST_DIR)/seabios.bin If $(SEABIOS_ROM) doesn't exist surely that indicates something has gone wrong prior to this? The -e $(TARGET) you are (presumably) copying is similarly strange, although maybe that is to do with it not being inside an ifeq like the CONFIG_SEABIOS which protects your new one. IOW I think you can drop the -e check from both, the ifeq is clearer and less prone to carrying on in the face of other errors AFAICT. > +endif > +endif > +ifeq ($(CONFIG_OVMF),y) > +ifeq ($(OVMF_PATH),) > + [ ! -e $(OVMF_ROM) ] || $(INSTALL_DATA) $(OVMF_ROM) > $(INST_DIR)/ovmf.bin > +endif > +endif > +>> [ ! -e $(ACPI_TABLE_QEMU_PC_I440FX) ] || $(INSTALL_DATA) > $(ACPI_TABLE_QEMU_PC_I440FX) $(INST_DIR) > > .PHONY: clean > clean: subdirs-clean > diff --git a/tools/firmware/hvmloader/acpi/Makefile > b/tools/firmware/hvmloader/acpi/Makefile > index d3e882a..3d8dd21 100644 > --- a/tools/firmware/hvmloader/acpi/Makefile > +++ b/tools/firmware/hvmloader/acpi/Makefile > @@ -46,7 +46,7 @@ $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl > iasl -vs -p $* -tc $*.asl > sed -e 's/AmlCode/$*/g' $*.hex >$@ > echo "int $*_len=sizeof($*);" >>$@ > - rm -f $*.aml $*.hex > + rm -f $*.hex Is $*.aml an incidental output of iasl? The iasl manpage is not terribly informative on the issue, but it says that -tc will create C output in a .hex fix. I suppose the .aml is implied. ./mk_dsdt --debug=$(debug) --maxcpu $* >> $@ I think (but please check, there may be some weird subtlety with this sort of rule) that it would be correct to change the rule here from: $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl into: $(filter dsdt_%.c,$(C_SRC)): %.c %.aml: iasl %.asl (my actual preference would be to invoke iasl twice, once for asl->aml and then again for aml->hex, but that doesn't seem possible). Oh, maybe the need for .hex file will go away later in this series and so you can adjust this to only talk about the .aml? In which case after mentioning that in the commit message you could ignore all the above and fix up the rules properly at that point Do you not need to also add this newly left lying around file to .gitignore. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 07/16] hvmloader: Grab the hvmlite info page and parse the cmdline
On 26/10/15 16:03, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD > --- > tools/firmware/hvmloader/hvmloader.c | 69 > +++- > 1 file changed, 68 insertions(+), 1 deletion(-) As part of using the DMLite infrastructure, hvmloader should gain appropriate elfnotes. This will avoid the need to duplicate the dmlite codepath in libxc just for hvmloader. > > diff --git a/tools/firmware/hvmloader/hvmloader.c > b/tools/firmware/hvmloader/hvmloader.c > index 716d03c..9df12ac 100644 > --- a/tools/firmware/hvmloader/hvmloader.c > +++ b/tools/firmware/hvmloader/hvmloader.c > @@ -62,7 +62,7 @@ asm ( > "mov %ax,%ss\n" mov %ebx, start_info and have static const struct hvm_start_info *hvmlite_start_info; somewhere other than the main() stack. However, I would avoid naming it "hvmlite". > /* Initialise all 32-bit GPRs to zero. */ > "xor %eax,%eax \n" > -"xor %ebx,%ebx \n" > +/* Keep ebx, for HVMLite start info */ > "xor %ecx,%ecx \n" > "xor %edx,%edx \n" > "xor %esp,%esp \n" > @@ -249,15 +249,82 @@ static void acpi_enable_sci(void) > BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN)); > } > > +static const char *module_list_order = NULL; > +void cmdline_parser(const char *the_cmdline) > +{ > +char cmdline[MAX_GUEST_CMDLINE]; > +char *p, *q; > +char *optval; > + > +strncpy(cmdline, the_cmdline, sizeof (cmdline)); > +cmdline[MAX_GUEST_CMDLINE-1] = '\0'; Why do you need to copy the command line? It is already in writable hvmloader memory. You also need to check for NULL, being the indication that no command line has been provided. ~Andrew > + > +for ( p = cmdline; p < (cmdline + MAX_GUEST_CMDLINE) && *p; p++ ) > +{ > +if ( *p == ' ' ) > +continue; > + > +/* search for the end of the parameter name */ > +for ( q = p; *q && *q != '=' && *q != ' '; q++ ) > +; > + > +/* search for the end of the optional paremeter value */ > +if ( *q == '=' ) > +{ > +optval = q+1; > +if (*optval == '\0' || *optval == ' ') { > +optval = NULL; > +} > +} else > +optval = NULL; > +*q = '\0'; > + > +if ( optval ) > +{ > +for ( q = optval; *q && *q != ' '; q++ ) > +; > +*q = '\0'; > +} > + > +/* compare known parameters */ > +if ( !strcmp(p, "modules") ) > +{ > +printf(" cmdline: found '%s', with val '%s'\n", p, optval); > +if ( optval ) > +{ > +unsigned size = strlen(optval) + 1; > +char *tmp = scratch_alloc(size, 0); > +strncpy(tmp, optval, size); > +module_list_order = tmp; > +} > +} else { > +printf(" Unknown cmdline option '%s'", p); > +if ( optval ) > +printf(" with val '%s'\n", optval); > +else > +printf("\n"); > +} > + > +p = q; > +} > +} > + > int main(void) > { > const struct bios_config *bios; > int acpi_enabled; > +const struct hvm_start_info *hvmlite_start_info; > + > +/* Load hvmlite start info pointer from ebx. */ > +asm volatile ( "mov %%ebx,%0" : "=r" (hvmlite_start_info) ); > > /* Initialise hypercall stubs with RET, rendering them no-ops. */ > memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE); > > printf("HVM Loader\n"); > +BUG_ON(hvmlite_start_info->magic != HVM_START_MAGIC_VALUE); > +printf("cmdline: %s\n", (char*)hvmlite_start_info->cmdline_paddr); > +cmdline_parser((char*)hvmlite_start_info->cmdline_paddr); > > init_hypercalls(); > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [VOTE] Release cycle scheme
>>> On 02.11.15 at 14:47, wrote: > So I propose we use the following scheme: > > - 6 months release cycle from unstable branch. > - 4 months development. > - 2 months freeze. > - Eat into next cycle if doesn't release on time. > - Fixed cut-off date: the Fridays of the week in which the last day of > March and September falls. > - No more freeze exception, but heads-up mails about freeze will be > sent a few weeks before hand. > - Stable branch maintained for 18 months full support plus 18 months > security support. No mixed maintainership for stable trees. -1 overall (the different points would get different ratings individually) Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3] xl: log an error if libxl_cpupool_destroy() fails
In fact, right now, failing at destroying a cpupool is just not reported to the user in any explicit way. Let's log an error, as it is customary for xl in these cases. Signed-off-by: Dario Faggioli Reviewed-by: Juergen Gross Acked-by: Wei Liu --- Cc: Ian Jackson Cc: Stefano Stabellini Cc: Ian Campbell --- Changes from v2: * rebased on top of staging; * fixed Juergen's email address Changes from v1: * avoid changing the return code to EXIT_SUCCESS/FAILURE; that is being taken care of in another series anyway. --- tools/libxl/xl_cmdimpl.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 2756d2f..5469735 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -7581,8 +7581,10 @@ int main_cpupooldestroy(int argc, char **argv) return EXIT_FAILURE; } -if (libxl_cpupool_destroy(ctx, poolid)) +if (libxl_cpupool_destroy(ctx, poolid)) { +fprintf(stderr, "Can't destroy cpupool '%s'\n", pool); return EXIT_FAILURE; +} return EXIT_SUCCESS; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 05/16] libxl: Load guest BIOS from file
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > The path to the BIOS blob can be override by the xl's bios_override > option, > or provided by u.hvm.bios_filename in the domain_build_info struct by > other > libxl user. > > Signed-off-by: Anthony PERARD > --- > tools/libxl/libxl_dom.c | 66 > + > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c| 11 +--- > 3 files changed, 75 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index b40d744..27a0021 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -858,6 +858,42 @@ err: > return ret; > } > > +static const char *seabios_path(libxl__gc *gc) > +{ > +return SEABIOS_PATH; > +} > + > +static const char *ovmf_path(libxl__gc *gc) > +{ > +return OVMF_PATH; > +} Can you put these in libxl_paths.c (for consistency) please. > + > +static int libxl__load_hvm_firmware_module(libxl__gc *gc, > + const char *filename, > + const char *what, > + struct xc_hvm_firmware_module > *m) > +{ > +int datalen = 0; > +void *data = NULL; > +int e; > + > +LOG(DEBUG, "Loading %s: %s", what, filename); > +e = libxl_read_file_contents(CTX, filename, &data, &datalen); > +if (e) { > +LOGEV(ERROR, e, "failed to read %s file %s", "e" here should be an errno val, not a libxl ERROR_*. So you need to print manually via the format string using either LOG or LOGE (if libxl_read_file_contents also sets errno, probably not). > + what, > + filename); > +return ERROR_FAIL; > +} > +libxl__ptr_add(gc, data); > +if (datalen) { > +/* Only accept non-empty files */ Error or logging otherwise? Silently ignoring seems likely to surprise someone eventually. > +m->data = data; > +m->length = (uint32_t)datalen; I hope that cast isn't really necessary. > +} > +return 0; > +} > + > static int libxl__domain_firmware(libxl__gc *gc, > libxl_domain_build_info *info, > struct xc_dom_image *dom) > @@ -910,6 +946,36 @@ static int libxl__domain_firmware(libxl__gc *gc, > goto out; > } > > +if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) { > +const char *bios_filename; > +// Look for BIOS and load it Proper comment style please. > +if (info->u.hvm.bios_filename) { > +bios_filename = info->u.hvm.bios_filename; > +} else { > +switch (info->u.hvm.bios) > +{ > +case LIBXL_BIOS_TYPE_ROMBIOS: > +bios_filename = libxl__abs_path(gc, "rombios.bin", > + > libxl__xenfirmwaredir_path()); The cover letter implied that rombios was still built in, this suggests not? > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 082fed8..a3fbcab 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -468,6 +468,7 @@ libxl_domain_build_info = > Struct("domain_build_info",[ > ("u", KeyedUnion(None, libxl_domain_type, "type", > [("hvm", Struct(None, [("firmware", string), > ("bios", libxl_bios_type), > + ("bios_filename",string), Such new additions require a #define LIBXL_HAVE_* in libxl.h so applications know they can use it. > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 840d73f..27d7c25 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1500,12 +1500,17 @@ static void parse_config_data(const char > *config_source, > > xlu_cfg_replace_string (config, "firmware_override", > &b_info->u.hvm.firmware, 0); > -if (!xlu_cfg_get_string(config, "bios", &buf, 0) && > -libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) { > +xlu_cfg_replace_string (config, "bios_override", > +&b_info->u.hvm.bios_filename, 0); > +if (!xlu_cfg_get_string(config, "bios", &buf, 0)) { > +if (libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) { Please update the xl.cfg man page to reflect this. > fprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n", > buf); > exit (1); > -} > +} > +} else if (b_info->u.hvm.bios_filename) > +fprintf(stderr, "WARNING: " > +"bios_override given without specific bios name\n"); What I'm about to say is probably not a good idea, but: Given that we might keep rombios in-hvmloader (hence bios=rombios
Re: [Xen-devel] [RFC PATCH v2 06/16] libxl: Load guest ACPI table from file
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > The path to the ACPI tables blob can be override by xl's option "overridden" > acpi_table_override or by acpi_tables_filename in the domain_build_info > struct for libxl user. This needs the same libxl.h #define and xl.cfg update I mentioned before. The code, docs and commit message all need further consideration of the interactions with the existing acpi_firmware option which exists in libxl and is exposed in xl. It allows you to specify some extra tables which are merged (by hvmloader) into the base ones. The naming is a bit unfortunate but we are now stuck with those semantics for the existing option I think. If we can distinguish partial from full tables in the tools reusing the name and doing so might be the best approach. If we can't tell the difference then the new option needs some suitable name such that it is clear it is the full or base table or something. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [GIT PULL] xen: features for 4.4-rc0
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-4.4-rc0-tag xen: features for 4.4-rc0 - - Improve balloon driver memory hotplug placement. - - Use unpopulated hotplugged memory for foreign pages (if supported/enabled). - - Support 64 KiB guest pages on arm64. - - CPU hotplug support on arm/arm64. Thanks. David arch/arm/include/asm/xen/hypervisor.h| 10 + arch/arm/include/asm/xen/page-coherent.h | 26 +- arch/arm/include/asm/xen/page.h | 22 +- arch/arm/xen/enlighten.c | 20 +- arch/arm/xen/mm.c| 39 ++- arch/arm/xen/p2m.c | 6 +- arch/x86/include/asm/xen/hypervisor.h| 5 + arch/x86/include/asm/xen/page.h | 8 +- arch/x86/xen/enlighten.c | 15 + arch/x86/xen/grant-table.c | 2 +- arch/x86/xen/mmu.c | 1 + arch/x86/xen/p2m.c | 19 +- arch/x86/xen/setup.c | 9 +- drivers/block/xen-blkback/blkback.c | 13 +- drivers/block/xen-blkback/common.h | 17 +- drivers/block/xen-blkback/xenbus.c | 11 +- drivers/block/xen-blkfront.c | 560 --- drivers/net/xen-netback/common.h | 16 +- drivers/net/xen-netback/netback.c| 167 + drivers/net/xen-netfront.c | 122 +-- drivers/tty/hvc/hvc_xen.c| 4 +- drivers/xen/Makefile | 2 - drivers/xen/balloon.c| 341 --- drivers/xen/biomerge.c | 8 + drivers/xen/cpu_hotplug.c| 14 +- drivers/xen/events/events_base.c | 2 +- drivers/xen/events/events_fifo.c | 2 +- drivers/xen/grant-table.c| 56 +++- drivers/xen/privcmd.c| 10 +- drivers/xen/swiotlb-xen.c| 43 ++- drivers/xen/xenbus/xenbus_client.c | 128 --- drivers/xen/xenbus/xenbus_probe.c| 3 +- drivers/xen/xlate_mmu.c | 124 --- include/linux/memory_hotplug.h | 2 + include/uapi/xen/gntalloc.h | 22 +- include/uapi/xen/gntdev.h| 34 +- include/xen/balloon.h| 12 +- include/xen/grant_table.h| 57 include/xen/page.h | 27 +- include/xen/xenbus.h | 4 +- mm/memory_hotplug.c | 29 +- 41 files changed, 1365 insertions(+), 647 deletions(-) David Vrabel (11): mm: memory hotplug with an existing resource xen/balloon: remove scratch page left overs x86/xen: discard RAM regions above the maximum reservation xen/balloon: find non-conflicting regions to place hotplugged memory xen/balloon: rationalize memory hotplug stats xen/balloon: only hotplug additional memory if required xen/balloon: make alloc_xenballoon_pages() always allocate low pages xen/balloon: use hotplugged pages for foreign mappings etc. x86/xen: export xen_alloc_p2m_entry() xen/balloon: pre-allocate p2m entries for ballooned pages x86/xen: add reschedule point when mapping foreign GFNs Juergen Gross (1): xen/arm: correct comment in enlighten.c Julien Grall (26): net/xen-netback: xenvif_gop_frag_copy: move GSO check out of the loop arm/xen: Drop pte_mfn and mfn_pte xen: Add Xen specific page definition xen/grant: Introduce helpers to split a page into grant xen/grant: Add helper gnttab_page_grant_foreign_access_ref_one block/xen-blkfront: Split blkif_queue_request in 2 block/xen-blkfront: Store a page rather a pfn in the grant structure block/xen-blkfront: split get_grant in 2 xen/biomerge: Don't allow biovec's to be merged when Linux is not using 4KB pages xen/xenbus: Use Xen page definition tty/hvc: xen: Use xen page definition xen/balloon: Don't rely on the page granularity is the same for Xen and Linux xen/events: fifo: Make it running on 64KB granularity xen/grant-table: Make it running on 64KB granularity block/xen-blkfront: Make it running on 64KB page granularity block/xen-blkback: Make it running on 64KB page granularity net/xen-netfront: Make it running on 64KB page granularity net/xen-netback: Make it running on 64KB page granularity xen/privcmd: Add support for Linux 64KB page granularity arm/xen: Add support for 64KB page granularity xen/swiotlb: Pass addresses rather than frame numbers to xen_arch_need_swiotlb xen/swiotlb: Add support for 64KB page granularity xen/balloon: Use the correct sizeof when declaring frame_list xen/xenbus: Rename *RING_PAGE* to *RING_GRANT* xen/grant-table: Add an helper to iterate over a specific number of grants xenb
Re: [Xen-devel] [RFC PATCH v2 07/16] hvmloader: Grab the hvmlite info page and parse the cmdline
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD > --- > tools/firmware/hvmloader/hvmloader.c | 69 > +++- > 1 file changed, 68 insertions(+), 1 deletion(-) > > diff --git a/tools/firmware/hvmloader/hvmloader.c > b/tools/firmware/hvmloader/hvmloader.c > index 716d03c..9df12ac 100644 > --- a/tools/firmware/hvmloader/hvmloader.c > +++ b/tools/firmware/hvmloader/hvmloader.c > @@ -62,7 +62,7 @@ asm ( > "mov %ax,%ss\n" > /* Initialise all 32-bit GPRs to zero. */ > "xor %eax,%eax \n" > -"xor %ebx,%ebx \n" > +/* Keep ebx, for HVMLite start info */ Isn't this now an ABI between the tools and HVM loader, which is embedded in the more formal PVH/HVMLite startup protocol? In which case hopefully there is somewhere it can be documented. Is a HVMlite/PVH guest allowed to assume anything about %ebx, those sorts of questions need to be considered. > "xor %ecx,%ecx \n" > "xor %edx,%edx \n" > "xor %esp,%esp \n" > @@ -249,15 +249,82 @@ static void acpi_enable_sci(void) > BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN)); > } > > +static const char *module_list_order = NULL; > +void cmdline_parser(const char *the_cmdline) I'll leave this until we've decided if this approach is the one we want. > int main(void) > { > const struct bios_config *bios; > int acpi_enabled; > +const struct hvm_start_info *hvmlite_start_info; > + > +/* Load hvmlite start info pointer from ebx. */ > +asm volatile ( "mov %%ebx,%0" : "=r" (hvmlite_start_info) ); The stub which calls main should perhaps arrange for this to be in a register which ends up being a parameter to this struct. I think otherwise nothing ensures that the function prolog hasn't clobbered %ebx already. > > /* Initialise hypercall stubs with RET, rendering them no-ops. */ > memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, > PAGE_SIZE); > > printf("HVM Loader\n"); > +BUG_ON(hvmlite_start_info->magic != HVM_START_MAGIC_VALUE); > +printf("cmdline: %s\n", (char*)hvmlite_start_info->cmdline_paddr); > +cmdline_parser((char*)hvmlite_start_info->cmdline_paddr); > > init_hypercalls(); > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 08/16] hvmloader: Locate the BIOS blob
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > The BIOS can be found in one of the entry of the modlist of the > hvm_start_info struct. The order of the modules are define by the command > line option "modules". > > The found BIOS blob is not loaded by this patch, but only passed as > argument to bios_load() function. It is going to be used by the next few > patches. > > Signed-off-by: Anthony PERARD > --- > tools/firmware/hvmloader/config.h| 2 +- > tools/firmware/hvmloader/hvmloader.c | 34 +- > tools/firmware/hvmloader/ovmf.c | 3 ++- But not seabios.c or rombios.c? Doesn't this break rombios since it has bios_load hook? (Seabios doesn't it seems). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 09/16] hvmloader: Load SeaBIOS from hvm_start_info modules ...
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > ... and do not include the SeaBIOS ROM into hvmloader anymore. ... but don't yet stop including it in rom.inc (I suppose that comes later) Can we add a new hook to chose an address for the bios which the common code which handles the no-bios_load-hook case could use and which could be propagated to setup_e280 instead of using a global? seabios_config isn't a const, so you could even just set .bios_address dynamically? > > Signed-off-by: Anthony PERARD > --- > tools/firmware/hvmloader/seabios.c | 24 ++-- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/tools/firmware/hvmloader/seabios.c > b/tools/firmware/hvmloader/seabios.c > index c6b3d9f..766bd6b 100644 > --- a/tools/firmware/hvmloader/seabios.c > +++ b/tools/firmware/hvmloader/seabios.c > @@ -27,9 +27,6 @@ > #include "smbios_types.h" > #include "acpi/acpi2_0.h" > > -#define ROM_INCLUDE_SEABIOS > -#include "roms.inc" > - > extern unsigned char dsdt_anycpu_qemu_xen[]; > extern int dsdt_anycpu_qemu_xen_len; > > @@ -121,6 +118,7 @@ static void seabios_create_pir_tables(void) > add_table(create_pir_tables()); > } > > +unsigned int seabios_bios_address = 0; If it really has to remain should at least be static. > static void seabios_setup_e820(void) > { > struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS; > @@ -128,21 +126,27 @@ static void seabios_setup_e820(void) > info->e820 = (uint32_t)e820; > > /* SeaBIOS reserves memory in e820 as necessary so no low > reservation. */ > -info->e820_nr = build_e820_table(e820, 0, 0x10-sizeof(seabios)); > +BUG_ON(seabios_bios_address == 0); > +info->e820_nr = build_e820_table(e820, 0, seabios_bios_address); > dump_e820_table(e820, info->e820_nr); > } > > -struct bios_config seabios_config = { > -.name = "SeaBIOS", > +static void seabios_load(const struct bios_config *bios, > + void *bios_addr, uint32_t bios_length) > +{ > +unsigned int bios_dest = 0x10 - bios_length; > +seabios_bios_address = 0x10 - bios_length; > > -.image = seabios, > -.image_size = sizeof(seabios), > +BUG_ON(bios_dest + bios_length > HVMLOADER_PHYSICAL_ADDRESS); > +memcpy((void *)bios_dest, bios_addr, bios_length); > +} > > -.bios_address = 0x10 - sizeof(seabios), > +struct bios_config seabios_config = { > +.name = "SeaBIOS", > > .load_roms = NULL, > > -.bios_load = NULL, > +.bios_load = seabios_load, > > .bios_info_setup = seabios_setup_bios_info, > .bios_info_finish = seabios_finish_bios_info, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xl: don't print out uninitialised rc
In 5b725e56 (xl: improve return and exit codes of vcpu related functions), the return value of libxl_cpu_bitmap_alloc was not stored in rc anymore. Yet the subsequent fprintf still used that. While it is trivial to reinstate the original implementation, xl's caller has no idea what a libxl error code is so printing out rc won't be that useful. Don't print out rc in that case. Signed-off-by: Wei Liu --- Cc: Ian Campbell Cc: Ian Jackson Cc: Dario Faggioli Cc: Harmandeep Kaur --- tools/libxl/xl_cmdimpl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 2756d2f..6056344 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) return 1; } if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { -fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc); +fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); return 1; } for (i = 0; i < max_vcpus; i++) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 10/16] hvmloader: Load OVMF from modules
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > ... and do not include the OVMF ROM into hvmloader anymore. > > Signed-off-by: Anthony PERARD > --- > tools/firmware/hvmloader/ovmf.c | 22 +++--- > 1 file changed, 7 insertions(+), 15 deletions(-) > > diff --git a/tools/firmware/hvmloader/ovmf.c > b/tools/firmware/hvmloader/ovmf.c > index 2be9752..3c0ec91 100644 > --- a/tools/firmware/hvmloader/ovmf.c > +++ b/tools/firmware/hvmloader/ovmf.c > @@ -34,17 +34,9 @@ > #include > #include > > -#define ROM_INCLUDE_OVMF > -#include "roms.inc" > - > -#define OVMF_SIZE (sizeof(ovmf)) > -#define OVMF_MAXOFFSET 0x000FULL > -#define OVMF_BEGIN (0x1ULL - ((OVMF_SIZE + > OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET)) > -#define OVMF_END(OVMF_BEGIN + OVMF_SIZE) > #define LOWCHUNK_BEGIN 0x000F > #define LOWCHUNK_SIZE 0x0001 > #define LOWCHUNK_MAXOFFSET 0x > -#define LOWCHUNK_END(OVMF_BEGIN + OVMF_SIZE) > #define OVMF_INFO_PHYSICAL_ADDRESS 0x1000 > > extern unsigned char dsdt_anycpu[]; > @@ -96,12 +88,16 @@ static void ovmf_finish_bios_info(void) > static void ovmf_load(const struct bios_config *config, > void *bios_addr, uint32_t bios_length) > { > +#define OVMF_MAXOFFSET 0x000FULL > +#define OVMF_BEGIN (0x1ULL - ((bios_length + OVMF_MAXOFFSET) & > ~OVMF_MAXOFFSET)) > +#define OVMF_END(OVMF_BEGIN + bios_length) Would be far better converted to proper (possibly const) local variables IMHO. But if you don't want to do that (and can give a good reason not to) then you should at least #undef them when the things they refer to go out of scope. > xen_pfn_t mfn; > uint64_t addr = OVMF_BEGIN; > +unsigned int dest = OVMF_BEGIN; Two things both assigned OVMF_BEGIN, but with very differently sized types. One of them is suspicious (IMHO the new one) > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v2 11/16] hvmloader: No BIOS ROM image allowed to be compiled in
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: This might invalidate some of my earlier comments about reusing bios_addr, in which case that other commit ought to make reference to the eventual goal. But... > @@ -432,9 +424,9 @@ int main(void) > if ( SCRATCH_PHYSICAL_ADDRESS != scratch_start ) > printf(" %05x-%05lx: Scratch space\n", > SCRATCH_PHYSICAL_ADDRESS, scratch_start); > -printf(" %05x-%05x: Main BIOS\n", > - bios->bios_address, > - bios->bios_address + bios->image_size - 1); > +/* printf(" %05x-%05x: Main BIOS\n", */ > + /* bios->bios_address, */ > + /* bios->bios_address + bios->image_size - 1); */ A way should be found to keep this useful debug output, which may involve having the per-bios code propagate what they have done, or logging it themselves etc. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Hi, On 03/11/15 14:34, Ian Campbell wrote: > On Tue, 2015-11-03 at 14:01 +, Julien Grall wrote: >> On 03/11/15 12:35, Ian Campbell wrote: >>> On Mon, 2015-11-02 at 15:55 +, Stefano Stabellini wrote: > +/* > + * Macro to set a guest pointer in the handle. > + * > + * Note that it's not possible to implement safely a macro to > retrieve the > + * handle unless the guest is built with strict aliasing > disabling. > + * Hence, we don't provide a such macro in the public headers. > + */ > +#define set_xen_guest_handle_raw(hnd, > val) \ > +do > {\ > +/* Check if the handle is 64-bit (i.e 8-byte) > */\ > +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); > });\ > +/* Check if the type of val is compatible with the handle > */\ > +(void) sizeof((val) != > (hnd).p);\ > +(hnd).q = > (uint64_t)(uintptr_t)(val); \ > } while ( 0 ) Honestly I would be OK with having a typeof in the public headers to avoid this code, which is much harder to follow. >>> >>> I suppose your objection is to two things: >>> >>> +/* Check if the handle is 64-bit (i.e 8-byte) >>> */\ >>> +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); >>> });\ >>> >>> and >>> >>> +/* Check if the type of val is compatible with the handle >>> */\ >>> +(void) sizeof((val) != >>> (hnd).p);\ >>> >>> The first is really just an open coding of BUILD_BUG_ON, I suppose for >>> some >>> reason BUILD_BUG_ON cannot just be used here (I assume because this is >>> itself a macro). >>> >>> Personally I think a comment referring back to BUILD_BUG_ON e.g.: >>> /* BUILD_BUG_ON(sizeof(hnd) != 8); Cannot use real B_B_O in a macro >>> */ >>> would be sufficient. >> >> You could use BUILD_BUG_ON in a macro, but this is part of the public >> interface and I don't think we should require the guest/toolstack to >> provide a BUILD_BUG_ON macro. > > Ah, yes, that makes more sense. > > we could: > #ifdef(__XEN__) > #define XEN_BUILD_BUG_ON(x) BUILD_BUG_ON(x) > #elif !defined(XEN_BUILD_BUG_ON) > #define XEN_BUILD_BUG_ON(x) > #endif > and using XEN_BUILD_BUG_ON in the macro > > So, __XEN__ builds get the check and users can opt in by providing > XEN_BUILD_BUG_ON if they want. If they don't then they don't get the check. I wouldn't let the user the choice to disable build-time check. There is no harm to open-code them as I did today and avoid possible issue in the code later. > Or maybe we could just omit this from the public API by one or both of a) > adding an explicit 8 byte type to the union purely to force the size and/or This is already done in the current version: typedef union { type *p; uint64_aligned_t q; } \ __guest_handle_64_ ## name; However I don't see how this ensure that the caller of set_xen_raw_guest_handle will effectively hnd as a pointer to an 8-byte placeholder. > b) adding something to the non-public Xen side which consumes such > structures. After all any struct in the Xen API ought to be used somewhere > I suppose. > [...] >>> Also given this new usage I think it >>> would be worth renaming p and q to something less opaque, value and >>> type_check or something would be fine IMHO. >> >> I guess you mean replacing "p" by "type_check" and "q" by value. If so, >> I disagree with that because we have to use the actual "p" within Xen in >> order to get the guest pointer and have type safety. It would be odd to >> return "type_check". > > Ah, yes. I was thinking the read was of q not p. Having the Xen side use > type check would indeed be odd (even considering it is indirect via the > access stuff most of the time). > > Perhaps the uint could be "bits" or "raw" and the actual pointer ptr, or > tbh "p" probably suffices in this case. > >> I didn't change the names because I wasn't able to find better ones that >> could fit for the 2 usages. >> >>> Why don't we do something like the following: >>> >>> Apart from Jan's comment about __asm__ and a question I have about >>> whether >>> it isn't even needed, how certain are you that this doesn't violate any >>> of >>> the C aliasing rules etc? >>> >>> BTW, Julien, I think it would be fine to also make this macro differ >>> for >>> arm32 and arm64, since the arm64 variant would then surely be simpler >>> and >>> the arm32 one might (or might not) be. >> >> I agree that the macro could be simpler (only a single line). However I >> didn't want to differ because there is no advantage other than have a >> good looking code for the arm64 bits. It's just add extra code to take >> care. > > Would the arm32 case be simplified since it woul
Re: [Xen-devel] [RFC PATCH v2 12/16] hvmloader: Load ACPI tables from hvm_start_info module
On Mon, 2015-10-26 at 16:03 +, Anthony PERARD wrote: > ... and use it with both SeaBIOS and OVMF. > > This may change the behavior of guest using OVMF as the ACPI tables > currently loaded are the one for qemu-xen-traditionnal. "traditional". I'm a bit surprised using the trad ones works, given OVMF is always associated with qemu-xen. I'm inclined to suggest that this bugfix should be split out and committed sooner rather than later, independently of this series. Apart from cleanly separating out a potentially interesting change it also gives us a longer runway for testing it doesn't knacker anything. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xl: don't print out uninitialised rc
On Wed, 2015-11-04 at 11:15 +, Wei Liu wrote: > In 5b725e56 (xl: improve return and exit codes of vcpu related > functions), the return value of libxl_cpu_bitmap_alloc was not stored in > rc anymore. Yet the subsequent fprintf still used that. > > While it is trivial to reinstate the original implementation, xl's > caller has no idea what a libxl error code is so printing out rc won't > be that useful. I don't buy this I'm afraid. xl prints the libxl error code in lots of places and it is indeed useful for a person (e.g. a developer reading a bug report) since they can go and look the number up. I'm not sure where "xl's caller" comes into it, since this is only about printing and not returning. > Don't print out rc in that case. > > Signed-off-by: Wei Liu > --- > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Dario Faggioli > Cc: Harmandeep Kaur > --- > tools/libxl/xl_cmdimpl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 2756d2f..6056344 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char* > nr_vcpus, int check_host) > return 1; > } > if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > -fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc); > +fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); > return 1; > } > for (i = 0; i < max_vcpus; i++) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xl: don't print out uninitialised rc
On Wed, Nov 04, 2015 at 11:23:29AM +, Ian Campbell wrote: > On Wed, 2015-11-04 at 11:15 +, Wei Liu wrote: > > In 5b725e56 (xl: improve return and exit codes of vcpu related > > functions), the return value of libxl_cpu_bitmap_alloc was not stored in > > rc anymore. Yet the subsequent fprintf still used that. > > > > While it is trivial to reinstate the original implementation, xl's > > caller has no idea what a libxl error code is so printing out rc won't > > be that useful. > > I don't buy this I'm afraid. xl prints the libxl error code in lots of > places and it is indeed useful for a person (e.g. a developer reading a bug > report) since they can go and look the number up. > > I'm not sure where "xl's caller" comes into it, since this is only about > printing and not returning. > So the original implementation it is -- I don't feel strong enough to argue for one way or another. Wei. > > Don't print out rc in that case. > > > > Signed-off-by: Wei Liu > > --- > > Cc: Ian Campbell > > Cc: Ian Jackson > > Cc: Dario Faggioli > > Cc: Harmandeep Kaur > > --- > > tools/libxl/xl_cmdimpl.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 2756d2f..6056344 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char* > > nr_vcpus, int check_host) > > return 1; > > } > > if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > > -fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc); > > +fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); > > return 1; > > } > > for (i = 0; i < max_vcpus; i++) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 2015-11-04 at 11:17 +, Julien Grall wrote: > Hi, > > > On 03/11/15 14:34, Ian Campbell wrote: > > On Tue, 2015-11-03 at 14:01 +, Julien Grall wrote: > > > On 03/11/15 12:35, Ian Campbell wrote: > > > > On Mon, 2015-11-02 at 15:55 +, Stefano Stabellini wrote: > > > > > > > > > > > +/* > > > > > > + * Macro to set a guest pointer in the handle. > > > > > > + * > > > > > > + * Note that it's not possible to implement safely a macro to > > > > > > retrieve the > > > > > > + * handle unless the guest is built with strict aliasing > > > > > > disabling. > > > > > > + * Hence, we don't provide a such macro in the public headers. > > > > > > + */ > > > > > > +#define set_xen_guest_handle_raw(hnd, > > > > > > val) \ > > > > > > +do > > > > > > { > > > > > > \ > > > > > > +/* Check if the handle is 64-bit (i.e 8-byte) > > > > > > */\ > > > > > > +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); > > > > > > });\ > > > > > > +/* Check if the type of val is compatible with the > > > > > > handle > > > > > > */\ > > > > > > +(void) sizeof((val) != > > > > > > (hnd).p);\ > > > > > > +(hnd).q = > > > > > > (uint64_t)(uintptr_t)(val); \ > > > > > > } while ( 0 ) > > > > > > > > > > Honestly I would be OK with having a typeof in the public headers > > > > > to > > > > > avoid this code, which is much harder to follow. > > > > > > > > I suppose your objection is to two things: > > > > > > > > +/* Check if the handle is 64-bit (i.e 8-byte) > > > > */\ > > > > +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); > > > > });\ > > > > > > > > and > > > > > > > > +/* Check if the type of val is compatible with the handle > > > > */\ > > > > +(void) sizeof((val) != > > > > (hnd).p);\ > > > > > > > > The first is really just an open coding of BUILD_BUG_ON, I suppose > > > > for > > > > some > > > > reason BUILD_BUG_ON cannot just be used here (I assume because this > > > > is > > > > itself a macro). > > > > > > > > Personally I think a comment referring back to BUILD_BUG_ON e.g.: > > > > /* BUILD_BUG_ON(sizeof(hnd) != 8); Cannot use real B_B_O in a > > > > macro > > > > */ > > > > would be sufficient. > > > > > > You could use BUILD_BUG_ON in a macro, but this is part of the public > > > interface and I don't think we should require the guest/toolstack to > > > provide a BUILD_BUG_ON macro. > > > > Ah, yes, that makes more sense. > > > > we could: > > #ifdef(__XEN__) > > #define XEN_BUILD_BUG_ON(x) BUILD_BUG_ON(x) > > #elif !defined(XEN_BUILD_BUG_ON) > > #define XEN_BUILD_BUG_ON(x) > > #endif > > and using XEN_BUILD_BUG_ON in the macro > > > > So, __XEN__ builds get the check and users can opt in by providing > > XEN_BUILD_BUG_ON if they want. If they don't then they don't get the > > check. > > I wouldn't let the user the choice to disable build-time check. Up to you. Note that "the user" here would potentially include xen.git/tools, and I would expect them to want to define it. Also... > There is > no harm to open-code them as I did today and avoid possible issue in the > code later. ... there is always a downside to open coding. If you don't want to expose the ability to define a BUG_ON to the application then just drop that #elif from the chain. > > Or maybe we could just omit this from the public API by one or both of > > a) > > adding an explicit 8 byte type to the union purely to force the size > > and/or > > This is already done in the current version: > > typedef union { type *p; uint64_aligned_t q; } \ > __guest_handle_64_ ## name; > > However I don't see how this ensure that the caller of > set_xen_raw_guest_handle will effectively hnd as a pointer to an 8-byte > placeholder. Not sure I follow. If hnd isn't a suitable struct xen guest handle then other things will fail. If a user is passing a non struct xen_guest_handle to this which happens to contain the same fields then more fool them, and if it happens to be 8 bytes anyway your check won't catch that. > > Would the arm32 case be simplified since it would be a regular struct > > with > > 4 bytes padding and 4 bytes pointer, which can both be easily set in > > the > > macro. If that works then the simplicity would seem to justify having > > the > > two subarches use different cases here. > > One of my first idea was to use a structure with 2 fields (4 bytes > padding + 4 bytes pointer) on arm32. But it's not possible to assign in > a single expression all the fields as we lack of the type within the > macro: > > test.c:14:9: error: expected expression before ‘{’ token > b = { .a = 0, .b = 0 }; Right, that's a shame. > > > This looses out on the arm32 hypervisor sani
Re: [Xen-devel] [VOTE] Release cycle scheme
On Mon, 2015-11-02 at 13:47 +, Wei Liu wrote: > > So I propose we use the following scheme: > > - 6 months release cycle from unstable branch. > - 4 months development. > - 2 months freeze. +1 > - Eat into next cycle if doesn't release on time. +2 > - Fixed cut-off date: the Fridays of the week in which the last day of > March and September falls. +2 > - No more freeze exception, but heads-up mails about freeze will be > sent a few weeks before hand. +2 > - Stable branch maintained for 18 months full support plus 18 months > security support. No mixed maintainership for stable trees. 0. > Please vote to ack or nack this proposal. Overall +1. I just wanted to highlight the three aspects which really matter IMHO (although I agree with George that the "Fixed..." and "No more..." are really up to the RM to decide). Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xl: initialise rc before using it in vcpuset
In 5b725e56 (xl: improve return and exit codes of vcpu related functions), the return value of libxl_cpu_bitmap_alloc was not stored in rc anymore. Yet the subsequent fprintf still used that. Reinstate the original implementation, that is, to store return value of libxl_cpu_bitmap_alloc in rc before using rc. Signed-off-by: Wei Liu --- Cc: Ian Campbell Cc: Ian Jackson Cc: Dario Faggioli Cc: Harmandeep Kaur Cc: Andrew Cooper v2: print rc. --- tools/libxl/xl_cmdimpl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 2756d2f..9b6b42c 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -5473,7 +5473,8 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) if (rc) return 1; } -if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { +rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); +if (rc) { fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc); return 1; } -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 63584: tolerable all pass - PUSHED
flight 63584 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/63584/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass version targeted for testing: xen 700772bc13a3d4e7a61cea166893c0f53b251f08 baseline version: xen 32b31c17da1ba0da970cd182a8865ac20fabd0fa Last test of basis63544 2015-11-03 19:01:38 Z0 days Testing same since63584 2015-11-04 10:01:27 Z0 days1 attempts People who touched revisions under test: Harmandeep Kaur Ian Campbell Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=700772bc13a3d4e7a61cea166893c0f53b251f08 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 700772bc13a3d4e7a61cea166893c0f53b251f08 + branch=xen-unstable-smoke + revision=700772bc13a3d4e7a61cea166893c0f53b251f08 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-unstable + '[' x700772bc13a3d4e7a61cea166893c0f53b251f08 = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print $c{"GitCacheProxy"} or die $!; ' +++ local cache=git://cache:9419/ +++ '[' xgit://cache:9419/ '!=' x ']' +++ echo 'git:
Re: [Xen-devel] [PATCH v2] xl: initialise rc before using it in vcpuset
On Wed, 2015-11-04 at 11:32 +, Wei Liu wrote: > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 2756d2f..9b6b42c 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5473,7 +5473,8 @@ static int vcpuset(uint32_t domid, const char* > nr_vcpus, int check_host) > if (rc) > return 1; > } > -if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > +rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > +if (rc) { > fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", > rc); > return 1; > } > Don't we also need to initialise rc to 0? It seems to me that it may be used uninitialised also inside of if(check_host)... Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xl: initialise rc before using it in vcpuset
On Wed, 2015-11-04 at 11:32 +, Wei Liu wrote: > In 5b725e56 (xl: improve return and exit codes of vcpu related > functions), the return value of libxl_cpu_bitmap_alloc was not stored in > rc anymore. Yet the subsequent fprintf still used that. > > Reinstate the original implementation, that is, to store return value of > libxl_cpu_bitmap_alloc in rc before using rc. > > Signed-off-by: Wei Liu Acked + applied. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().
In commit: d37d63d symbols: prefix static symbols with their source file names An unchecked fgets was added. This causes a compile error: symbols.c: In function 'read_symbol': symbols.c:181:3: error: ignoring return value of 'fgets', declared with attribute warn_unused_result [-Werror=unused-result] fgets(str, 500, in); /* discard rest of line */ ^ Paper over the warning like in the other similar fgets-on-error-path earlier in the same file. Cc: Jan Beulich Signed-off-by: Riku Voipio --- xen/tools/symbols.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c index dbf6a1a..7e75be2 100644 --- a/xen/tools/symbols.c +++ b/xen/tools/symbols.c @@ -178,8 +178,8 @@ static int read_symbol(FILE *in, struct sym_entry *s) skip_tail: if (input_format == fmt_sysv) - fgets(str, 500, in); /* discard rest of line */ - + if (fgets(str, 500, in) == NULL) /* discard rest of line */ + return -1; /* must check fgets result */ return rc; } -- 2.6.2 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On 04/11/15 11:27, Ian Campbell wrote: > On Wed, 2015-11-04 at 11:17 +, Julien Grall wrote: >>> >>> we could: >>> #ifdef(__XEN__) >>> #define XEN_BUILD_BUG_ON(x) BUILD_BUG_ON(x) >>> #elif !defined(XEN_BUILD_BUG_ON) >>> #define XEN_BUILD_BUG_ON(x) >>> #endif >>> and using XEN_BUILD_BUG_ON in the macro >>> >>> So, __XEN__ builds get the check and users can opt in by providing >>> XEN_BUILD_BUG_ON if they want. If they don't then they don't get the >>> check. >> >> I wouldn't let the user the choice to disable build-time check. > > Up to you. Note that "the user" here would potentially include > xen.git/tools, and I would expect them to want to define it. > > Also... > >> There is >> no harm to open-code them as I did today and avoid possible issue in the >> code later. > > ... there is always a downside to open coding. If you don't want to expose > the ability to define a BUG_ON to the application then just drop that #elif > from the chain. Good point. I will give a look. > >>> Or maybe we could just omit this from the public API by one or both of >>> a) >>> adding an explicit 8 byte type to the union purely to force the size >>> and/or >> >> This is already done in the current version: >> >> typedef union { type *p; uint64_aligned_t q; } \ >> __guest_handle_64_ ## name; >> >> However I don't see how this ensure that the caller of >> set_xen_raw_guest_handle will effectively hnd as a pointer to an 8-byte >> placeholder. > > Not sure I follow. If hnd isn't a suitable struct xen guest handle then > other things will fail. If a user is passing a non struct xen_guest_handle > to this which happens to contain the same fields then more fool them, and > if it happens to be 8 bytes anyway your check won't catch that. With the 2 checks in set_xen_raw_guest_handle we catch most of the problem. They both ensure that the handle is 8-byte and the pointer is valid. However we don't check that the padding is at the beginning of the structure. It's better than what we have today as we don't even check that the handle is 8-byte. [...] >>> This looses out on the arm32 hypervisor sanity checking that the padding >>> bytes are 0 (as required by the ABI) but TBH I haven't checked that the >>> current version has that property either. >> >> It's done during the assignation by the compiler: >> >> (hnd).q = (uint64_t)(uintptr_t)(val); > > I meant on the reading side. It's the responsibility of the caller to zero the padding. There is nothing to do on the reading side, the hypervisor will use "p" which will be the size of the natural pointer. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xl: initialise rc before using it in vcpuset
On Wed, 2015-11-04 at 12:38 +0100, Dario Faggioli wrote: > On Wed, 2015-11-04 at 11:32 +, Wei Liu wrote: > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 2756d2f..9b6b42c 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -5473,7 +5473,8 @@ static int vcpuset(uint32_t domid, const char* > > nr_vcpus, int check_host) > > if (rc) > > return 1; > > } > > -if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > > +rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > > +if (rc) { > > fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", > > rc); > > return 1; > > } > > > Don't we also need to initialise rc to 0? It seems to me that it may be > used uninitialised also inside of if(check_host)... Looks like it, a separate issue though. Don't initialise rc to 0, add a suitable else though. > > Regards, > Dario > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xl: initialise rc before using it in vcpuset
On Wed, 2015-11-04 at 11:47 +, Ian Campbell wrote: > On Wed, 2015-11-04 at 12:38 +0100, Dario Faggioli wrote: > > On Wed, 2015-11-04 at 11:32 +, Wei Liu wrote: > > > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > > index 2756d2f..9b6b42c 100644 > > > --- a/tools/libxl/xl_cmdimpl.c > > > +++ b/tools/libxl/xl_cmdimpl.c > > > @@ -5473,7 +5473,8 @@ static int vcpuset(uint32_t domid, const > > > char* > > > nr_vcpus, int check_host) > > > if (rc) > > > return 1; > > > } > > > -if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > > > +rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > > > +if (rc) { > > > fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: > > > %d\n", > > > rc); > > > return 1; > > > } > > > > > Don't we also need to initialise rc to 0? It seems to me that it > > may be > > used uninitialised also inside of if(check_host)... > > Looks like it, a separate issue though. > The compiler is not spotting it again, though (just as a side note). > Don't initialise rc to 0, add a suitable else though. > Yeah, sure, I figured that out after having sent the mail. I'll send a patch. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Linux 4.4 MW: Boot under Xen fails with CONFIG_DEBUG_WX enabled: RIP: ptdump_walk_pgd_level_core
Hi All, I just tried to boot with the current linus mergewindow tree under Xen. It fails with a kernel panic at boot with the new "CONFIG_DEBUG_WX" option enabled. Disabling it makes the kernel boot fine. The splat: [ 18.424241] Freeing unused kernel memory: 1104K (822fc000 - 8241) [ 18.430314] Write protecting the kernel read-only data: 18432k [ 18.441054] Freeing unused kernel memory: 1144K (880001ae2000 - 880001c0) [ 18.447966] Freeing unused kernel memory: 1560K (88000207a000 - 88000220) [ 18.453947] BUG: unable to handle kernel paging request at 88055c883000 [ 18.459943] IP: [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.465847] PGD 2212067 PUD 0 [ 18.471564] Oops: [#1] SMP [ 18.477248] Modules linked in: [ 18.482918] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.3.0-mw-20151104-linus-doflr+ #1 [ 18.488804] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010 [ 18.494778] task: 880059b9 ti: 880059b98000 task.ti: 880059b98000 [ 18.500852] RIP: e030:[] [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.507102] RSP: e02b:880059b9be48 EFLAGS: 00010296 [ 18.513351] RAX: 88055c883000 RBX: 81ae2000 RCX: 8800 [ 18.519733] RDX: 0067 RSI: 880059b9be98 RDI: 88001000 [ 18.526129] RBP: 880059b9bf00 R08: R09: [ 18.532522] R10: 88005fd0e790 R11: 0001 R12: 88008000 [ 18.538891] R13: cfff R14: 880059b9be98 R15: [ 18.545247] FS: () GS:88005f68() knlGS: [ 18.551708] CS: e033 DS: ES: CR0: 8005003b [ 18.558153] CR2: 88055c883000 CR3: 02211000 CR4: 0660 [ 18.564686] Stack: [ 18.571106] 000159b9be50 82211000 88055c884000 0800 [ 18.577704] 8000 88055c883000 0007 88005fd0e790 [ 18.584291] 880059b9bed8 81156ace 0001 [ 18.590916] Call Trace: [ 18.597458] [] ? free_reserved_area+0x11e/0x120 [ 18.604180] [] ptdump_walk_pgd_level_checkwx+0x12/0x20 [ 18.611014] [] mark_rodata_ro+0xe9/0xf0 [ 18.617819] [] ? rest_init+0x80/0x80 [ 18.624512] [] kernel_init+0x18/0xe0 [ 18.631095] [] ret_from_fork+0x3f/0x70 [ 18.637650] [] ? rest_init+0x80/0x80 [ 18.644178] Code: 70 ff ff ff 48 3b 85 58 ff ff ff 0f 84 c0 fe ff ff 48 8b 85 68 ff ff ff 48 c1 e0 10 48 c1 f8 10 48 89 45 b0 48 8b 85 70 ff ff ff <48> 8b 38 48 85 ff 0f 85 4e ff ff ff b9 02 00 00 00 31 d2 4c 89 [ 18.658246] RIP [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.665211] RSP [ 18.672073] CR2: 88055c883000 [ 18.678852] ---[ end trace d84e34461c40637a ]--- [ 18.685641] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 [ 18.685641] [ 18.699520] Kernel Offset: disable ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xl: avoid (another) uninitialised use of rc in vcpuset()
Rearange the case when we check the new number of vCPUs against the number of host pCPUs not to use rc for internal error reporting. In fact: - rc was at risk of being used uninitialised; - rc should only be used for holding libxl error codes. Signed-off-by: Dario Faggioli --- Cc: Ian Campbell Cc: Ian Jackson Cc: Wei Liu Cc: Harmandeep Kaur Cc: Andrew Cooper --- tools/libxl/xl_cmdimpl.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 9b6b42c..78048a1 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -5457,21 +5457,21 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) * by the host's amount of pCPUs. */ if (check_host) { -unsigned int host_cpu = libxl_get_max_cpus(ctx); +unsigned int online_vcpus, host_cpu = libxl_get_max_cpus(ctx); libxl_dominfo dominfo; if (libxl_domain_info(ctx, &dominfo, domid)) return 1; -if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) { +online_vcpus = dominfo.vcpu_online; +libxl_dominfo_dispose(&dominfo); + +if (max_vcpus > online_vcpus && max_vcpus > host_cpu) { fprintf(stderr, "You are overcommmitting! You have %d physical" \ " CPUs and want %d vCPUs! Aborting, use --ignore-host to" \ " continue\n", host_cpu, max_vcpus); -rc = 1; -} -libxl_dominfo_dispose(&dominfo); -if (rc) return 1; +} } rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); if (rc) { ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/6] gitignore: Ignore *.orig, *.rej and *.swp files
Le 12 août 2015 11:04 AM, "Ian Campbell" a écrit : > > > (Having written the below I see too late that this is a grub patch not a > Xen one, a tag in the subject for such cross posted patches would be useful > please. Anyway, my opinion counts for very little in this context but I > leave it below since already I wrote it. I notice that xen.git#.gitignore > _does_ list *.rej, which I think is wrong...) > > On Mon, 2015-07-20 at 16:35 +0200, Daniel Kiper wrote: > > Signed-off-by: Daniel Kiper > > At least *.rej and perhaps *.orig are indicative of a failed patch > application, I think I want them to appear in "git status". > > By way of comparison Linux's .gitignore includes *.orig but not *.rej and > Qemu's includes neither. > > So nack to the addition of *.rej from me. I'm more or less ambivalent about > *.orig. > I have to agree. You should clean up *.rej *.orig after fixing conflicts > > --- > > .gitignore |3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index 18ab8e8..6d25d39 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -147,6 +147,7 @@ mod-*.c > > missing > > netboot_test > > *.o > > +*.orig > > *.a > > ohci_test > > partmap_test > > @@ -160,9 +161,11 @@ po/stamp-po > > printf_test > > priority_queue_unit_test > > pseries_test > > +*.rej > > stamp-h > > stamp-h1 > > stamp-h.in > > +*.swp > > symlist.c > > symlist.h > > trigtables.c ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [VOTE] Release cycle scheme
Wei Liu writes ("[VOTE] Release cycle scheme"): > There are comments made by individuals that couldn't be clearly > represent in tally. The most acceptable option to stable tree > maintainers is #1. > > So I propose we use the following scheme: +1 Following Ian Campbell's lead, I want to comment on the aspects of the proposal: > - 6 months release cycle from unstable branch. +2 > - 4 months development. > - 2 months freeze. > - Eat into next cycle if doesn't release on time. +2 > - Fixed cut-off date: the Fridays of the week in which the last day of > March and September falls. +1 You should perhaps adjust the wording here because the ends of a "week" are slightly fuzzy. How about "the last Friday in March or September" ? (With my own definition of "week", which has weeks running Sunday to Saturday, I think what you wrote means the same as "the Friday after the last Sunday in March or September" - which is rather complex!) > - No more freeze exception, but heads-up mails about freeze will be > sent a few weeks before hand. 0 > - Stable branch maintained for 18 months full support plus 18 months > security support. No mixed maintainership for stable trees. +1 Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 63530: all pass - PUSHED
flight 63530 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/63530/ Perfect :-) All tests in this flight passed version targeted for testing: ovmf fad21b7c57336bbf0ae363d56e35cbccca67ff3b baseline version: ovmf 8aa6ebe83f91fcd5e1a67d316e444f4ac8b1319d Last test of basis63472 2015-11-02 02:10:32 Z2 days Testing same since63530 2015-11-03 08:45:04 Z1 days1 attempts People who touched revisions under test: Cinnamon Shia Jeff Fan Michael Kinney Ruiyu Ni Samer El-Haj-Mahmoud Sunny Wang 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 : + branch=ovmf + revision=fad21b7c57336bbf0ae363d56e35cbccca67ff3b + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf fad21b7c57336bbf0ae363d56e35cbccca67ff3b + branch=ovmf + revision=fad21b7c57336bbf0ae363d56e35cbccca67ff3b + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=ovmf + xenbranch=xen-unstable + '[' xovmf = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-unstable + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable + prevxenbranch=xen-4.6-testing + '[' xfad21b7c57336bbf0ae363d56e35cbccca67ff3b = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print $c{"GitCacheProxy"} or die $!; ' +++ local cache=git://cache:9419/ +++ '[' xgit://cache:9419/ '!=' x ']' +++ echo 'git://cache:9419/https://github.com/
[Xen-devel] [linux-3.10 test] 63529: regressions - FAIL
flight 63529 linux-3.10 real [real] http://logs.test-lab.xenproject.org/osstest/logs/63529/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-pvops 5 kernel-build fail REGR. vs. 62642 Tests which are failing intermittently (not blocking): test-amd64-amd64-libvirt-xsm 18 guest-start/debian.repeat fail in 63391 pass in 63529 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate fail pass in 63391 test-amd64-amd64-rumpuserxen-amd64 15 rumpuserxen-demo-xenstorels/xenstorels.repeat fail pass in 63391 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail in 63391 blocked in 62642 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 62642 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 62642 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass version targeted for testing: linuxd17332ebfb5f2010ae5d3332a52df361f28ae4a8 baseline version: linuxf5552cd830e58c46dffae3617b3ce0c839771981 Last test of basis62642 2015-10-03 17:59:45 Z 31 days Failing since 63224 2015-10-22 22:20:05 Z 12 days 10 attempts Testing same since63332 2015-10-27 12:23:40 Z8 days6 attempts People who touched revisions under test: "Eric W. Biederman" Aaron Conole Adam Radford Al Viro Alexander Couzens Alexey Klimov Andi Kleen Andreas Schwab Andrew Morton Ard Biesheuvel Arnaldo Carvalho de Melo Ben Hutchings Charles Keepax Christoph Biedl Christoph Hellwig cov...@ccs.covici.com Daniel Vetter Daniel Vetter Dave Kleikamp David S. Miller David Vrabel David Woodhouse David Woodhouse Ding Tianhong dingtianhong Dirk Mueller Dirk Müller Doug Ledford Eric Dumazet Eric W. Biederman Geert Uytterhoeven Greg Kroah-Hartman Guenter Roeck Guillaume Nault H. Peter Anvin Herbert Xu Ian Abbott Ilya Dryomov Ingo Molnar James Bottomley James Chapman James Hogan Jan Kara Jann Horn Jarkko Nikula Jeff Mahoney Jiri Slaby Joe Perches Joe Stringer Joe Thornber Johan Hovold John Covici Julian Anastasov Kees Cook Linus Torvalds Liu.Zhao Mark Brown Mark Salyzyn Mathias Nyman Mel Gorman Michael Ellerman Michal Hocko Michel Stam Mike Marciniszyn Mike Snitzer Mikulas Patocka Namhyung Kim NeilBrown Nicolas Pitre Nikolay Aleksandrov Oleksii Berezhniak Pablo Neira Ayuso Paolo Bonzini Paul Bolle Paul Mackerras Paul Mackerras Pravin B Shelar Ralf Baechle Reyad Attiyat Richard Weinberger Riku Voipio Riley Andrews Robert Jarzmik Roger Quadros Roland Dreier Russell King Samuel Thibault Shaohua Li Sheng Yong shengyong Simon Horman Stephen Smalley Steve French Steve French Takashi Iwai Tan, Jui Nee Tejun Heo Thomas Gleixner Tom Herbert Tóth Attila Vincent Palatin Vitaly Kuznetsov Will Deacon Wolfram Sang Wolfram Sang Yao-Wen Mao Yitian Bu Yitian Bu Zhang Zhen jobs: build-amd64-xsm pass build-armhf-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-pvopsfail build-
[Xen-devel] [xen-unstable-smoke test] 63594: tolerable all pass - PUSHED
flight 63594 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/63594/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass version targeted for testing: xen 6919a7dde48bf1a9314c328bfb93a8a2f741bb80 baseline version: xen 700772bc13a3d4e7a61cea166893c0f53b251f08 Last test of basis63584 2015-11-04 10:01:27 Z0 days Testing same since63594 2015-11-04 12:00:59 Z0 days1 attempts People who touched revisions under test: Ian Campbell Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=6919a7dde48bf1a9314c328bfb93a8a2f741bb80 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 6919a7dde48bf1a9314c328bfb93a8a2f741bb80 + branch=xen-unstable-smoke + revision=6919a7dde48bf1a9314c328bfb93a8a2f741bb80 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-unstable + '[' x6919a7dde48bf1a9314c328bfb93a8a2f741bb80 = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print $c{"GitCacheProxy"} or die $!; ' +++ local cache=git://cache:9419/ +++ '[' xgit://cache:9419/ '!=' x ']' +++ echo 'git://cache:9419/https:
Re: [Xen-devel] [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().
>>> On 04.11.15 at 12:39, wrote: > In commit: > > d37d63d symbols: prefix static symbols with their source file names > > An unchecked fgets was added. This causes a compile error: > > symbols.c: In function 'read_symbol': > symbols.c:181:3: error: ignoring return value of 'fgets', declared with > attribute warn_unused_result [-Werror=unused-result] >fgets(str, 500, in); /* discard rest of line */ >^ > > Paper over the warning like in the other similar fgets-on-error-path > earlier in the same file. But the two cases are dissimilar - the original one skips a line the format of which is not recognized, while here you may be converting success into an error. (I did notice the comment on the earlier fgets(), but since so far I didn't get any compiler warning on any system I built this on, I assumed we'd be fine without the check, since if we need the check, then it will end up even more clumsy than the other one.) > --- a/xen/tools/symbols.c > +++ b/xen/tools/symbols.c > @@ -178,8 +178,8 @@ static int read_symbol(FILE *in, struct sym_entry *s) > > skip_tail: > if (input_format == fmt_sysv) > - fgets(str, 500, in); /* discard rest of line */ > - > + if (fgets(str, 500, in) == NULL) /* discard rest of line */ > + return -1; /* must check fgets result */ As to formal things - two such consecutive if()-s should be combined. Since we really want to ignore the return value here, perhaps just cast the function result to void? (I admit that I don't recall whether this would take care of that compiler warning.) Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS
On Mon, 2015-11-02 at 09:45 -0500, Meng Xu wrote: > > > I guess maybe you forgot to change it in this commit but change > > > it > > > the > > > following commit? > > > > > No, this is one of the few thing that changed between v2 and v3. > > > > Regards, > > Dario > > Thanks for the explanation! Then the patch looks good to me, at least > for RTDS scheduler. :-) > Thanks for looking at the patch. Just FTR (and for next time :-D), is the above something that can be interpreted as a 'Reviewed-by: Meng Xu ' ? If no (e.g., because you haven't looking thoroughly enough to feel confident to express it), then fine, I was just asking. If yes, I encourage you to say it explicitly, to avoid errors and misjudgements. If you 'only' looked at the patch with the RTDS scheduler in mind, that is fine too. You can say something like "As far as the RTDS scheduler is concerned: Reviewed-by: Meng Xu ". Other reviewers and committers will take this into account and properly weight it. Every akc/review is important, and, if you took the time to look at a patch, why don't say it in the proper way? :-) I'm about to send v4 of this series. Feel free (only if you want, of course!), to chime in in that thread. Thanks again and Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().
On Wed, 2015-11-04 at 13:39 +0200, Riku Voipio wrote: Not to do with this patch (and this suggestion wouldn't have helped in this case) but IIRC Linaro tests the xen.git#staging branch. We recently introduced a new "smoke test" phase which is a quick[0] test which runs on staging before the regular big test runs to check for obvious errors, build problems, simple guest booting etc. The sequence of tests on xen's development branch is now: * A committer pushes to xen.git#staging * osstest runs smoke tests on staging and propagates passes to xen.git#smoked * osstest runs full suite of tests tests on smoked and propagates passes to xen.git#master I mention this because perhaps Linaro would want to switch to running their tests on #smoked instead of #staging, and avoid the cases where things are completely knackered? In this case the compiler which osstest uses apparently doesn't warn about fgets() not being checked, so it wouldn't actually have helped... Ian. [0] ~2 hours turnaround, compared with a day or more for regular test flights. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 2015-11-04 at 11:40 +, Julien Grall wrote: > > Not sure I follow. If hnd isn't a suitable struct xen guest handle then > > other things will fail. If a user is passing a non struct > > xen_guest_handle > > to this which happens to contain the same fields then more fool them, > > and > > if it happens to be 8 bytes anyway your check won't catch that. > > With the 2 checks in set_xen_raw_guest_handle we catch most of the > problem. They both ensure that the handle is 8-byte and the pointer is > valid. However we don't check that the padding is at the beginning of > the structure. > > It's better than what we have today as we don't even check that the > handle is 8-byte. I'm not sure I'm all that worried about callers constructing their own guest handle structs and getting it wrong. > [...] > > > > > This looses out on the arm32 hypervisor sanity checking that the > > > > padding > > > > bytes are 0 (as required by the ABI) but TBH I haven't checked that > > > > the > > > > current version has that property either. > > > > > > It's done during the assignation by the compiler: > > > > > > (hnd).q = (uint64_t)(uintptr_t)(val); > > > > I meant on the reading side. > > It's the responsibility of the caller to zero the padding. There is > nothing to do on the reading side, the hypervisor will use "p" which > will be the size of the natural pointer. For a 32-bit Xen the check would be that a guest was not inadvertently violating this rule, such a guest would crash if it was run on a 64-bit hypervisor (which would see the non-zero padding as part of the pointer), by rejecting such cases on 32-bit Xen we avoid such guests becoming established and therefore presenting a case for us to relax this rule in one way or another. This is the same reason as check_multicall_32bit_clean() exists. multicall is special only in that it was pretty easy to know where to add that check. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On 04/11/15 14:29, Ian Campbell wrote: > This looses out on the arm32 hypervisor sanity checking that the > padding > bytes are 0 (as required by the ABI) but TBH I haven't checked that > the > current version has that property either. It's done during the assignation by the compiler: (hnd).q = (uint64_t)(uintptr_t)(val); >>> >>> I meant on the reading side. >> >> It's the responsibility of the caller to zero the padding. There is >> nothing to do on the reading side, the hypervisor will use "p" which >> will be the size of the natural pointer. > > For a 32-bit Xen the check would be that a guest was not inadvertently > violating this rule, such a guest would crash if it was run on a 64-bit > hypervisor (which would see the non-zero padding as part of the pointer), > by rejecting such cases on 32-bit Xen we avoid such guests becoming > established and therefore presenting a case for us to relax this rule in > one way or another. It would add overhead each time we want to copy to/from the guest memory. TBH, I don't think it's our business to check if the guest properly filling out the structure. It could happen only when the guest decides to implement it's own set_guest_handle_macro. As long as we don't crash the hypervisor it's fine. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
>>> On 30.10.15 at 20:59, wrote: > Using local labels causes the stack trace to use the last non-local > label emitted by the compiler in the translation unit, which is almost > always unrelated. > > e.g. A (contrived debug) example switches from: > > (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] > (XEN) CPU:0 > (XEN) RIP:e008:[] > asm_domain_crash_synchronous+0x44/0x4c > ... > (XEN) Xen call trace: > (XEN)[] asm_domain_crash_synchronous+0x44/0x4c > (XEN)[] handle_keypress+0xa4/0xd9 > > to: > > (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] > (XEN) CPU:0 > (XEN) RIP:e008:[] unlikely.vmptrld.921+0/0x8 > ... > (XEN) Xen call trace: > (XEN)[] unlikely.vmptrld.921+0/0x8 > (XEN)[] handle_keypress+0xa4/0xd9 > > which is far more relevant when identifying %eip. I disagree; I specifically used local labels here to avoid cluttering the symbol table. If anything I'd agree to adding a label to the start of subsection 1 (or .fixup), but it's not clear how emitting such labels could be avoided when that section is empty (such labels would clash with whatever label is first in the next compilation unit). Maybe we could discard them in the symbols processing tool if their address matches another symbol. > Additionally, correct the inclusion of the tag parameter in the C UNLIKELY > blocks, to use the passed tag, rather than a literal ".tag". This part I agree with of course, and would therefore like to as for this to be split off. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 2015-11-04 at 14:42 +, Julien Grall wrote: > On 04/11/15 14:29, Ian Campbell wrote: > > > > > > This looses out on the arm32 hypervisor sanity checking that > > > > > > the > > > > > > padding > > > > > > bytes are 0 (as required by the ABI) but TBH I haven't checked > > > > > > that > > > > > > the > > > > > > current version has that property either. > > > > > > > > > > It's done during the assignation by the compiler: > > > > > > > > > > (hnd).q = (uint64_t)(uintptr_t)(val); > > > > > > > > I meant on the reading side. > > > > > > It's the responsibility of the caller to zero the padding. There is > > > nothing to do on the reading side, the hypervisor will use "p" which > > > will be the size of the natural pointer. > > > > For a 32-bit Xen the check would be that a guest was not inadvertently > > violating this rule, such a guest would crash if it was run on a 64-bit > > hypervisor (which would see the non-zero padding as part of the > > pointer), > > by rejecting such cases on 32-bit Xen we avoid such guests becoming > > established and therefore presenting a case for us to relax this rule > > in > > one way or another. > > It would add overhead each time we want to copy to/from the guest memory. I was simply commenting that the approach taken here might be removing such a check _if_ it exists today, and as I said up front I'm not sure we have such a check right now or not. I'm not suggesting you add one as a new check, just that _if_ it already exists then it being removed needs to be considered. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [ovmf baseline-only test] 38247: all pass
This run is configured for baseline tests only. flight 38247 ovmf real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/38247/ Perfect :-) All tests in this flight passed version targeted for testing: ovmf fad21b7c57336bbf0ae363d56e35cbccca67ff3b baseline version: ovmf 8aa6ebe83f91fcd5e1a67d316e444f4ac8b1319d Last test of basis38245 2015-11-03 08:58:06 Z1 days Testing same since38247 2015-11-04 12:50:28 Z0 days1 attempts People who touched revisions under test: Cinnamon Shia Jeff Fan Michael Kinney Ruiyu Ni Samer El-Haj-Mahmoud Sunny Wang 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.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. commit fad21b7c57336bbf0ae363d56e35cbccca67ff3b Author: Sunny Wang Date: Tue Nov 3 02:58:30 2015 + MdeModulePkg: Fix memory leak issues Fix memory leak issues Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Sunny Wang Reviewed-by: Ruiyu Ni git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18722 6f19259b-4bc3-4df7-8a09-765794883524 commit e7e346962b2ae6fcf05b840149cb0b1768173ae2 Author: Cinnamon Shia Date: Tue Nov 3 02:44:48 2015 + MdeModulePkg/RegularExpressionDxe: Add missing PrintLib AsciiVSPrint is used in RegularExpressionDxe/Oniguruma/OnigurumaUefiPort.c. But PrintLib is missing in RegularExpressionDxe.inf. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Cinnamon Shia Reviewed-by: Samer El-Haj-Mahmoud git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18721 6f19259b-4bc3-4df7-8a09-765794883524 commit 0af8e57c740304a5ee79d40d227b673fa9f223ef Author: Cinnamon Shia Date: Tue Nov 3 02:43:03 2015 + MdeModulePkg/RegularExpressionDxe: Correct copyright Correct copyrights in RegularExpressionDxe Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Cinnamon Shia Reviewed-by: Samer El-Haj-Mahmoud git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18720 6f19259b-4bc3-4df7-8a09-765794883524 commit c7e7613e09ff10257dfc301ac1cd3bff807e6c02 Author: Ruiyu Ni Date: Tue Nov 3 02:34:21 2015 + MdeModulePkg: Fix a PciBusDxe hot plug bug For a hot plug bridge with device attached, PciBusDxe driver reserves the resources which equal to the total amount of padding resource returned from HotPlug->GetResourcePadding() and the actual occupied resource by the attached device. The behavior is incorrect. Correct behavior is to reserve the bigger one between the padding resource and the actual occupied resource. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Reviewed-by: Feng Tian git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18719 6f19259b-4bc3-4df7-8a09-765794883524 commit f67bd32ddaae9d5c11481007bbd63e0a3e881f3a Author: Ruiyu Ni Date: Tue Nov 3 02:33:41 2015 + MdeModulePkg: Fix a PCI resource dumping bug in PciBusDxe driver The resource dumping logic contains a bug which cannot dump the resource for hot plug controller correctly. The patch fixes this bug. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Reviewed-by: Feng Tian git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18718 6f19259b-4bc3-4df7-8a09-765794883524 commit b3800cfd10abe8033e16147ad16b01cd16aae0d7 Author: Ruiyu Ni Date: Tue Nov 3 02:33:05 2015 + Revert "MdeModulePkg: Fix a PciBusDxe hot plug bug" Leif suggested to split the big patch to smaller ones. This reverts commit 73b7f115c653c807b9d0be97bf516871d8aff7ba. Contributed-under: TianoCore Contribution Agr
Re: [Xen-devel] [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS
Hi Dario, 2015-11-04 9:12 GMT-05:00 Dario Faggioli : > On Mon, 2015-11-02 at 09:45 -0500, Meng Xu wrote: > > > > I guess maybe you forgot to change it in this commit but change > > > > it > > > > the > > > > following commit? > > > > > > > No, this is one of the few thing that changed between v2 and v3. > > > > > > Regards, > > > Dario > > > > Thanks for the explanation! Then the patch looks good to me, at least > > for RTDS scheduler. :-) > > > Thanks for looking at the patch. > > Just FTR (and for next time :-D), is the above something that can be > interpreted as a 'Reviewed-by: Meng Xu ' ? If no (e.g., because > you haven't looking thoroughly enough to feel confident to express it), > then fine, I was just asking. > Thank you very much for explaining this for me. :-) I feel confident about the changes for RTDS scheduler. I'm not so confident about the change in the schedule.c. To be specific, this patch removes insert_vcpu in schedule_cpu_switch () in schedule.c; I'm not so sure if it is ok to insert_vcpu when a domain is moved. (Next time, I will stand out and ask although it may be a stupid question. ;-) ) So as to this patch, I will say: As far as the RTDS scheduler is concerned: Reviewed-by: Meng Xu < men...@cis.upenn.edu > > > If yes, I encourage you to say it explicitly, to avoid errors and > misjudgements. If you 'only' looked at the patch with the RTDS > scheduler in mind, that is fine too. You can say something like "As far > as the RTDS scheduler is concerned: Reviewed-by: Meng Xu ". Other > reviewers and committers will take this into account and properly > weight it. > > Every akc/review is important, and, if you took the time to look at a > patch, why don't say it in the proper way? :-) > Sure! I will keep this in mind. > I'm about to send v4 of this series. Feel free (only if you want, of > course!), to chime in in that thread. > Sure. I will try to have a look. :-) Thanks, Meng --- Meng Xu PhD Student in Computer and Information Science University of Pennsylvania http://www.cis.upenn.edu/~mengxu/ ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
On 04/11/15 14:45, Jan Beulich wrote: On 30.10.15 at 20:59, wrote: >> Using local labels causes the stack trace to use the last non-local >> label emitted by the compiler in the translation unit, which is almost >> always unrelated. >> >> e.g. A (contrived debug) example switches from: >> >> (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] >> (XEN) CPU:0 >> (XEN) RIP:e008:[] >> asm_domain_crash_synchronous+0x44/0x4c >> ... >> (XEN) Xen call trace: >> (XEN)[] asm_domain_crash_synchronous+0x44/0x4c >> (XEN)[] handle_keypress+0xa4/0xd9 >> >> to: >> >> (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] >> (XEN) CPU:0 >> (XEN) RIP:e008:[] unlikely.vmptrld.921+0/0x8 >> ... >> (XEN) Xen call trace: >> (XEN)[] unlikely.vmptrld.921+0/0x8 >> (XEN)[] handle_keypress+0xa4/0xd9 >> >> which is far more relevant when identifying %eip. > I disagree; I specifically used local labels here to avoid cluttering the > symbol table. Needless cluttering is indeed bad. However, the effect here is a misleading stack trace. As the entire point of the symbol table is to generate usable information in situations like this, I don't see this as an issue. > If anything I'd agree to adding a label to the start of > subsection 1 (or .fixup), How is this suggestion any different to what I have presented? Each unlikely section still gets a non-local label. > but it's not clear how emitting such labels > could be avoided when that section is empty (such labels would clash > with whatever label is first in the next compilation unit). All UNLIKELY() sections have content, as they are manually created. Or have I missed something? > Maybe we > could discard them in the symbols processing tool if their address > matches another symbol. > >> Additionally, correct the inclusion of the tag parameter in the C UNLIKELY >> blocks, to use the passed tag, rather than a literal ".tag". > This part I agree with of course, and would therefore like to as for this > to be split off. I will see about this, after we have agreed on the other bits. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Xen 4.5.2 released
All, I am pleased to announce the release of Xen 4.5.2. This is available immediately from its git repository http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.5 (tag RELEASE-4.5.2) or from the XenProject download page http://www.xenproject.org/downloads/xen-archives/xen-45-series/xen-452.html (where a list of changes can also be found). We recommend all users of the 4.5 stable series to update to this latest point release. Regards, Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] (XEN) APIC error on CPU0: 40(00)
Hi, i just build the latest 4.3.0 kernel and ran it on qubes-os with xen 4.4.3. I get this error just before the reboot: [(XEN) APIC error on CPU0: 40(00) (XEN) Resetting with ACPI MEMORY or I/O RESET_REG. I can also see some weird acpi-related messages on the log i attached, like: (XEN) ACPI: Invalid sleep control/status register data: 0:0x8:0x3 0:0x8:0x3 (XEN) ERST table was not found [3.608404] ACPI Error: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND (20150818/dswload-210) [3.608410] ACPI Exception: AE_NOT_FOUND, During name lookup/catalog (20150818/psobject-227) [3.608439] ACPI Exception: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20150818/tbxfload-193) [3.619914] ACPI Error: 1 table load failures, 5 successful (20150818/tbxfload-214) [3.679113] [Firmware Bug]: ACPI: BIOS _OSI(Linux) query ignored [3.688942] ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20150818/hwxface-580) [3.688948] ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S2_] (20150818/hwxface-580) [3.756027] PM-Timer failed consistency check (0xff) - aborting. With older kernels (like 3.12 or 4.2.1) i always see those messages but i'm able to boot without the acpi error. My machine runs on skylake cpu with z170 chipset and the latest bios. Any ideas on how to fix this? Thanks, John. Loading Xen 4.4.3 ... Loading Linux 4.3-0.pvops.qubes.x86_64 ... Xen 4.4.3-7.fc20 (XEN) Xen version 4.4.3 (user@) (gcc (GCC) 4.8.3 20140911 (Red Hat 4.8.3-7)) debug=n Mon Oct 19 16:28:51 UTC 2015 (XEN) Latest ChangeSet: (XEN) Bootloader: GRUB 2.00 (XEN) Command line: placeholder console=ttyS0 com1=115200,8n1 console=com1 dom0_mem=min:1024Mi loglvl=all apic_verbosity=debug e820-verbose=true xsave=0 iommu=verbose (XEN) Video information: (XEN) VGA is text mode 80x25, font 8x16 (XEN) VBE/DDC methods: V2; EDID transfer time: 1 seconds (XEN) Disc information: (XEN) Found 1 MBR signatures (XEN) Found 1 EDD information structures (XEN) Initial Xen-e820 RAM map: (XEN) - 0009c800 (usable) (XEN) 0009c800 - 000a (reserved) (XEN) 000e - 0010 (reserved) (XEN) 0010 - 61984000 (usable) (XEN) 61984000 - 61985000 (ACPI NVS) (XEN) 61985000 - 619cf000 (reserved) (XEN) 619cf000 - 61a22000 (usable) (XEN) 61a22000 - 62127000 (reserved) (XEN) 62127000 - 66b96000 (usable) (XEN) 66b96000 - 677ab000 (reserved) (XEN) 677ab000 - 67f9a000 (ACPI NVS) (XEN) 67f9a000 - 67fff000 (ACPI data) (XEN) 67fff000 - 6800 (usable) (XEN) 0001 - 00089200 (usable) (XEN) 6800 - 6810 (reserved) (XEN) e000 - f000 (reserved) (XEN) fe00 - fe011000 (reserved) (XEN) fec0 - fec01000 (reserved) (XEN) fee0 - fee01000 (reserved) (XEN) ff00 - 0001 (reserved) (XEN) Checking MTRR ranges... (XEN) MTRR cap: 1d0a type: c06 (XEN) Xen-e820 RAM map: (XEN) - 0009c800 (usable) (XEN) 0009c800 - 000a (reserved) (XEN) 000e - 0010 (reserved) (XEN) 0010 - 61984000 (usable) (XEN) 61984000 - 61985000 (ACPI NVS) (XEN) 61985000 - 619cf000 (reserved) (XEN) 619cf000 - 61a22000 (usable) (XEN) 61a22000 - 62127000 (reserved) (XEN) 62127000 - 66b96000 (usable) (XEN) 66b96000 - 677ab000 (reserved) (XEN) 677ab000 - 67f9a000 (ACPI NVS) (XEN) 67f9a000 - 67fff000 (ACPI data) (XEN) 67fff000 - 6800 (usable) (XEN) 6800 - 6810 (reserved) (XEN) e000 - f000 (reserved) (XEN) fe00 - fe011000 (reserved) (XEN) fec0 - fec01000 (reserved) (XEN) fee0 - fee01000 (reserved) (XEN) ff00 - 0001 (reserved) (XEN) 0001 - 00089200 (usable) (XEN) ACPI: RSDP 000F05B0, 0024 (r2 ALASKA) (XEN) ACPI: XSDT 67A1F098, 00B4 (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FACP 67A41B30, 010C (r5 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: DSDT 67A1F1E8, 22943 (r2 ALASKA A M I 1072009 INTL 20120913) (XEN) ACPI: FACS 67F99F80, 0040 (XEN) ACPI: APIC 67A41C40, 00BC (r3 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FPDT 67A41D00, 0044 (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FIDT 67A41D48, 009C (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: MCFG 67A41DE8, 003C (r1 ALASKAA M I 1072009 MSFT 97) (XEN) ACPI: HPET 67A41E28, 0038 (r1 ALASKA
Re: [Xen-devel] [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
>>> On 04.11.15 at 16:07, wrote: > On 04/11/15 14:45, Jan Beulich wrote: > On 30.10.15 at 20:59, wrote: >>> Using local labels causes the stack trace to use the last non-local >>> label emitted by the compiler in the translation unit, which is almost >>> always unrelated. >>> >>> e.g. A (contrived debug) example switches from: >>> >>> (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] >>> (XEN) CPU:0 >>> (XEN) RIP:e008:[] >>> asm_domain_crash_synchronous+0x44/0x4c >>> ... >>> (XEN) Xen call trace: >>> (XEN)[] asm_domain_crash_synchronous+0x44/0x4c >>> (XEN)[] handle_keypress+0xa4/0xd9 >>> >>> to: >>> >>> (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] >>> (XEN) CPU:0 >>> (XEN) RIP:e008:[] unlikely.vmptrld.921+0/0x8 >>> ... >>> (XEN) Xen call trace: >>> (XEN)[] unlikely.vmptrld.921+0/0x8 >>> (XEN)[] handle_keypress+0xa4/0xd9 >>> >>> which is far more relevant when identifying %eip. >> I disagree; I specifically used local labels here to avoid cluttering the >> symbol table. > > Needless cluttering is indeed bad. However, the effect here is a > misleading stack trace. > > As the entire point of the symbol table is to generate usable > information in situations like this, I don't see this as an issue. > >> If anything I'd agree to adding a label to the start of >> subsection 1 (or .fixup), > > How is this suggestion any different to what I have presented? Each > unlikely section still gets a non-local label. Your proposal produces one label per code addition to the unlikely section. My proposal generates just one label, no matter how many contributions to the section get done in a compilation unit. >> but it's not clear how emitting such labels >> could be avoided when that section is empty (such labels would clash >> with whatever label is first in the next compilation unit). > > All UNLIKELY() sections have content, as they are manually created. Or > have I missed something? I guess you take each .subsection as representation of a separate section, while I take only their set per compilation unit as one (and this being a sub-section means even that is still to wide a term). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 4 Nov 2015, Julien Grall wrote: > On 04/11/15 11:27, Ian Campbell wrote: > > On Wed, 2015-11-04 at 11:17 +, Julien Grall wrote: > >>> > >>> we could: > >>> #ifdef(__XEN__) > >>> #define XEN_BUILD_BUG_ON(x) BUILD_BUG_ON(x) > >>> #elif !defined(XEN_BUILD_BUG_ON) > >>> #define XEN_BUILD_BUG_ON(x) > >>> #endif > >>> and using XEN_BUILD_BUG_ON in the macro > >>> > >>> So, __XEN__ builds get the check and users can opt in by providing > >>> XEN_BUILD_BUG_ON if they want. If they don't then they don't get the > >>> check. > >> > >> I wouldn't let the user the choice to disable build-time check. > > > > Up to you. Note that "the user" here would potentially include > > xen.git/tools, and I would expect them to want to define it. > > > > Also... > > > >> There is > >> no harm to open-code them as I did today and avoid possible issue in the > >> code later. > > > > ... there is always a downside to open coding. If you don't want to expose > > the ability to define a BUG_ON to the application then just drop that #elif > > from the chain. > > Good point. I will give a look. > > > > >>> Or maybe we could just omit this from the public API by one or both of > >>> a) > >>> adding an explicit 8 byte type to the union purely to force the size > >>> and/or > >> > >> This is already done in the current version: > >> > >> typedef union { type *p; uint64_aligned_t q; } \ > >> __guest_handle_64_ ## name; > >> > >> However I don't see how this ensure that the caller of > >> set_xen_raw_guest_handle will effectively hnd as a pointer to an 8-byte > >> placeholder. > > > > Not sure I follow. If hnd isn't a suitable struct xen guest handle then > > other things will fail. If a user is passing a non struct xen_guest_handle > > to this which happens to contain the same fields then more fool them, and > > if it happens to be 8 bytes anyway your check won't catch that. > > With the 2 checks in set_xen_raw_guest_handle we catch most of the > problem. They both ensure that the handle is 8-byte and the pointer is > valid. However we don't check that the padding is at the beginning of > the structure. > > It's better than what we have today as we don't even check that the > handle is 8-byte. > > [...] > > >>> This looses out on the arm32 hypervisor sanity checking that the padding > >>> bytes are 0 (as required by the ABI) but TBH I haven't checked that the > >>> current version has that property either. > >> > >> It's done during the assignation by the compiler: > >> > >> (hnd).q = (uint64_t)(uintptr_t)(val); > > > > I meant on the reading side. > > It's the responsibility of the caller to zero the padding. There is > nothing to do on the reading side, the hypervisor will use "p" which > will be the size of the natural pointer. Sorry to be pedantic, but why is this safe given that previously you wrote: Finally, based on the defect report #283 [2], the behavior of writing from one member and reading from another is not clear. Looks like it might be better to remove the union completely? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > Finally, based on the defect report #283 [2], the behavior of writing > from one member and reading from another is not clear. Because the hypervisor is compiled with -fno-strict-aliasing. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 2015-11-04 at 15:19 +, Stefano Stabellini wrote: > On Wed, 4 Nov 2015, Julien Grall wrote: > > On 04/11/15 11:27, Ian Campbell wrote: > > > On Wed, 2015-11-04 at 11:17 +, Julien Grall wrote: > > > > > > > > > > we could: > > > > > #ifdef(__XEN__) > > > > > #define XEN_BUILD_BUG_ON(x) BUILD_BUG_ON(x) > > > > > #elif !defined(XEN_BUILD_BUG_ON) > > > > > #define XEN_BUILD_BUG_ON(x) > > > > > #endif > > > > > and using XEN_BUILD_BUG_ON in the macro > > > > > > > > > > So, __XEN__ builds get the check and users can opt in by > > > > > providing > > > > > XEN_BUILD_BUG_ON if they want. If they don't then they don't get > > > > > the > > > > > check. > > > > > > > > I wouldn't let the user the choice to disable build-time check. > > > > > > Up to you. Note that "the user" here would potentially include > > > xen.git/tools, and I would expect them to want to define it. > > > > > > Also... > > > > > > > There is > > > > no harm to open-code them as I did today and avoid possible issue > > > > in the > > > > code later. > > > > > > ... there is always a downside to open coding. If you don't want to > > > expose > > > the ability to define a BUG_ON to the application then just drop that > > > #elif > > > from the chain. > > > > Good point. I will give a look. > > > > > > > > > > Or maybe we could just omit this from the public API by one or > > > > > both of > > > > > a) > > > > > adding an explicit 8 byte type to the union purely to force the > > > > > size > > > > > and/or > > > > > > > > This is already done in the current version: > > > > > > > > typedef union { type *p; uint64_aligned_t q; } \ > > > > __guest_handle_64_ ## name; > > > > > > > > However I don't see how this ensure that the caller of > > > > set_xen_raw_guest_handle will effectively hnd as a pointer to an 8- > > > > byte > > > > placeholder. > > > > > > Not sure I follow. If hnd isn't a suitable struct xen guest handle > > > then > > > other things will fail. If a user is passing a non struct > > > xen_guest_handle > > > to this which happens to contain the same fields then more fool them, > > > and > > > if it happens to be 8 bytes anyway your check won't catch that. > > > > With the 2 checks in set_xen_raw_guest_handle we catch most of the > > problem. They both ensure that the handle is 8-byte and the pointer is > > valid. However we don't check that the padding is at the beginning of > > the structure. > > > > It's better than what we have today as we don't even check that the > > handle is 8-byte. > > > > [...] > > > > > > > This looses out on the arm32 hypervisor sanity checking that the > > > > > padding > > > > > bytes are 0 (as required by the ABI) but TBH I haven't checked > > > > > that the > > > > > current version has that property either. > > > > > > > > It's done during the assignation by the compiler: > > > > > > > > (hnd).q = (uint64_t)(uintptr_t)(val); > > > > > > I meant on the reading side. > > > > It's the responsibility of the caller to zero the padding. There is > > nothing to do on the reading side, the hypervisor will use "p" which > > will be the size of the natural pointer. > > Sorry to be pedantic, but why is this safe given that previously you > wrote: > > Finally, based on the defect report #283 [2], the behavior of writing > from one member and reading from another is not clear. The writer via one is the guest and reader via the other is the hypervisor, so no matter what they are certainly different compilation units, even in the face of whole program optimisations. The concerning issue is that if the compiler can observe you writing to both halves of the union then it can either omit the first write or dive off into deep undefined behaviour territory. > Looks like it might be better to remove the union completely? If that were possible I think someone would have suggested a scheme which achieves it within the other constraints. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xenconsoled: Remove unexpected daemonize behavior
On Tue, 2015-11-03 at 17:20 +, Wei Liu wrote: > Acked-by: Wei Liu Applied w/ this + Ian J's ack. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xl: avoid (another) uninitialised use of rc in vcpuset()
On Wed, 2015-11-04 at 13:03 +0100, Dario Faggioli wrote: > Rearange the case when we check the new number of vCPUs > against the number of host pCPUs not to use rc for internal > error reporting. In fact: > - rc was at risk of being used uninitialised; > - rc should only be used for holding libxl error codes. > > Signed-off-by: Dario Faggioli acked + applied. > --- > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Wei Liu > Cc: Harmandeep Kaur > Cc: Andrew Cooper > --- > tools/libxl/xl_cmdimpl.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 9b6b42c..78048a1 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5457,21 +5457,21 @@ static int vcpuset(uint32_t domid, const char* > nr_vcpus, int check_host) > * by the host's amount of pCPUs. > */ > if (check_host) { > -unsigned int host_cpu = libxl_get_max_cpus(ctx); > +unsigned int online_vcpus, host_cpu = libxl_get_max_cpus(ctx); > libxl_dominfo dominfo; > > if (libxl_domain_info(ctx, &dominfo, domid)) > return 1; > > -if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) { > +online_vcpus = dominfo.vcpu_online; > +libxl_dominfo_dispose(&dominfo); > + > +if (max_vcpus > online_vcpus && max_vcpus > host_cpu) { > fprintf(stderr, "You are overcommmitting! You have %d > physical" \ > " CPUs and want %d vCPUs! Aborting, use --ignore- > host to" \ > " continue\n", host_cpu, max_vcpus); > -rc = 1; > -} > -libxl_dominfo_dispose(&dominfo); > -if (rc) > return 1; > +} > } > rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > if (rc) { > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xl: log an error if libxl_cpupool_destroy() fails
On Wed, 2015-11-04 at 11:48 +0100, Dario Faggioli wrote: > In fact, right now, failing at destroying a cpupool is just > not reported to the user in any explicit way. > > Let's log an error, as it is customary for xl in these cases. > > Signed-off-by: Dario Faggioli > Reviewed-by: Juergen Gross > Acked-by: Wei Liu Applied (cleanly) thanks. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [CALL-FOR-AGENDA] Monthly Xen.org Technical Call (2015-11-11)
The next Xen technical call will be at: Wed 11 Nov 17:00:00 GMT 2015 `date -d @1447261200` See http://lists.xen.org/archives/html/xen-devel/2015-01/msg00414.html for more information on the call. Please let me know (CC-ing the list) any topics which you would like to discuss. It might be useful to include: * References to any relevant/recent mailing list threads; * Other people who you think should be involved in the discussion (and CC them); If you would like to attend then please let me know so I can send you the dial in details. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Ian Jackson writes ("Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 4/4] xen/public: arm: > rework the macro set_xen_guest_handle_raw"): > > Finally, based on the defect report #283 [2], the behavior of writing > > from one member and reading from another is not clear. > > Because the hypervisor is compiled with -fno-strict-aliasing. Incidentally, it is _necessary_ to write to the uint64_t last. C99 6.2.6.1(5) says that if you write to the 32-bit pointer, the upper bits (corresponding to the uint64 but not part of the pointer) take on unspecified values. So writing the 32-bit pointer always invalidates the top bits, so if you write to the uint64_t first, the 32-bit write makes the 64-bit write a dead store. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Ian Campbell writes ("Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > The writer via one is the guest and reader via the other is the hypervisor, > so no matter what they are certainly different compilation units, even in > the face of whole program optimisations. The question of them being different `compilation units' (YM translation units) is irrelevant I think. > The concerning issue is that if the compiler can observe you writing to > both halves of the union then it can either omit the first write or dive > off into deep undefined behaviour territory. If the compiler can see you write to p, it is allowed to assume that all subsequent readers will read the object as typeof(p). Reading typeof(p) does not read the padding. Therefore the compiler is allowed to `prove' that the padding is a dead store, and remove the write to the padding. This applies even if the compiler can't see the code which is doing the reading. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS
On Wed, 2015-11-04 at 10:01 -0500, Meng Xu wrote: > 2015-11-04 9:12 GMT-05:00 Dario Faggioli : > > Just FTR (and for next time :-D), is the above something that can > > be > > interpreted as a 'Reviewed-by: Meng Xu ' ? If no (e.g., > > because > > you haven't looking thoroughly enough to feel confident to express > > it), > > then fine, I was just asking. > Thank you very much for explaining this for me. :-) > > I feel confident about the changes for RTDS scheduler. > Ok. > I'm not so confident about the change in the schedule.c. To be > specific, this patch removes insert_vcpu in schedule_cpu_switch() in > schedule.c; > It removes the attempt of inserting the idle vCPU in the runqueue of the scheduler of the target cpupool for the pCPU. More specifically, this line: SCHED_OP(new_ops, insert_vcpu, idle); If we look at the various ways in which insert_vcpu is implemented, we have: csched_vcpu_insert: if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc->is_running ) __runq_insert(vc->processor, svc); but the pCPU being switched is free, i.e., it is not in any cpupool, and it is idling. So, the idle vCPU is running, and the condition above is false, which means __runq_insert() is not really called. csched2_vcpu_insert: if ( ! is_idle_vcpu(vc) ) { ... } so trying to insert the idle vCPU is actually a nop. rt_vcpu_insert: if ( is_idle_vcpu(vc) ) return; a nop again. My point being that this patch actually removes nothing but a bunch of if()-s, with no effect at all. > I'm not so sure if it is ok to insert_vcpu when a domain is moved. > Hopefully, I addressed your doubts. BTW, we're not talking about a domain being moved between pools. That is done with another function. schedule_cpu_switch() is about taking a pCPU off from a cpupool, or assigning a pCPU to cpupool. > (Next time, I will stand out and ask although it may be a stupid > question. ;-) ) > Sure! And this does not look a stupid question to me. :-) > So as to this patch, I will say: > As far as the RTDS scheduler is concerned: Reviewed-by: Meng Xu < > men...@cis.upenn.edu> > Ok, I haven't sent v4 yet, so I'll apply it there. Thanks. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On Wed, 2015-11-04 at 15:46 +, Ian Jackson wrote: > Ian Campbell writes ("Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework > the macro set_xen_guest_handle_raw"): > > The writer via one is the guest and reader via the other is the > > hypervisor, > > so no matter what they are certainly different compilation units, even > > in > > the face of whole program optimisations. > > The question of them being different `compilation units' (YM > translation units) is irrelevant I think. > > > The concerning issue is that if the compiler can observe you writing to > > both halves of the union then it can either omit the first write or > > dive > > off into deep undefined behaviour territory. > > If the compiler can see you write to p, it is allowed to assume that > all subsequent readers will read the object as typeof(p). Reading > typeof(p) does not read the padding. Therefore the compiler is > allowed to `prove' that the padding is a dead store, and remove the > write to the padding. > > This applies even if the compiler can't see the code which is doing > the reading. Ah, yes :-( Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7 16/32] xen/x86: allow disabling the pmtimer
El 03/11/15 a les 13.41, Jan Beulich ha escrit: On 03.11.15 at 11:57, wrote: >> On 03/11/15 07:21, Jan Beulich wrote: >> On 30.10.15 at 16:36, wrote: On 30/10/15 13:16, Jan Beulich wrote: On 30.10.15 at 13:50, wrote: >> El 14/10/15 a les 16.37, Jan Beulich ha escrit: >> On 02.10.15 at 17:48, wrote: Signed-off-by: Roger Pau Monné Cc: Jan Beulich Cc: Andrew Cooper --- Changes since v6: - Return ENODEV in pmtimer_load if the timer is disabled. - hvm_acpi_power_button and hvm_acpi_sleep_button become noops if the pmtimer is disabled. >>> But how are those two features connected? I don't think you can >>> assume absence of a PM block just because there's no PM timer. >>> Or if you want to tie them together for now, the predicate needs >>> to be renamed. >>> - Return ENODEV if pmtimer_change_ioport is called with the pmtimer disabled. >>> Same here. >> What about changing XEN_X86_EMU_PMTIMER into XEN_X86_EMU_PM and this >> flags disables all PM stuff? > Ah, right, that's a reasonable option. It still might be a nice idea to split them in two, given future work. To support hotplug properly (cpu, ram and pci), Xen needs to inject GPEs, which comes from part of the PM infrastructure. To support PCI devices in the future without the whole PM infrastructure, it would be nice to keep the split. >>> Coming back to this - I'm not sure: The hotplug aspect as you >>> mention it should matter for Dom0 only. DomU could (and perhaps >>> should) use a PV interface instead. >> >> I disagree. >> >> All PVH guests should use the same mechanism; making a split between >> dom0 and domU will only make our lives harder. >> >> Where reasonable, we should follow what happens on native; one of the >> underlying points of PVH is to have less of an impact on the guest >> side. In some cases it is indeed nasty, but has the advantage of being >> well understood. > > What meaning would ACPI have to a PVH DomU? > >>> So I'd like to suggest quite the opposite: Don't call the thing PM, >>> but make it more general and call it ACPI. And instead of >>> separating HPET, we might have this fall under ACPI as well, or >>> we might have a second TIMER flag, requiring both to be set >>> for there to be a HPET and PMTMR. This leaves open the option >>> of Dom0 getting ACPI enabled (despite this then being "real", >>> not emulated ACPI), but TIMER left off. >> >> An HPET can exist independently of other features such as ACPI. It >> should have its own option. > > Without ACPI there's no defined way to discover it. Doing what > Linux does - applying chipset knowledge - won't work on PVH either, > because there's no emulated chipset. Which would leave scanning > physical memory, but if there is none, none can be found. > >> +1 to having an ACPI option, but as indicated above, I expect it to be >> used in the longterm even for domU. > > Again - why and how? I think that at this point in the design it's not so important to have all the XEN_X86_EMU_* properly defined. This is not a public interface, so we can expand/reduce them whenever we want. Would it be fine, for the time being to just have a XEN_X86_EMU_PM and control both the PM and the PMTMR? Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] xen/serial: Return actual bytes stored in TX FIFO for OMAP
On Tue, Nov 3, 2015 at 6:36 PM, Ian Campbell wrote: > On Tue, 2015-10-27 at 10:40 -0600, Jan Beulich wrote: >> > > > On 27.10.15 at 17:32, wrote: >> > And what are you suggesting about the patch? Create a separate OMAP >> > specific header? >> >> For #define-s used by only a single source file I'd suggest putting >> them into the source file instead of into a (shared) header. > > I agree. (Any to be clear, any existing OMAP specific things in 8250,h > should move too). OK. I will take it into account. Thank you > > Ian. > -- Oleksandr Tyshchenko | Embedded Dev GlobalLogic www.globallogic.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v4 07/23] tools: Refactor /dev/xen/gnt{dev, shr} wrappers into libxengnttab.
On Thu, 2015-10-29 at 16:28 +, Wei Liu wrote: > On Wed, Oct 21, 2015 at 04:23:14PM +0100, Ian Campbell wrote: > > libxengnttab will provide a stable API and ABI for accessing the > > grant table devices. > > > > The functions are moved into the xengnt{tab,shr} namespace to make a > > clean break from libxc and avoid ambiguity regarding which interfaces > > are stable. > > > > XXX consider combining into a single namespace (i.e. with > > xengnttab_handle having two open fd's in it on Linux) > > > > This? Currently there are two separate interfaces (i.e. 2x_open, 2x handle types, 2x sets of functions for using the handle, 2x namespaces, etc) in this library corresponding to two underlying devices in /dev/xen, one of which is for utilising grant references which you have been given (gntdev), the other of which is for allocating suitable memory for granting and creating grants of it to give away to others (gntshr). Only Linux and mini-os implement gntshr today, I think. AFAIK most consumers (e.g. qemu's pv backends, or userspace pv backends generally) are only using the first (consuming grants). The only user of gntshr I know of is libvchan, but in general any userspace frontend would likely need this functionality. There are three possibilities here for the stable library interface: * Do nothing, keep it as two interfaces in one library. * Split into separate libgntdev and libgntshr libraries. * Unify the two APIs such that a single handle actually contains two fd's and functions are all in the same namespace etc and just the correct underlying fd associated with the op. The comment is wondering if we should do the third, I hadn't thought of the middle one at the time. Right now my preference is towards the first, the implementation of which is "delete the XXX comment". The downside of not merging is that if some new OS port comes along which has gntdev and gntshr in a single device that the applications then still have to open and manage two handles and updating the ABI to unify them later would be a pain. I'm inclined towards lazily thinking that having two handles is not such a big deal, even on an OS where the underlying implementation is such that they contain "redundant" file handles. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Linux 4.4 MW: Boot under Xen fails with CONFIG_DEBUG_WX enabled: RIP: ptdump_walk_pgd_level_core
On 11/04/2015 06:55 AM, Sander Eikelenboom wrote: Hi All, I just tried to boot with the current linus mergewindow tree under Xen. It fails with a kernel panic at boot with the new "CONFIG_DEBUG_WX" option enabled. Disabling it makes the kernel boot fine. The splat: [ 18.424241] Freeing unused kernel memory: 1104K (822fc000 - 8241) [ 18.430314] Write protecting the kernel read-only data: 18432k [ 18.441054] Freeing unused kernel memory: 1144K (880001ae2000 - 880001c0) [ 18.447966] Freeing unused kernel memory: 1560K (88000207a000 - 88000220) [ 18.453947] BUG: unable to handle kernel paging request at 88055c883000 [ 18.459943] IP: [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.465847] PGD 2212067 PUD 0 [ 18.471564] Oops: [#1] SMP [ 18.477248] Modules linked in: [ 18.482918] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.3.0-mw-20151104-linus-doflr+ #1 [ 18.488804] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010 [ 18.494778] task: 880059b9 ti: 880059b98000 task.ti: 880059b98000 [ 18.500852] RIP: e030:[] [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.507102] RSP: e02b:880059b9be48 EFLAGS: 00010296 [ 18.513351] RAX: 88055c883000 RBX: 81ae2000 RCX: 8800 [ 18.519733] RDX: 0067 RSI: 880059b9be98 RDI: 88001000 [ 18.526129] RBP: 880059b9bf00 R08: R09: [ 18.532522] R10: 88005fd0e790 R11: 0001 R12: 88008000 [ 18.538891] R13: cfff R14: 880059b9be98 R15: [ 18.545247] FS: () GS:88005f68() knlGS: [ 18.551708] CS: e033 DS: ES: CR0: 8005003b [ 18.558153] CR2: 88055c883000 CR3: 02211000 CR4: 0660 [ 18.564686] Stack: [ 18.571106] 000159b9be50 82211000 88055c884000 0800 [ 18.577704] 8000 88055c883000 0007 88005fd0e790 [ 18.584291] 880059b9bed8 81156ace 0001 [ 18.590916] Call Trace: [ 18.597458] [] ? free_reserved_area+0x11e/0x120 [ 18.604180] [] ptdump_walk_pgd_level_checkwx+0x12/0x20 [ 18.611014] [] mark_rodata_ro+0xe9/0xf0 [ 18.617819] [] ? rest_init+0x80/0x80 [ 18.624512] [] kernel_init+0x18/0xe0 [ 18.631095] [] ret_from_fork+0x3f/0x70 [ 18.637650] [] ? rest_init+0x80/0x80 [ 18.644178] Code: 70 ff ff ff 48 3b 85 58 ff ff ff 0f 84 c0 fe ff ff 48 8b 85 68 ff ff ff 48 c1 e0 10 48 c1 f8 10 48 89 45 b0 48 8b 85 70 ff ff ff <48> 8b 38 48 85 ff 0f 85 4e ff ff ff b9 02 00 00 00 31 d2 4c 89 [ 18.658246] RIP [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.665211] RSP [ 18.672073] CR2: 88055c883000 [ 18.678852] ---[ end trace d84e34461c40637a ]--- [ 18.685641] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 [ 18.685641] [ 18.699520] Kernel Offset: disable What's your .config? Does cat /sys/kernel/debug/kernel_page_tables produce a similar fault even with CONFIG_DEBUG_WX=n? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] (XEN) APIC error on CPU0: 40(00)
>>> On 04.11.15 at 16:15, wrote: > Hi, > i just build the latest 4.3.0 kernel and ran it on qubes-os with xen > 4.4.3. I get this error just before the reboot: > > [(XEN) APIC error on CPU0: 40(00) And this appeared with the switch to kernel 4.3? No other variable (e.g. all config options new in 4.3 turned off)? I ask because the APIC is driven by Xen, not the kernel, and hence the kernel should have no (direct) influence on APIC behavior. > (XEN) Resetting with ACPI MEMORY or I/O RESET_REG. > > I can also see some weird acpi-related messages on the log i attached, like: > (XEN) ACPI: Invalid sleep control/status register data: 0:0x8:0x3 0:0x8:0x3 > (XEN) ERST table was not found > [3.608404] ACPI Error: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup > failure, AE_NOT_FOUND (20150818/dswload-210) > [3.608410] ACPI Exception: AE_NOT_FOUND, During name lookup/catalog > (20150818/psobject-227) > [3.608439] ACPI Exception: AE_NOT_FOUND, (SSDT:xh_rvp08) while > loading table (20150818/tbxfload-193) > [3.619914] ACPI Error: 1 table load failures, 5 successful > (20150818/tbxfload-214) > [3.679113] [Firmware Bug]: ACPI: BIOS _OSI(Linux) query ignored > [3.688942] ACPI Exception: AE_NOT_FOUND, While evaluating Sleep > State [\_S1_] (20150818/hwxface-580) > [3.688948] ACPI Exception: AE_NOT_FOUND, While evaluating Sleep > State [\_S2_] (20150818/hwxface-580) > [3.756027] PM-Timer failed consistency check (0xff) - aborting. > > With older kernels (like 3.12 or 4.2.1) i always see those messages but > i'm able to boot without the acpi error. I guess those latter messages are unrelated - not the difference between ACPI and APIC. Also I'm unsure about you saying "boot" here but "reboot" above? > My machine runs on skylake cpu with z170 chipset and the latest bios. > Any ideas on how to fix this? Not without first figuring out what bad vector is being received by the APIC, which likely will involved quite a bit of debugging and code instrumentation. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7 16/32] xen/x86: allow disabling the pmtimer
>>> On 04.11.15 at 17:05, wrote: > El 03/11/15 a les 13.41, Jan Beulich ha escrit: > On 03.11.15 at 11:57, wrote: >>> On 03/11/15 07:21, Jan Beulich wrote: >>> On 30.10.15 at 16:36, wrote: > On 30/10/15 13:16, Jan Beulich wrote: > On 30.10.15 at 13:50, wrote: >>> El 14/10/15 a les 16.37, Jan Beulich ha escrit: >>> On 02.10.15 at 17:48, wrote: > Signed-off-by: Roger Pau Monné > Cc: Jan Beulich > Cc: Andrew Cooper > --- > Changes since v6: > - Return ENODEV in pmtimer_load if the timer is disabled. > - hvm_acpi_power_button and hvm_acpi_sleep_button become noops if the >pmtimer is disabled. But how are those two features connected? I don't think you can assume absence of a PM block just because there's no PM timer. Or if you want to tie them together for now, the predicate needs to be renamed. > - Return ENODEV if pmtimer_change_ioport is called with the pmtimer >disabled. Same here. >>> What about changing XEN_X86_EMU_PMTIMER into XEN_X86_EMU_PM and this >>> flags disables all PM stuff? >> Ah, right, that's a reasonable option. > It still might be a nice idea to split them in two, given future work. > > To support hotplug properly (cpu, ram and pci), Xen needs to inject > GPEs, which comes from part of the PM infrastructure. To support PCI > devices in the future without the whole PM infrastructure, it would be > nice to keep the split. Coming back to this - I'm not sure: The hotplug aspect as you mention it should matter for Dom0 only. DomU could (and perhaps should) use a PV interface instead. >>> >>> I disagree. >>> >>> All PVH guests should use the same mechanism; making a split between >>> dom0 and domU will only make our lives harder. >>> >>> Where reasonable, we should follow what happens on native; one of the >>> underlying points of PVH is to have less of an impact on the guest >>> side. In some cases it is indeed nasty, but has the advantage of being >>> well understood. >> >> What meaning would ACPI have to a PVH DomU? >> So I'd like to suggest quite the opposite: Don't call the thing PM, but make it more general and call it ACPI. And instead of separating HPET, we might have this fall under ACPI as well, or we might have a second TIMER flag, requiring both to be set for there to be a HPET and PMTMR. This leaves open the option of Dom0 getting ACPI enabled (despite this then being "real", not emulated ACPI), but TIMER left off. >>> >>> An HPET can exist independently of other features such as ACPI. It >>> should have its own option. >> >> Without ACPI there's no defined way to discover it. Doing what >> Linux does - applying chipset knowledge - won't work on PVH either, >> because there's no emulated chipset. Which would leave scanning >> physical memory, but if there is none, none can be found. >> >>> +1 to having an ACPI option, but as indicated above, I expect it to be >>> used in the longterm even for domU. >> >> Again - why and how? > > I think that at this point in the design it's not so important to have > all the XEN_X86_EMU_* properly defined. This is not a public interface, > so we can expand/reduce them whenever we want. Would it be fine, for the > time being to just have a XEN_X86_EMU_PM and control both the PM and the > PMTMR? I think so, yes. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Julien Grall writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > On 03/11/15 14:18, Stefano Stabellini wrote: > >> +#define set_xen_guest_handle_raw(hnd, val) \ > >> +do {\ > >> +/* Check if the handle is 64-bit (i.e 8-byte) */\ > >> +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); });\ > >> +/* Check if the type of val is compatible with the handle */\ > >> +(void) sizeof((val) != (hnd).p);\ > >> +(hnd).q = (uint64_t)(uintptr_t)(val); \ > >> } while ( 0 ) > >> #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) I hate to throw yet more variables into this, but I had a discussion with some C experts in the pub about this problem. Further thought (including a red herring where someone suggested that memcpy would "launder" the types) results in this proposal: union xen_guest_handle_TYPE { TYPE *p; uint64_t align; }; struct xen_guest_handle__pointer { uint8_t p0,p1,p2,p3; }; /* * void set_xen_guest_handle_raw(xen_guest_handle_TYPE *hnd, TYPE *val); */ #define set_xen_guest_handle_raw(hnd, val) do { void *copy = (val); struct xen_guest_handle__pointer *src = &(hnd); struct xen_guest_handle__pointer *dst = &(xen_xgh_copy); dst->p0 = src->p0; dst->p1 = src->p1; dst->p2 = src->p2; dst->p3 = src->p3; dst[1]->p0 = 0; dst[1]->p1 = 0; dst[1]->p2 = 0; dst[1]->p3 = 0; sizeof((hnd).p == (val)); /* typecheck */ } #define get_xen_guest_handle(hnd) ((hnd).p) The above is legal for the following reasons (C99 6.5): Considering the accesses to src and dst. These are legal according to (7) point 5. (Accesses via a character type are always legal.) After the above macro has been used to set, what is the effective type of hnd for a subsequent get_xen_guest_handle ? If hnd is an object with a declared type (ie, an actual variable, function parameter, or whatever, rather than from malloc), then the effective type is that of hnd, so the effective type is the union, and the effective type of hnd.p is TYPE*. If hnd is not a declared object, then the effective type might be set by one of the stores in (6): store through an non-character lvalue; copy via memcpy or memmove; copy as `an array of character type'. But we store only through character lvalues. We do not store with memcpy or memmove. We do not copy as an array, but as a struct. So none of the rules defining the effective type apply. We are left with the fallback: the effective type of the subsequent read is the type used for access. So a subsequent get's read of hnd.p is legal. With luck the compiler's optimiser will eliminate the temporaries and aggregate the byte-wide stores. In the actual macro all of the struct and union fields and all of the temporary variables should be prefixed with `xen_xgh_' or some such, in case the calling code has (arguably foolishly) #defined src or dst or p or something. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Ian Jackson writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > void *copy = (val); > struct xen_guest_handle__pointer *src = &(hnd); > struct xen_guest_handle__pointer *dst = &(xen_xgh_copy); I have my src and dst the wrong way round, and have used the a variable name from a previous draft. This should be: > struct xen_guest_handle__pointer *src = &(copy); > struct xen_guest_handle__pointer *dst = &(hnd); Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-4.5-testing test] 63532: tolerable FAIL - PUSHED
flight 63532 xen-4.5-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/63532/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 11 guest-start fail REGR. vs. 63437 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 63437 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 63437 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 63437 test-amd64-amd64-xl-rtds 6 xen-boot fail like 63437 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-armhf-armhf-libvirt-qcow2 9 debian-di-installfail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-armhf-armhf-xl-vhd 9 debian-di-installfail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 9 debian-di-installfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass version targeted for testing: xen e0a36c028bd6d9f50982bc2eaacadc0036267e27 baseline version: xen 423d2cd814e8460d5ea8bd191a770f3c48b3947c Last test of basis63437 2015-11-01 07:11:06 Z3 days Testing same since63532 2015-11-03 09:43:38 Z1 days1 attempts People who touched revisions under test: Jan Beulich jobs: 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-rumpuserxen pass build-i386-rumpuserxen pass test-amd64-amd64-xl pass test-armhf-armhf-xl pass test-amd64-i386-xl pass test-amd64-amd64-xl-pvh-amd fail test-amd64-i386-qemut-rhel6hvm-amd pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemut-debianhvm-amd64pass test-amd64-i386-xl-qemut-debianhvm-amd64 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-rumpuserxen-amd64 pass test-amd64-amd64-xl-qemut-win7-amd64 fail test-amd64-i386-xl-qemut-win7-amd64 fail test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-armhf-armhf-xl-arndale
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
>>> On 04.11.15 at 17:22, wrote: > Julien Grall writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro > set_xen_guest_handle_raw"): >> On 03/11/15 14:18, Stefano Stabellini wrote: >> >> +#define set_xen_guest_handle_raw(hnd, val) \ >> >> +do {\ >> >> +/* Check if the handle is 64-bit (i.e 8-byte) */\ >> >> +(void) sizeof(struct { int : -!!(sizeof (hnd) != 8); });\ >> >> +/* Check if the type of val is compatible with the handle */\ >> >> +(void) sizeof((val) != (hnd).p);\ >> >> +(hnd).q = (uint64_t)(uintptr_t)(val); \ >> >> } while ( 0 ) >> >> #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) > > I hate to throw yet more variables into this, but I had a discussion > with some C experts in the pub about this problem. Further thought > (including a red herring where someone suggested that memcpy would > "launder" the types) results in this proposal: > > > union xen_guest_handle_TYPE { > TYPE *p; > uint64_t align; > }; > > struct xen_guest_handle__pointer { > uint8_t p0,p1,p2,p3; > }; > > /* > * void set_xen_guest_handle_raw(xen_guest_handle_TYPE *hnd, TYPE *val); > */ > #define set_xen_guest_handle_raw(hnd, val) > do { > void *copy = (val); > struct xen_guest_handle__pointer *src = &(hnd); > struct xen_guest_handle__pointer *dst = &(xen_xgh_copy); > dst->p0 = src->p0; > dst->p1 = src->p1; > dst->p2 = src->p2; > dst->p3 = src->p3; > dst[1]->p0 = 0; > dst[1]->p1 = 0; > dst[1]->p2 = 0; > dst[1]->p3 = 0; > sizeof((hnd).p == (val)); /* typecheck */ > } > > #define get_xen_guest_handle(hnd) ((hnd).p) > > > The above is legal for the following reasons (C99 6.5): > > Considering the accesses to src and dst. These are legal according to > (7) point 5. (Accesses via a character type are always legal.) > > After the above macro has been used to set, what is the effective type > of hnd for a subsequent get_xen_guest_handle ? All quite interesting, but pretty moot with there not being any get_xen_guest_handle() anymore. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Jan Beulich writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > All quite interesting, but pretty moot with there not being any > get_xen_guest_handle() anymore. If we don't provide a get_xen_guest_handle, a kernel developer will be sorely tempted to make one. If they do and they write it in the obvious way, then the compiler could `deduce' that the top part of the uint64_t store was dead, and eliminate it. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [OSSTEST PATCH v15] Nested HVM
Ian Jackson writes ("[OSSTEST PATCH v14 PART 1 0/9] Nested HVM preparation patches"): > This is the first part of Robert Hu's osstest patch series to support > nested HVM tests. (v13 was passed to me as a git branch `under the > table' by Robert.) I have rewritten the commit messages, refactored a > few patches, and reordered the series slightly. Robert has passed me a tentative version of v15 via `git bundle'. I have made it available here: http://xenbits.xen.org/gitweb/?p=people/iwj/osstest.git;a=shortlog;h=refs/heads/wip.nested-v15 I'm going to fold in the fixup!s, review it, and probably adjust it somewhat. I will call the result v16, even though I am not sending a v15 patchbomb to the list. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
On 04/11/15 16:50, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro > set_xen_guest_handle_raw"): >> All quite interesting, but pretty moot with there not being any >> get_xen_guest_handle() anymore. > > If we don't provide a get_xen_guest_handle, a kernel developer will be > sorely tempted to make one. > > If they do and they write it in the obvious way, then the compiler > could `deduce' that the top part of the uint64_t store was dead, and > eliminate it. The developers could also decide to rewrite the set_xen_raw_guest_handle/get_xen_guest_handle in an obvious way because they think ours it's too complicate... See the Linux version. TBH, we can't protect ourself for a developer writing it own macro and don't think about the big picture. The comment in the header pretty much explain the constraint. If it doesn't read, well it's not our business. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [V9 1/3] x86/xsaves: enable xsaves/xrstors/xsavec in xen
>>> On 03.11.15 at 07:27, wrote: > This patch uses xsaves/xrstors/xsavec instead of xsaveopt/xrstor > to perform the xsave_area switching so that xen itself > can benefit from them when available. > > For xsaves/xrstors/xsavec only use compact format. Add format conversion > support when perform guest os migration. > > Also, pv guest will not support xsaves/xrstors. > > Signed-off-by: Shuai Ruan > --- > xen/arch/x86/domain.c| 7 ++ > xen/arch/x86/domctl.c| 30 +- > xen/arch/x86/hvm/hvm.c | 18 +++- > xen/arch/x86/i387.c | 4 + > xen/arch/x86/traps.c | 6 +- > xen/arch/x86/xstate.c| 242 > +-- > xen/include/asm-x86/xstate.h | 2 + > 7 files changed, 265 insertions(+), 44 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index fe3be30..108d4f8 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -883,7 +883,12 @@ int arch_set_info_guest( > { > memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt)); > if ( v->arch.xsave_area ) > +{ > v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE; > + if ( cpu_has_xsaves || cpu_has_xsavec ) > + v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_FP_SSE | > + > XSTATE_COMPACTION_ENABLED; > +} So here you nicely extend the existing conditional. > @@ -1568,6 +1573,8 @@ static void __context_switch(void) > if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) ) > BUG(); > } > +if ( cpu_has_xsaves && has_hvm_container_vcpu(n) ) > +set_msr_xss(n->arch.hvm_vcpu.msr_xss); Why not also here (the previous if() uses cpu_has_xsave, which you surely depend on)? Agreed the difference is minor for modern CPUs, but I wanted to ask anyway. I.e. an explanation will do, no need to re-submit just because of this. > @@ -158,6 +334,20 @@ void xsave(struct vcpu *v, uint64_t mask) > ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = word_size; > } > > +#define XSTATE_FIXUP ".section .fixup,\"ax\" \n"\ > + "2: mov %5,%%ecx \n"\ > + " xor %1,%1\n"\ > + " rep stosb\n"\ > + " lea %2,%0\n"\ > + " mov %3,%1\n"\ > + " jmp 1b \n"\ > + ".previous \n"\ > + _ASM_EXTABLE(1b, 2b)\ > + : "+&D" (ptr), "+&a" (lmask)\ > + : "m" (*ptr), "g" (lmask), "d" (hmask), \ > + "m" (xsave_cntxt_size)\ > + : "ecx" > + > void xrstor(struct vcpu *v, uint64_t mask) > { > uint32_t hmask = mask >> 32; > @@ -187,39 +377,22 @@ void xrstor(struct vcpu *v, uint64_t mask) > switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) ) > { > default: > -asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n" > - ".section .fixup,\"ax\" \n" > - "2: mov %5,%%ecx \n" > - " xor %1,%1\n" > - " rep stosb\n" > - " lea %2,%0\n" > - " mov %3,%1\n" > - " jmp 1b \n" > - ".previous \n" > - _ASM_EXTABLE(1b, 2b) > - : "+&D" (ptr), "+&a" (lmask) > - : "m" (*ptr), "g" (lmask), "d" (hmask), > - "m" (xsave_cntxt_size) > - : "ecx" ); > +alternative_input("1: "".byte 0x48,0x0f,0xae,0x2f", > + ".byte 0x48,0x0f,0xc7,0x1f", > + X86_FEATURE_XSAVES, > + "D" (ptr), "m" (*ptr), "a" (lmask), "d" (hmask)); > +asm volatile (XSTATE_FIXUP); > break; > case 4: case 2: > -asm volatile ( "1: .byte 0x0f,0xae,0x2f\n" > - ".section .fixup,\"ax\" \n" > - "2: mov %5,%%ecx\n" > - " xor %1,%1 \n" > - " rep stosb \n" > - " lea %2,%0 \n" > - " mov %3,%1 \n" > - " jmp 1b \n" > - ".previous \n" > - _ASM_EXTABLE(1b, 2b) > - : "+&D" (ptr), "+&a" (lmask) > - : "m" (*ptr), "g" (lmask), "d"
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
>>> On 04.11.15 at 17:50, wrote: > Jan Beulich writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro > set_xen_guest_handle_raw"): >> All quite interesting, but pretty moot with there not being any >> get_xen_guest_handle() anymore. > > If we don't provide a get_xen_guest_handle, a kernel developer will be > sorely tempted to make one. What use would it be to them? Kernels only write handles, they shouldn't have a need for reading them. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw
Jan Beulich writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"): > On 04.11.15 at 17:50, wrote: > > If we don't provide a get_xen_guest_handle, a kernel developer will be > > sorely tempted to make one. > > What use would it be to them? Kernels only write handles, they > shouldn't have a need for reading them. I foresee situations where a kernel might like to update a proposed hypercall argument structure in place, which might involve reading the handles. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/6] xen: sched: fix locking of {insert, remove}_vcpu()
Hi, This series, improves how inserting vCPUs in schedulers runqueues is done, including fixing a couple of bugs, and paving the way for more improvement in Credit2 runqueue handling (will be submitted as a separate series). v3 is here: http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg03278.html Message-Id: <20151029225158.25219.4625.stgit@Solace.station> Only patch 1 really changed from v3, and patches 1 and 2 are the only one that seem to me to be missing suitable Ack-s. There is a git branch for this series here: git://xenbits.xen.org/people/dariof/xen.git rel/sched/fix-vcpu-ins-rem-v4 Regards, Dario --- Dario Faggioli (6): xen: sched: fix locking of remove_vcpu() in credit1 xen: sched: fix locking for insert_vcpu() in credit1 and RTDS xen: sched: clarify use cases of schedule_cpu_switch() xen: sched: better handle (not) inserting idle vCPUs in runqueues xen: sched: get rid of the per domain vCPU list in RTDS xen: sched: get rid of the per domain vCPU list in Credit2 xen/common/cpupool.c |7 - xen/common/sched_credit.c | 18 - xen/common/sched_credit2.c | 55 ++-- xen/common/sched_rt.c | 61 ++-- xen/common/schedule.c | 57 +++-- 5 files changed, 103 insertions(+), 95 deletions(-) -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 6/6] xen: sched: get rid of the per domain vCPU list in Credit2
As, curently, there is no reason for bothering having it and keeping it updated. In fact, it is only used for dumping and changing vCPUs parameters, but that can be achieved easily with for_each_vcpu. While there, improve alignment of comments, ad add a const qualifier to a pointer, making things more consistent with what happens everywhere else in the source file. This also allows us to kill one of the remaining FIXMEs in the code, which is always good. Signed-off-by: Dario Faggioli Acked-by: George Dunlap --- Cc: Andrew Cooper --- Changes from v1: * removed spurious hunk about sched_rt.c, as noted during review; * fixed the BUG_ON inside csched2_dom_destroy(), as noted during review; * used 'v' instead of 'vc' for 'vcpu' (in new code), as suggested during review. --- xen/common/sched_credit2.c | 34 +++--- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 556ca0f..3c49ffa 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -234,9 +234,8 @@ struct csched2_private { * Virtual CPU */ struct csched2_vcpu { -struct list_head rqd_elem; /* On the runqueue data list */ -struct list_head sdom_elem; /* On the domain vcpu list */ -struct list_head runq_elem; /* On the runqueue */ +struct list_head rqd_elem; /* On the runqueue data list */ +struct list_head runq_elem;/* On the runqueue*/ struct csched2_runqueue_data *rqd; /* Up-pointer to the runqueue */ /* Up-pointers */ @@ -261,7 +260,6 @@ struct csched2_vcpu { * Domain */ struct csched2_dom { -struct list_head vcpu; struct list_head sdom_elem; struct domain *dom; uint16_t weight; @@ -770,7 +768,6 @@ csched2_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd) return NULL; INIT_LIST_HEAD(&svc->rqd_elem); -INIT_LIST_HEAD(&svc->sdom_elem); INIT_LIST_HEAD(&svc->runq_elem); svc->sdom = dd; @@ -874,9 +871,6 @@ csched2_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) BUG_ON(is_idle_vcpu(vc)); -/* FIXME: Do we need the private lock here? */ -list_add_tail(&svc->sdom_elem, &svc->sdom->vcpu); - /* Add vcpu to runqueue of initial processor */ lock = vcpu_schedule_lock_irq(vc); @@ -921,10 +915,6 @@ csched2_vcpu_remove(const struct scheduler *ops, struct vcpu *vc) vcpu_schedule_unlock_irq(lock, vc); -/* Remove from sdom list. Don't need a lock for this, as it's called - * syncronously when nothing else can happen. */ -list_del_init(&svc->sdom_elem); - svc->sdom->nr_vcpus--; } } @@ -1441,7 +1431,7 @@ csched2_dom_cntl( if ( op->u.credit2.weight != 0 ) { -struct list_head *iter; +struct vcpu *v; int old_weight; old_weight = sdom->weight; @@ -1449,9 +1439,9 @@ csched2_dom_cntl( sdom->weight = op->u.credit2.weight; /* Update weights for vcpus, and max_weight for runqueues on which they reside */ -list_for_each ( iter, &sdom->vcpu ) +for_each_vcpu ( d, v ) { -struct csched2_vcpu *svc = list_entry(iter, struct csched2_vcpu, sdom_elem); +struct csched2_vcpu *svc = CSCHED2_VCPU(v); /* NB: Locking order is important here. Because we grab this lock here, we * must never lock csched2_priv.lock if we're holding a runqueue lock. @@ -1485,7 +1475,6 @@ csched2_alloc_domdata(const struct scheduler *ops, struct domain *dom) return NULL; /* Initialize credit and weight */ -INIT_LIST_HEAD(&sdom->vcpu); INIT_LIST_HEAD(&sdom->sdom_elem); sdom->dom = dom; sdom->weight = CSCHED2_DEFAULT_WEIGHT; @@ -1537,9 +1526,7 @@ csched2_free_domdata(const struct scheduler *ops, void *data) static void csched2_dom_destroy(const struct scheduler *ops, struct domain *dom) { -struct csched2_dom *sdom = CSCHED2_DOM(dom); - -BUG_ON(!list_empty(&sdom->vcpu)); +BUG_ON(CSCHED2_DOM(dom)->nr_vcpus > 0); csched2_free_domdata(ops, CSCHED2_DOM(dom)); } @@ -1879,7 +1866,7 @@ csched2_dump_pcpu(const struct scheduler *ops, int cpu) static void csched2_dump(const struct scheduler *ops) { -struct list_head *iter_sdom, *iter_svc; +struct list_head *iter_sdom; struct csched2_private *prv = CSCHED2_PRIV(ops); unsigned long flags; int i, loop; @@ -1924,6 +1911,8 @@ csched2_dump(const struct scheduler *ops) list_for_each( iter_sdom, &prv->sdom ) { struct csched2_dom *sdom; +struct vcpu *v; + sdom = list_entry(iter_sdom, struct csched2_dom, sdom_elem); printk("\tDomain: %d w %d v %d\n", @@ -1931,12 +1920,11 @@ csched2_dump(const struct scheduler *ops) sdom->weight, sdom->nr_vcpus); -
[Xen-devel] [PATCH v4 3/6] xen: sched: clarify use cases of schedule_cpu_switch()
schedule_cpu_switch() is meant to be only used for moving pCPUs from a cpupool to no cpupool, and from there back to a cpupool, *not* to move them directly from one cpupool to another. This is something inherent to the way the function is implemented and called, but is not that clear, just by the look of it. Make it more evident by: - adding commentary and ASSERT()s; - update the cpupool per-CPU variable (mapping pCPUs to pools) directly in schedule_cpu_switch(), rather than in various places in cpupool.c. Signed-off-by: Dario Faggioli Acked-by: Juergen Gross Reviewed-by: George Dunlap --- Cc: Jan Beulich --- Changes from v2: * updating of per_cpu(cpupool, cpu) is now done in schedule_cpu_switch(), as suggested during review. Changes from v1: * new patch, was not there in v1; --- xen/common/cpupool.c |7 --- xen/common/schedule.c | 31 ++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c index e79850b..8e7b723 100644 --- a/xen/common/cpupool.c +++ b/xen/common/cpupool.c @@ -261,19 +261,13 @@ int cpupool_move_domain(struct domain *d, struct cpupool *c) static int cpupool_assign_cpu_locked(struct cpupool *c, unsigned int cpu) { int ret; -struct cpupool *old; struct domain *d; if ( (cpupool_moving_cpu == cpu) && (c != cpupool_cpu_moving) ) return -EBUSY; -old = per_cpu(cpupool, cpu); -per_cpu(cpupool, cpu) = c; ret = schedule_cpu_switch(cpu, c); if ( ret ) -{ -per_cpu(cpupool, cpu) = old; return ret; -} cpumask_clear_cpu(cpu, &cpupool_free_cpus); if (cpupool_moving_cpu == cpu) @@ -326,7 +320,6 @@ static long cpupool_unassign_cpu_helper(void *info) cpumask_clear_cpu(cpu, &cpupool_free_cpus); goto out; } -per_cpu(cpupool, cpu) = NULL; cpupool_moving_cpu = -1; cpupool_put(cpupool_cpu_moving); cpupool_cpu_moving = NULL; diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 88e456f..1512d79 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -1486,6 +1486,17 @@ void __init scheduler_init(void) BUG(); } +/* + * Move a pCPU outside of the influence of the scheduler of its current + * cpupool, or subject it to the scheduler of a new cpupool. + * + * For the pCPUs that are removed from their cpupool, their scheduler becomes + * &ops (the default scheduler, selected at boot, which also services the + * default cpupool). However, as these pCPUs are not really part of any pool, + * there won't be any scheduling event on them, not even from the default + * scheduler. Basically, they will just sit idle until they are explicitly + * added back to a cpupool. + */ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) { struct vcpu *idle; @@ -1493,9 +1504,24 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) void *ppriv, *ppriv_old, *vpriv, *vpriv_old; struct scheduler *old_ops = per_cpu(scheduler, cpu); struct scheduler *new_ops = (c == NULL) ? &ops : c->sched; +struct cpupool *old_pool = per_cpu(cpupool, cpu); + +/* + * pCPUs only move from a valid cpupool to free (i.e., out of any pool), + * or from free to a valid cpupool. In the former case (which happens when + * c is NULL), we want the CPU to have been marked as free already, as + * well as to not be valid for the source pool any longer, when we get to + * here. In the latter case (which happens when c is a valid cpupool), we + * want the CPU to still be marked as free, as well as to not yet be valid + * for the destination pool. + */ +ASSERT(c != old_pool && (c != NULL || old_pool != NULL)); +ASSERT(cpumask_test_cpu(cpu, &cpupool_free_cpus)); +ASSERT((c == NULL && !cpumask_test_cpu(cpu, old_pool->cpu_valid)) || + (c != NULL && !cpumask_test_cpu(cpu, c->cpu_valid))); if ( old_ops == new_ops ) -return 0; +goto out; idle = idle_vcpu[cpu]; ppriv = SCHED_OP(new_ops, alloc_pdata, cpu); @@ -1523,6 +1549,9 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) SCHED_OP(old_ops, free_vdata, vpriv_old); SCHED_OP(old_ops, free_pdata, ppriv_old, cpu); + out: +per_cpu(cpupool, cpu) = c; + return 0; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS
The insert_vcpu() hook is handled with inconsistent locking. In fact, schedule_cpu_switch() calls the hook with runqueue lock held, while sched_move_domain() relies on the hook implementations to take the lock themselves (and, since that is not done in Credit1 and RTDS, such operation is not safe in those cases). This is fixed as follows: - take the lock in the hook implementations, in specific schedulers' code; - avoid calling insert_vcpu(), for the idle vCPU, in schedule_cpu_switch(). In fact, idle vCPUs are set to run immediately, and the various schedulers won't insert them in their runqueues anyway, even when explicitly asked to. While there, still in schedule_cpu_switch(), locking with _irq() is enough (there's no need to do *_irqsave()). Signed-off-by: Dario Faggioli Reviewed-by: Meng Xu --- Cc: George Dunlap Cc: Andrew Cooper Cc: Jan Beulich --- Cahnges from v2: * locking in schedule_cpu_switch() is left in place (but turned to just _irq(), instead than *_irqsave()); * call to insert_vcpu() in schedule_cpu_switch() is removed in this patch (rather than later in the series). Changes from v1 (of this series): * in Credit1, the lock wants to be an _irqsave() one. If not, the ASSERT() in _spin_lock_irq() will trigger when the hook is called, during boot, from sched_init_vcpu(); * reprhased the changelog (to be less verbose); * add a few words, in the changelog, about why it is safe to get rid of the locking in schedule_cpu_switch(). Proper commentary and ASSERT()-s about that will come in another patch. Changes from the other series: * split the patch (wrt the original patch, in the original series), and take care, in this one, only of insert_vcpu(); --- xen/common/sched_credit.c |6 ++ xen/common/sched_rt.c |3 +++ xen/common/schedule.c |6 ++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 6dfcff6..0e26986 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -911,10 +911,16 @@ static void csched_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) { struct csched_vcpu *svc = vc->sched_priv; +spinlock_t *lock; +unsigned long flags; + +lock = vcpu_schedule_lock_irqsave(vc, &flags); if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc->is_running ) __runq_insert(vc->processor, svc); +vcpu_schedule_unlock_irqrestore(lock, flags, vc); + SCHED_STAT_CRANK(vcpu_insert); } diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index 822f23c..3a66c9a 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -622,16 +622,19 @@ rt_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) { struct rt_vcpu *svc = rt_vcpu(vc); s_time_t now = NOW(); +spinlock_t *lock; /* not addlocate idle vcpu to dom vcpu list */ if ( is_idle_vcpu(vc) ) return; +lock = vcpu_schedule_lock_irq(vc); if ( now >= svc->cur_deadline ) rt_update_deadline(now, svc); if ( !__vcpu_on_q(svc) && vcpu_runnable(vc) && !vc->is_running ) __runq_insert(ops, svc); +vcpu_schedule_unlock_irq(lock, vc); /* add rt_vcpu svc to scheduler-specific vcpu list of the dom */ list_add_tail(&svc->sdom_elem, &svc->sdom->vcpu); diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 292e9a0..88e456f 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -1488,7 +1488,6 @@ void __init scheduler_init(void) int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) { -unsigned long flags; struct vcpu *idle; spinlock_t *lock; void *ppriv, *ppriv_old, *vpriv, *vpriv_old; @@ -1509,7 +1508,7 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) return -ENOMEM; } -lock = pcpu_schedule_lock_irqsave(cpu, &flags); +lock = pcpu_schedule_lock_irq(cpu); SCHED_OP(old_ops, tick_suspend, cpu); vpriv_old = idle->sched_priv; @@ -1518,9 +1517,8 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) ppriv_old = per_cpu(schedule_data, cpu).sched_priv; per_cpu(schedule_data, cpu).sched_priv = ppriv; SCHED_OP(new_ops, tick_resume, cpu); -SCHED_OP(new_ops, insert_vcpu, idle); -pcpu_schedule_unlock_irqrestore(lock, flags, cpu); +pcpu_schedule_unlock_irq(lock, cpu); SCHED_OP(old_ops, free_vdata, vpriv_old); SCHED_OP(old_ops, free_pdata, ppriv_old, cpu); ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 1/6] xen: sched: fix locking of remove_vcpu() in credit1
In fact, csched_vcpu_remove() (i.e., the credit1 implementation of remove_vcpu()) manipulates runqueues, so holding the runqueue lock is necessary. However, the vCPU just can't be on the runqueue, when the function is called. We can therefore ASSERT() that, and avoid doing any runqueue manipulations (rather than adding the runqueue locking around it). Also, while there, *_lock_irq() (for the private lock) is enough, there is no need to *_lock_irqsave(). Signed-off-by: Dario Faggioli --- Cc: George Dunlap Cc: Andrew Cooper Cc: Jan Beulich --- Changes from v3: * instead of adding locking, get rid of __runq_remove(), and add an ASSERT() about vCPU not being in runq already, as suggested during review. Changes from the other series: * split the patch (wrt the original patch, in the original series), and take care, in this one, only of remove_vcpu(); * removed pointless parentheses. --- The fact that vCPU can't be in runqueue when calling remove_vcpu() is true for other schedulers as well. In them, though, there isn't any race condition to fix. Therefore, taking care of the other schedulers will happen in a followup series. --- xen/common/sched_credit.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 1b30e67..6dfcff6 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -934,28 +934,25 @@ csched_vcpu_remove(const struct scheduler *ops, struct vcpu *vc) struct csched_private *prv = CSCHED_PRIV(ops); struct csched_vcpu * const svc = CSCHED_VCPU(vc); struct csched_dom * const sdom = svc->sdom; -unsigned long flags; SCHED_STAT_CRANK(vcpu_remove); +ASSERT(!__vcpu_on_runq(svc)); + if ( test_and_clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) { SCHED_STAT_CRANK(vcpu_unpark); vcpu_unpause(svc->vcpu); } -if ( __vcpu_on_runq(svc) ) -__runq_remove(svc); - -spin_lock_irqsave(&(prv->lock), flags); +spin_lock_irq(&prv->lock); if ( !list_empty(&svc->active_vcpu_elem) ) __csched_vcpu_acct_stop_locked(prv, svc); -spin_unlock_irqrestore(&(prv->lock), flags); +spin_unlock_irq(&prv->lock); BUG_ON( sdom == NULL ); -BUG_ON( !list_empty(&svc->runq_elem) ); } static void ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 4/6] xen: sched: better handle (not) inserting idle vCPUs in runqueues
Idle vCPUs are set to run immediately, as a part of their own initialization, so we shouldn't even try to put them in a runqueue. In fact, no scheduler does that, even when asked to (that is rather explicit in Credit2 and RTDS, a bit less evident in Credit1). Let's make things look as follows: - in generic code, explicitly avoid even trying to insert idle vCPUs in runqueues; - in specific schedulers' code, enforce that. Note that, as csched_vcpu_insert() is no longer being called, during boot (from sched_init_vcpu()) we can safely avoid saving the flags when taking the runqueue lock. Signed-off-by: Dario Faggioli Acked-by: George Dunlap Reviewed-by: Juergen Gross --- Changes from v1: * changelog: updated and made a little bit less verbose. --- xen/common/sched_credit.c |7 --- xen/common/sched_credit2.c | 25 ++--- xen/common/sched_rt.c |4 +--- xen/common/schedule.c | 20 +++- 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 0e26986..021a9dd 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -912,14 +912,15 @@ csched_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) { struct csched_vcpu *svc = vc->sched_priv; spinlock_t *lock; -unsigned long flags; -lock = vcpu_schedule_lock_irqsave(vc, &flags); +BUG_ON( is_idle_vcpu(vc) ); + +lock = vcpu_schedule_lock_irq(vc); if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc->is_running ) __runq_insert(vc->processor, svc); -vcpu_schedule_unlock_irqrestore(lock, flags, vc); +vcpu_schedule_unlock_irq(lock, vc); SCHED_STAT_CRANK(vcpu_insert); } diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index fc51a75..556ca0f 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -868,30 +868,25 @@ csched2_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) { struct csched2_vcpu *svc = vc->sched_priv; struct csched2_dom * const sdom = svc->sdom; +spinlock_t *lock; printk("%s: Inserting %pv\n", __func__, vc); -/* NB: On boot, idle vcpus are inserted before alloc_pdata() has - * been called for that cpu. - */ -if ( ! is_idle_vcpu(vc) ) -{ -spinlock_t *lock; +BUG_ON(is_idle_vcpu(vc)); -/* FIXME: Do we need the private lock here? */ -list_add_tail(&svc->sdom_elem, &svc->sdom->vcpu); +/* FIXME: Do we need the private lock here? */ +list_add_tail(&svc->sdom_elem, &svc->sdom->vcpu); -/* Add vcpu to runqueue of initial processor */ -lock = vcpu_schedule_lock_irq(vc); +/* Add vcpu to runqueue of initial processor */ +lock = vcpu_schedule_lock_irq(vc); -runq_assign(ops, vc); +runq_assign(ops, vc); -vcpu_schedule_unlock_irq(lock, vc); +vcpu_schedule_unlock_irq(lock, vc); -sdom->nr_vcpus++; +sdom->nr_vcpus++; -SCHED_STAT_CRANK(vcpu_insert); -} +SCHED_STAT_CRANK(vcpu_insert); CSCHED2_VCPU_CHECK(vc); } diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index 3a66c9a..cbe7b17 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -624,9 +624,7 @@ rt_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) s_time_t now = NOW(); spinlock_t *lock; -/* not addlocate idle vcpu to dom vcpu list */ -if ( is_idle_vcpu(vc) ) -return; +BUG_ON( is_idle_vcpu(vc) ); lock = vcpu_schedule_lock_irq(vc); if ( now >= svc->cur_deadline ) diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 1512d79..e3582d1 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -240,20 +240,22 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor) init_timer(&v->poll_timer, poll_timer_fn, v, v->processor); -/* Idle VCPUs are scheduled immediately. */ +v->sched_priv = SCHED_OP(DOM2OP(d), alloc_vdata, v, d->sched_priv); +if ( v->sched_priv == NULL ) +return 1; + +TRACE_2D(TRC_SCHED_DOM_ADD, v->domain->domain_id, v->vcpu_id); + +/* Idle VCPUs are scheduled immediately, so don't put them in runqueue. */ if ( is_idle_domain(d) ) { per_cpu(schedule_data, v->processor).curr = v; v->is_running = 1; } - -TRACE_2D(TRC_SCHED_DOM_ADD, v->domain->domain_id, v->vcpu_id); - -v->sched_priv = SCHED_OP(DOM2OP(d), alloc_vdata, v, d->sched_priv); -if ( v->sched_priv == NULL ) -return 1; - -SCHED_OP(DOM2OP(d), insert_vcpu, v); +else +{ +SCHED_OP(DOM2OP(d), insert_vcpu, v); +} return 0; } ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 5/6] xen: sched: get rid of the per domain vCPU list in RTDS
As, curently, there is no reason for bothering having it and keeping it updated. In fact, it is only used for dumping and changing vCPUs parameters, but that can be achieved easily with for_each_vcpu. While there, take care of the case when XEN_DOMCTL_SCHEDOP_getinfo is called but no vCPUs have been allocated yet (by returning the default scheduling parameters). Signed-off-by: Dario Faggioli Reviewed-by: Meng Xu --- Cc: George Dunlap Cc: Andrew Cooper --- Changes from v1: * address the case when d->vcpu[] has not been allocated yet, as requested during review; * style: space before brackets of for_each_vcpu, as requested during review; * used 'v' instead of 'vc' for 'vcpu' (in new code). --- xen/common/sched_rt.c | 54 - 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index cbe7b17..3f1d047 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -160,7 +160,6 @@ struct rt_private { */ struct rt_vcpu { struct list_head q_elem;/* on the runq/depletedq list */ -struct list_head sdom_elem; /* on the domain VCPU list */ /* Up-pointers */ struct rt_dom *sdom; @@ -182,7 +181,6 @@ struct rt_vcpu { * Domain */ struct rt_dom { -struct list_head vcpu; /* link its VCPUs */ struct list_head sdom_elem; /* link list on rt_priv */ struct domain *dom; /* pointer to upper domain */ }; @@ -290,7 +288,7 @@ rt_dump_pcpu(const struct scheduler *ops, int cpu) static void rt_dump(const struct scheduler *ops) { -struct list_head *iter_sdom, *iter_svc, *runq, *depletedq, *iter; +struct list_head *runq, *depletedq, *iter; struct rt_private *prv = rt_priv(ops); struct rt_vcpu *svc; struct rt_dom *sdom; @@ -319,14 +317,16 @@ rt_dump(const struct scheduler *ops) } printk("Domain info:\n"); -list_for_each( iter_sdom, &prv->sdom ) +list_for_each( iter, &prv->sdom ) { -sdom = list_entry(iter_sdom, struct rt_dom, sdom_elem); +struct vcpu *v; + +sdom = list_entry(iter, struct rt_dom, sdom_elem); printk("\tdomain: %d\n", sdom->dom->domain_id); -list_for_each( iter_svc, &sdom->vcpu ) +for_each_vcpu ( sdom->dom, v ) { -svc = list_entry(iter_svc, struct rt_vcpu, sdom_elem); +svc = rt_vcpu(v); rt_dump_vcpu(ops, svc); } } @@ -527,7 +527,6 @@ rt_alloc_domdata(const struct scheduler *ops, struct domain *dom) if ( sdom == NULL ) return NULL; -INIT_LIST_HEAD(&sdom->vcpu); INIT_LIST_HEAD(&sdom->sdom_elem); sdom->dom = dom; @@ -587,7 +586,6 @@ rt_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd) return NULL; INIT_LIST_HEAD(&svc->q_elem); -INIT_LIST_HEAD(&svc->sdom_elem); svc->flags = 0U; svc->sdom = dd; svc->vcpu = vc; @@ -614,8 +612,7 @@ rt_free_vdata(const struct scheduler *ops, void *priv) * This function is called in sched_move_domain() in schedule.c * When move a domain to a new cpupool. * It inserts vcpus of moving domain to the scheduler's RunQ in - * dest. cpupool; and insert rt_vcpu svc to scheduler-specific - * vcpu list of the dom + * dest. cpupool. */ static void rt_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) @@ -634,15 +631,11 @@ rt_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) __runq_insert(ops, svc); vcpu_schedule_unlock_irq(lock, vc); -/* add rt_vcpu svc to scheduler-specific vcpu list of the dom */ -list_add_tail(&svc->sdom_elem, &svc->sdom->vcpu); - SCHED_STAT_CRANK(vcpu_insert); } /* - * Remove rt_vcpu svc from the old scheduler in source cpupool; and - * Remove rt_vcpu svc from scheduler-specific vcpu list of the dom + * Remove rt_vcpu svc from the old scheduler in source cpupool. */ static void rt_vcpu_remove(const struct scheduler *ops, struct vcpu *vc) @@ -659,9 +652,6 @@ rt_vcpu_remove(const struct scheduler *ops, struct vcpu *vc) if ( __vcpu_on_q(svc) ) __q_remove(svc); vcpu_schedule_unlock_irq(lock, vc); - -if ( !is_idle_vcpu(vc) ) -list_del_init(&svc->sdom_elem); } /* @@ -1135,20 +1125,28 @@ rt_dom_cntl( struct xen_domctl_scheduler_op *op) { struct rt_private *prv = rt_priv(ops); -struct rt_dom * const sdom = rt_dom(d); struct rt_vcpu *svc; -struct list_head *iter; +struct vcpu *v; unsigned long flags; int rc = 0; switch ( op->cmd ) { case XEN_DOMCTL_SCHEDOP_getinfo: -spin_lock_irqsave(&prv->lock, flags); -svc = list_entry(sdom->vcpu.next, struct rt_vcpu, sdom_elem); -op->u.rtds.period = svc->period / MICROSECS(1); /* transfer to us */ -op->u.rtds.budget = svc->budget / MICROSECS(1); -spin_unlock_irqrestore(&prv->lock, flags); +if ( d->max_vcpus > 0 ) +{ +sp
Re: [Xen-devel] [PATCH v4 1/6] xen: sched: fix locking of remove_vcpu() in credit1
On 04/11/15 17:17, Dario Faggioli wrote: > In fact, csched_vcpu_remove() (i.e., the credit1 > implementation of remove_vcpu()) manipulates runqueues, > so holding the runqueue lock is necessary. > > However, the vCPU just can't be on the runqueue, when > the function is called. We can therefore ASSERT() that, > and avoid doing any runqueue manipulations (rather than > adding the runqueue locking around it). > > Also, while there, *_lock_irq() (for the private lock) is > enough, there is no need to *_lock_irqsave(). > > Signed-off-by: Dario Faggioli Reviewed-by: George Dunlap > --- > Cc: George Dunlap > Cc: Andrew Cooper > Cc: Jan Beulich > --- > Changes from v3: > * instead of adding locking, get rid of __runq_remove(), >and add an ASSERT() about vCPU not being in runq already, >as suggested during review. > > Changes from the other series: > * split the patch (wrt the original patch, in the original >series), and take care, in this one, only of remove_vcpu(); > * removed pointless parentheses. > --- > The fact that vCPU can't be in runqueue when calling remove_vcpu() is true for > other schedulers as well. In them, though, there isn't any race condition to > fix. Therefore, taking care of the other schedulers will happen in a followup > series. > --- > xen/common/sched_credit.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > index 1b30e67..6dfcff6 100644 > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -934,28 +934,25 @@ csched_vcpu_remove(const struct scheduler *ops, struct > vcpu *vc) > struct csched_private *prv = CSCHED_PRIV(ops); > struct csched_vcpu * const svc = CSCHED_VCPU(vc); > struct csched_dom * const sdom = svc->sdom; > -unsigned long flags; > > SCHED_STAT_CRANK(vcpu_remove); > > +ASSERT(!__vcpu_on_runq(svc)); > + > if ( test_and_clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) > { > SCHED_STAT_CRANK(vcpu_unpark); > vcpu_unpause(svc->vcpu); > } > > -if ( __vcpu_on_runq(svc) ) > -__runq_remove(svc); > - > -spin_lock_irqsave(&(prv->lock), flags); > +spin_lock_irq(&prv->lock); > > if ( !list_empty(&svc->active_vcpu_elem) ) > __csched_vcpu_acct_stop_locked(prv, svc); > > -spin_unlock_irqrestore(&(prv->lock), flags); > +spin_unlock_irq(&prv->lock); > > BUG_ON( sdom == NULL ); > -BUG_ON( !list_empty(&svc->runq_elem) ); > } > > static void > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 63615: tolerable all pass - PUSHED
flight 63615 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/63615/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass version targeted for testing: xen 26d4eebee81e5537dc2a04b57968ff3afe35e446 baseline version: xen 6919a7dde48bf1a9314c328bfb93a8a2f741bb80 Last test of basis63594 2015-11-04 12:00:59 Z0 days Testing same since63615 2015-11-04 16:01:29 Z0 days1 attempts People who touched revisions under test: Dario Faggioli Ian Campbell Ian Jackson Ross Lagerwall Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=26d4eebee81e5537dc2a04b57968ff3afe35e446 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 26d4eebee81e5537dc2a04b57968ff3afe35e446 + branch=xen-unstable-smoke + revision=26d4eebee81e5537dc2a04b57968ff3afe35e446 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-unstable + '[' x26d4eebee81e5537dc2a04b57968ff3afe35e446 = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print $c{"GitCacheProxy"} or die $!; ' +++ local cache=git://cache:9419/ +++ '[' xgit://cache:
Re: [Xen-devel] Linux 4.4 MW: Boot under Xen fails with CONFIG_DEBUG_WX enabled: RIP: ptdump_walk_pgd_level_core
* Stephen Smalley wrote: > On 11/04/2015 06:55 AM, Sander Eikelenboom wrote: > >Hi All, > > > >I just tried to boot with the current linus mergewindow tree under Xen. > >It fails with a kernel panic at boot with the new "CONFIG_DEBUG_WX" > >option enabled. > >Disabling it makes the kernel boot fine. > > > >The splat: > >[ 18.424241] Freeing unused kernel memory: 1104K (822fc000 - > >8241) > >[ 18.430314] Write protecting the kernel read-only data: 18432k > >[ 18.441054] Freeing unused kernel memory: 1144K (880001ae2000 - > >880001c0) > >[ 18.447966] Freeing unused kernel memory: 1560K (88000207a000 - > >88000220) > >[ 18.453947] BUG: unable to handle kernel paging request at > >88055c883000 > >[ 18.459943] IP: [] > >ptdump_walk_pgd_level_core+0x20e/0x440 > >[ 18.465847] PGD 2212067 PUD 0 > >[ 18.471564] Oops: [#1] SMP > >[ 18.477248] Modules linked in: > >[ 18.482918] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > >4.3.0-mw-20151104-linus-doflr+ #1 > >[ 18.488804] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS > >V1.8B1 09/13/2010 > >[ 18.494778] task: 880059b9 ti: 880059b98000 task.ti: > >880059b98000 > >[ 18.500852] RIP: e030:[] [] > >ptdump_walk_pgd_level_core+0x20e/0x440 It would be nice to see which line of code this corresponds to. Doing this: gdb vmlinux list *0x8105af8e should normally do the trick. Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Linux 4.4 MW: Boot under Xen fails with CONFIG_DEBUG_WX enabled: RIP: ptdump_walk_pgd_level_core
On 2015-11-04 19:06, Ingo Molnar wrote: * Stephen Smalley wrote: On 11/04/2015 06:55 AM, Sander Eikelenboom wrote: >Hi All, > >I just tried to boot with the current linus mergewindow tree under Xen. >It fails with a kernel panic at boot with the new "CONFIG_DEBUG_WX" >option enabled. >Disabling it makes the kernel boot fine. > >The splat: >[ 18.424241] Freeing unused kernel memory: 1104K (822fc000 - >8241) >[ 18.430314] Write protecting the kernel read-only data: 18432k >[ 18.441054] Freeing unused kernel memory: 1144K (880001ae2000 - >880001c0) >[ 18.447966] Freeing unused kernel memory: 1560K (88000207a000 - >88000220) >[ 18.453947] BUG: unable to handle kernel paging request at >88055c883000 >[ 18.459943] IP: [] >ptdump_walk_pgd_level_core+0x20e/0x440 >[ 18.465847] PGD 2212067 PUD 0 >[ 18.471564] Oops: [#1] SMP >[ 18.477248] Modules linked in: >[ 18.482918] CPU: 2 PID: 1 Comm: swapper/0 Not tainted >4.3.0-mw-20151104-linus-doflr+ #1 >[ 18.488804] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS >V1.8B1 09/13/2010 >[ 18.494778] task: 880059b9 ti: 880059b98000 task.ti: >880059b98000 >[ 18.500852] RIP: e030:[] [] >ptdump_walk_pgd_level_core+0x20e/0x440 It would be nice to see which line of code this corresponds to. Doing this: gdb vmlinux list *0x8105af8e should normally do the trick. Thanks, Ingo Hi Ingo, (gdb) list *0x8105af8e 0x8105af8e is in ptdump_walk_pgd_level_core (arch/x86/mm/dump_pagetables.c:181). warning: Source file is more recent than executable. 176 * On 64 bits, sign-extend the 48 bit address to 64 bit 177 */ 178 static unsigned long normalize_addr(unsigned long u) 179 { 180 #ifdef CONFIG_X86_64 181 return (signed long)(u << 16) >> 16; 182 #else 183 return u; 184 #endif 185 } -- Sander ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Linux 4.4 MW: Boot under Xen fails with CONFIG_DEBUG_WX enabled: RIP: ptdump_walk_pgd_level_core
On 2015-11-04 16:52, Stephen Smalley wrote: On 11/04/2015 06:55 AM, Sander Eikelenboom wrote: Hi All, I just tried to boot with the current linus mergewindow tree under Xen. It fails with a kernel panic at boot with the new "CONFIG_DEBUG_WX" option enabled. Disabling it makes the kernel boot fine. The splat: [ 18.424241] Freeing unused kernel memory: 1104K (822fc000 - 8241) [ 18.430314] Write protecting the kernel read-only data: 18432k [ 18.441054] Freeing unused kernel memory: 1144K (880001ae2000 - 880001c0) [ 18.447966] Freeing unused kernel memory: 1560K (88000207a000 - 88000220) [ 18.453947] BUG: unable to handle kernel paging request at 88055c883000 [ 18.459943] IP: [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.465847] PGD 2212067 PUD 0 [ 18.471564] Oops: [#1] SMP [ 18.477248] Modules linked in: [ 18.482918] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.3.0-mw-20151104-linus-doflr+ #1 [ 18.488804] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010 [ 18.494778] task: 880059b9 ti: 880059b98000 task.ti: 880059b98000 [ 18.500852] RIP: e030:[] [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.507102] RSP: e02b:880059b9be48 EFLAGS: 00010296 [ 18.513351] RAX: 88055c883000 RBX: 81ae2000 RCX: 8800 [ 18.519733] RDX: 0067 RSI: 880059b9be98 RDI: 88001000 [ 18.526129] RBP: 880059b9bf00 R08: R09: [ 18.532522] R10: 88005fd0e790 R11: 0001 R12: 88008000 [ 18.538891] R13: cfff R14: 880059b9be98 R15: [ 18.545247] FS: () GS:88005f68() knlGS: [ 18.551708] CS: e033 DS: ES: CR0: 8005003b [ 18.558153] CR2: 88055c883000 CR3: 02211000 CR4: 0660 [ 18.564686] Stack: [ 18.571106] 000159b9be50 82211000 88055c884000 0800 [ 18.577704] 8000 88055c883000 0007 88005fd0e790 [ 18.584291] 880059b9bed8 81156ace 0001 [ 18.590916] Call Trace: [ 18.597458] [] ? free_reserved_area+0x11e/0x120 [ 18.604180] [] ptdump_walk_pgd_level_checkwx+0x12/0x20 [ 18.611014] [] mark_rodata_ro+0xe9/0xf0 [ 18.617819] [] ? rest_init+0x80/0x80 [ 18.624512] [] kernel_init+0x18/0xe0 [ 18.631095] [] ret_from_fork+0x3f/0x70 [ 18.637650] [] ? rest_init+0x80/0x80 [ 18.644178] Code: 70 ff ff ff 48 3b 85 58 ff ff ff 0f 84 c0 fe ff ff 48 8b 85 68 ff ff ff 48 c1 e0 10 48 c1 f8 10 48 89 45 b0 48 8b 85 70 ff ff ff <48> 8b 38 48 85 ff 0f 85 4e ff ff ff b9 02 00 00 00 31 d2 4c 89 [ 18.658246] RIP [] ptdump_walk_pgd_level_core+0x20e/0x440 [ 18.665211] RSP [ 18.672073] CR2: 88055c883000 [ 18.678852] ---[ end trace d84e34461c40637a ]--- [ 18.685641] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 [ 18.685641] [ 18.699520] Kernel Offset: disable What's your .config? Does cat /sys/kernel/debug/kernel_page_tables produce a similar fault even with CONFIG_DEBUG_WX=n? .config is attached Hmm that sysfs file doesn't seem to exist then: # cat /sys/kernel/debug/kernel_page_tables cat: /sys/kernel/debug/kernel_page_tables: No such file or directory -- Sander # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 4.3.0-mw-20151104-linus-doflr Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_PERF_EVENTS_INTEL_UNCORE=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_HAVE_LATENCYTOP_SUPPORT=y CONFIG_MMU=y CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_64_SMP=y CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_PGTABLE_LEVELS=4 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_INIT_ENV_ARG