Re: [PATCH v2] Introduce a description of a new optional tag for Backports
On 16.04.2020 23:14, Stefano Stabellini wrote: > On Wed, 15 Apr 2020, Jan Beulich wrote: >> On 15.04.2020 11:56, George Dunlap wrote: >>> >>> On Apr 15, 2020, at 10:49 AM, Julien Grall wrote: On 15/04/2020 10:43, George Dunlap wrote: >> On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: >> >> On 14.04.2020 18:54, Stefano Stabellini wrote: >>> On Tue, 14 Apr 2020, Jan Beulich wrote: On 10.04.2020 18:49, Stefano Stabellini wrote: >>> > [snip] > +Backport: all > + > +It marks a commit for being a candidate for backports to all relevant > +trees. I'm unconvinced of the utility of this form - what "all" resolves to changes over time. There's almost always a first version where a particular issue was introduced. If we want this to be generally useful, imo we shouldn't limit the scope of the tag to the upstream maintained stable trees. >>> >>> The reason why I suggested also to have a "wildcard" version of this >>> tag, is that the person adding the tag (could be the contributor trying >>> to be helpful) might not know exactly to which stable trees the patch >>> should be backported to. >>> >>> Writing this sentence, I realize that I really meant "any" rather than >>> "all". Would you prefer if I used "any"? Or we could even suggest to >>> leave >>> it black like this: >>> >>> Backport: >>> >>> But it looks a bit weird. >> >> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or >> "unknown"? Nevertheless, I still think people asking for a backport >> should be nudged towards determining the applicable range; them not >> doing so effectively pushes the burden to the general maintainers or >> the stable tree ones, both of which scales less well. Omitting the >> tag if they don't want to invest the time would to me then seem to >> be the cleanest alternative. Albeit I'm sure views here will vary. > FWIW asking people adding the tag to do the work of figuring out which > versions to backport to makes sense to me. If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport. For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+? I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree. >>> >>> I agree that specifying the oldest revision possible would be helpful. >>> >>> However, I don’t think finding the absolute oldest revision should be >>> *required* — imagine a bug that was introduced between 3.2 and 3.3. It’s >>> also perfectly fine if you go all the way back to 4.2 and stop because you >>> get bored, not because you found out that 4.1 didn’t need it. > > I dropped the definition of "Backport: all", and adopted George's > suggested wording: > > The backport requester is expected to specify which currently supported > releases need the backport; but encouraged to specify a release as far > back as possible which applies. > > >> In which case I'd like there to be a (canonical?) way of expressing >> this, like in XSAs we say "at least back to" in such a case. > > I couldn't think of anything better than: > > Backport: 4.9+ # maybe older > > We probably don't need to codify something like that in this document. The suggestion looks fine to me, and people using slightly varying wording wouldn't be a problem either. > As an alternative we could perhaps reuse the "Backport: all" idea in a > different light for this new purpose. > > I expect that in these cases where we don't know the oldest affected > tree, all the currently maintained stable trees will have to get the > backport. So maybe we could use one of the following: > > Backport: all > Backport: oldest > Backport: oldest-unknown > > To express that the fix needs to be backported to *all* the currently > maintained stable trees, but there might be also other older > unmaintained trees that could be affected; we don't know for sure how > far back it should go. My prior objection to "all" remains - it changes over time what "currently means", rendering the tag stale quite quickly. I think that without even providing a suggested means to create such a tag without an explicit version specified we underline the need to figure out a baseline from where to apply the backport. One more thing comes to mind that may want mentioning here: If people request a backport, I think this should take as an implication their willingness to actually be involved in doing the actual backporting work. Typically it's pretty simple, but every now and then quite a bit of effort is needed. It would be nice if the stable tree maintainers could push over some of th
[xen-unstable test] 149689: tolerable FAIL - PUSHED
flight 149689 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/149689/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 16 guest-localmigrate fail like 149667 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149667 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 149667 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149667 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 149667 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149667 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149667 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 149667 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 149667 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149667 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: xen 615bfe42c6d183a0e54a0525ef82b58580d01619 baseline version: xen fcd06227f83643194f8018f8dd37adce57763a61 Last test of basis 149667 2020-04-15 07:26:21 Z1 days Testing same since 149689 2020-04-16 13:17:05 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Julien Grall jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm
[qemu-mainline test] 149681: tolerable FAIL - PUSHED
flight 149681 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/149681/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail blocked in 149660 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149660 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149660 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149660 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149660 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149660 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: qemuu20038cd7a8412feeb49c01f6ede89e36c8995472 baseline version: qemuua457215ed2aaa9598bd4ebbc6745d2a494ba9990 Last test of basis 149660 2020-04-14 19:36:24 Z2 days Testing same since 149681 2020-04-16 02:09:07 Z0 days1 attempts People who touched revisions under test: Alex Bennée Howard Spoelstra Igor Mammedov James Le Cuirot Michael Roth Paolo Bonzini Peter Maydell Peter Xu Philippe Mathieu-Daudé Stefano Garzarella Volker Rümelin jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64
[ovmf test] 149685: all pass - PUSHED
flight 149685 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/149685/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 06033f5abad3815e8d80de22c97ba38a05017262 baseline version: ovmf 8c654bb3ec0b5232dec2b2b07234c5479eb14d62 Last test of basis 149665 2020-04-15 01:40:25 Z1 days Testing same since 149685 2020-04-16 05:35:47 Z0 days1 attempts People who touched revisions under test: Shenglei Zhang jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 8c654bb3ec..06033f5aba 06033f5abad3815e8d80de22c97ba38a05017262 -> xen-tested-master
Re: [PATCH] arm/xen: make _xen_start_info static
On Wed, 15 Apr 2020, Jason Yan wrote: > Fix the following sparse warning: > > arch/arm64/xen/../../arm/xen/enlighten.c:39:19: warning: symbol > '_xen_start_info' was not declared. Should it be static? > > Reported-by: Hulk Robot > Signed-off-by: Jason Yan Reviewed-by: Stefano Stabellini > --- > arch/arm/xen/enlighten.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > index dd6804a64f1a..fd4e1ce1daf9 100644 > --- a/arch/arm/xen/enlighten.c > +++ b/arch/arm/xen/enlighten.c > @@ -36,7 +36,7 @@ > > #include > > -struct start_info _xen_start_info; > +static struct start_info _xen_start_info; > struct start_info *xen_start_info = &_xen_start_info; > EXPORT_SYMBOL(xen_start_info); > > -- > 2.21.1 >
Re: [PATCH v2] Introduce a description of a new optional tag for Backports
On Wed, 15 Apr 2020, Jan Beulich wrote: > On 15.04.2020 11:56, George Dunlap wrote: > > > > > >> On Apr 15, 2020, at 10:49 AM, Julien Grall wrote: > >> > >> > >> > >> On 15/04/2020 10:43, George Dunlap wrote: > On Apr 15, 2020, at 7:23 AM, Jan Beulich wrote: > > On 14.04.2020 18:54, Stefano Stabellini wrote: > > On Tue, 14 Apr 2020, Jan Beulich wrote: > >> On 10.04.2020 18:49, Stefano Stabellini wrote: > > > >>> [snip] > >>> +Backport: all > >>> + > >>> +It marks a commit for being a candidate for backports to all relevant > >>> +trees. > >> > >> I'm unconvinced of the utility of this form - what "all" resolves to > >> changes over time. There's almost always a first version where a > >> particular issue was introduced. If we want this to be generally > >> useful, imo we shouldn't limit the scope of the tag to the upstream > >> maintained stable trees. > > > > The reason why I suggested also to have a "wildcard" version of this > > tag, is that the person adding the tag (could be the contributor trying > > to be helpful) might not know exactly to which stable trees the patch > > should be backported to. > > > > Writing this sentence, I realize that I really meant "any" rather than > > "all". Would you prefer if I used "any"? Or we could even suggest to > > leave > > it black like this: > > > > Backport: > > > > But it looks a bit weird. > > Indeed. Instead of "all" or "any", how about "yes", "unspecified", or > "unknown"? Nevertheless, I still think people asking for a backport > should be nudged towards determining the applicable range; them not > doing so effectively pushes the burden to the general maintainers or > the stable tree ones, both of which scales less well. Omitting the > tag if they don't want to invest the time would to me then seem to > be the cleanest alternative. Albeit I'm sure views here will vary. > >>> FWIW asking people adding the tag to do the work of figuring out which > >>> versions to backport to makes sense to me. > >> > >> If you ask the contributor to do the work then you need to give guidance > >> on the "older" version you can specify in Backport. > >> > >> For instance, let say the bug was introduced in Xen 4.2. Are we allowing > >> the user to specify Backport: 4.2+ or should it be 4.11+? > >> > >> I would favor the former as this helps for downstream user which haven't > >> yet moved to the supported stable tree. > > > > I agree that specifying the oldest revision possible would be helpful. > > > > However, I don’t think finding the absolute oldest revision should be > > *required* — imagine a bug that was introduced between 3.2 and 3.3. It’s > > also perfectly fine if you go all the way back to 4.2 and stop because you > > get bored, not because you found out that 4.1 didn’t need it. I dropped the definition of "Backport: all", and adopted George's suggested wording: The backport requester is expected to specify which currently supported releases need the backport; but encouraged to specify a release as far back as possible which applies. > In which case I'd like there to be a (canonical?) way of expressing > this, like in XSAs we say "at least back to" in such a case. I couldn't think of anything better than: Backport: 4.9+ # maybe older We probably don't need to codify something like that in this document. As an alternative we could perhaps reuse the "Backport: all" idea in a different light for this new purpose. I expect that in these cases where we don't know the oldest affected tree, all the currently maintained stable trees will have to get the backport. So maybe we could use one of the following: Backport: all Backport: oldest Backport: oldest-unknown To express that the fix needs to be backported to *all* the currently maintained stable trees, but there might be also other older unmaintained trees that could be affected; we don't know for sure how far back it should go.
[libvirt test] 149684: regressions - FAIL
flight 149684 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/149684/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-i386-libvirt6 libvirt-buildfail REGR. vs. 146182 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 146182 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a version targeted for testing: libvirt a158eb5c8e18336d1dd39148a5d5c6b0bd3f26c1 baseline version: libvirt a1cd25b919509be2645dbe6f952d5263e0d4e4e5 Last test of basis 146182 2020-01-17 06:00:23 Z 90 days Failing since146211 2020-01-18 04:18:52 Z 89 days 86 attempts Testing same since 149684 2020-04-16 05:00:04 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Arnaud Patard Bjoern Walk Boris Fiuczynski Christian Borntraeger Christian Ehrhardt Christian Schoenebeck Collin Walling Cornelia Huck Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Dario Faggioli Erik Skultety Gaurav Agrawal Han Han Jamie Strandboge Jim Fehlig Jiri Denemark Jonathon Jongsma Julio Faracco Ján Tomko Laine Stump Lin Ma Marc-André Lureau Marek Marczykowski-Górecki Mauro S. M. Rodrigues Michal Privoznik Nikolay Shirokovskiy Pavel Hrdina Pavel Mores Peter Krempa Pino Toscano Prathamesh Chavan Rafael Fonseca Richard W.M. Jones Rikard Falkeborn Ryan Moeller Sahid Orentino Ferdjaoui Sebastian Mitterle Seeteena Thoufeek Stefan Berger Stefan Berger Stefan Hajnoczi Thomas Huth Wu Qingliang Yi Li Your Name Zhang Bo zhenwei pi Zhimin Feng jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairblocked test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-l
[linux-linus test] 149678: tolerable FAIL - PUSHED
flight 149678 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/149678/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-rtds 16 guest-localmigrate fail in 149659 pass in 149678 test-amd64-amd64-examine4 memdisk-try-append fail in 149659 pass in 149678 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 149659 pass in 149678 test-arm64-arm64-xl-seattle 7 xen-boot fail pass in 149659 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-seattle 13 migrate-support-check fail in 149659 never pass test-arm64-arm64-xl-seattle 14 saverestore-support-check fail in 149659 never pass test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail like 149632 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 149632 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149632 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 149632 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149632 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149632 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149632 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 149632 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149632 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: linux8632e9b5645bbc2331d21d892b0d6961c1a08429 baseline version: linux8f3d9f354286745c751374f5f1fcafee6b3f3136 Last test of basis 149632 2020-04-13 01:08:56 Z3 days Testing
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
Dario Faggioli writes ("Re: [PATCH OSSTEST 2/2] make-flight: add a core
scheduling job"):
> Ok, sure. Maybe it would make sense to add just another one for Credit,
> sooner rather than later, as I guess there may be people wanting to
> continue use Credit, but they may want to try it with core-scheduling.
Maybe in return we could delete the rtds test which has been marked
nonblocking forever and which no-one seems to be fixing ? :-)
Patches welcome...
Ian.
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
Roger Pau Monne writes ("Re: [PATCH OSSTEST 2/2] make-flight: add a core
scheduling job"):
> On Thu, Apr 16, 2020 at 06:28:33PM +0200, Dario Faggioli wrote:
> > Yep, and that's enough for enabling and starting using ore-scheduling.
> > So, doing like this, core-scheduling should get the same amount and
> > kind of testing that scheduling in general gets.
>
> Well, we run a lot more tests without 'sched-gran=core', but I don't
> think it's feasible to duplicate the matrix to run all tests with and
> without core-scheduling.
Yes, I agree with Roger.
> > One question, as my OSSTest-fu is a bit rusty... does this create
> > "sched-gran=core" tests for all the schedulers? Or just one of them for
> > th default scheduler?
>
> Just for the default scheduler ATM, we can expand this if required.
> The test also is very simple, as it just creates a Debian PV guest
> and does some basic life cycle operations, it's exactly like the job
> below but with 'sched-gran=core':
>
> http://logs.test-lab.xenproject.org/osstest/logs/149667/test-amd64-amd64-xl/info.html
Right.
Thanks,
Ian.
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
On Thu, 2020-04-16 at 18:36 +0200, Roger Pau Monné wrote: > On Thu, Apr 16, 2020 at 06:28:33PM +0200, Dario Faggioli wrote: > > On Wed, 2020-04-15 at 14:06 +0100, Ian Jackson wrote: > > > > > > This seems fine as far as it goes, but all it does is check that > > > things still work if sched-gran=core is passed. > > > > > Yep, and that's enough for enabling and starting using ore- > > scheduling. > > So, doing like this, core-scheduling should get the same amount and > > kind of testing that scheduling in general gets. > > Well, we run a lot more tests without 'sched-gran=core', but I don't > think it's feasible to duplicate the matrix to run all tests with and > without core-scheduling. > Sure, but again, that's the kind of scheduling testing that we do. E.g., right now that Credit2 is the default scheduler, we still test the other schedulers, exactly with "just" a job like this (AFAICR, at least). It indeed would be good to have something more specific, not only for core-scheduling, but for scheduling in general. But it's not there right now... That was the point. :-) Of course the default setup (which, currently, has "sched-gran=cpu") is stressed much more, because all the other jobs also use it, but indeed it does not appear sensible to replicate the matrix for each job that we have with a non-default configuration. > > One question, as my OSSTest-fu is a bit rusty... does this create > > "sched-gran=core" tests for all the schedulers? Or just one of them > > for > > th default scheduler? > > Just for the default scheduler ATM, we can expand this if required. > Ok, sure. Maybe it would make sense to add just another one for Credit, sooner rather than later, as I guess there may be people wanting to continue use Credit, but they may want to try it with core-scheduling. Of course, this can be done on top of this patch... I was just thinking out loud here. :-) Thanks and Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
Re: [PATCH] sched: print information about scheduler granularity
On Thu, 2020-04-16 at 11:25 +0200, Jürgen Groß wrote: > On 16.04.20 11:20, Sergey Dyasli wrote: > > On 16/04/2020 09:57, Jürgen Groß wrote: > > > > > > The xen commandline ins part of the boot messages and is > > > contained > > > in the "xl info" output. > > > > It's true that you can see "sched-gran=core" in "xl info" output. > > But that's > > just the switch - not the end result. A user might want to verify > > that he did > > everything correctly and core-scheduling mode has indeed been > > enabled. > > I'm not opposed to your patch, but as soon as we have per-cpupool > granularity it should be reverted again. > Why reverted? Each cpupool dumps its own scheduling information. With per-pool granularity, we'll see something like cpupool: Pool-A Scheduler: SMP Credit Scheduler (credit) Scheduling granularity: cpu ... cpupool: Pool-B Scheduler: SMP Credit Scheduler (credit) Scheduling granularity: core etc. Or am I missing something? Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
Re: [PATCH] sched: print information about scheduler granularity
On Thu, 2020-04-16 at 09:33 +0100, Sergey Dyasli wrote:
> Currently it might be not obvious which scheduling mode is being used
> by the scheduler. Alleviate this by printing additional information
> about the selected granularity.
>
I like the idea. However, I don't like how verbose and long that line
becomes.
> Messages now look like these:
>
> 1. boot
> (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler
> (credit) in core-scheduling mode
>
> 2. xl debug-keys r
> (XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-
> way core-scheduling mode
>
What about adding an entry, just below these ones. Something looking
like, for instance (both at boot and in the debug-key dump):
"Scheduling granularity: cpu"
(or "core", or "socket")
Also
> --- a/xen/common/sched/cpupool.c
> +++ b/xen/common/sched/cpupool.c
> @@ -38,7 +38,35 @@ static cpumask_t cpupool_locked_cpus;
> static DEFINE_SPINLOCK(cpupool_lock);
>
> static enum sched_gran __read_mostly opt_sched_granularity =
> SCHED_GRAN_cpu;
> -static unsigned int __read_mostly sched_granularity = 1;
> +static unsigned int __read_mostly sched_granularity;
> +
> +char *sched_gran_str(char *str, size_t size)
> +{
> +char *mode = "";
> +
> +switch ( opt_sched_granularity )
> +{
> +case SCHED_GRAN_cpu:
> +mode = "cpu";
> +break;
> +case SCHED_GRAN_core:
> +mode = "core";
> +break;
> +case SCHED_GRAN_socket:
> +mode = "socket";
> +break;
> +default:
> +ASSERT_UNREACHABLE();
> +break;
> +}
> +
> +if ( sched_granularity )
> +snprintf(str, size, "%u-way %s", sched_granularity, mode);
>
I'm not sure about using the value of the enum like this.
E.g., in a system with 4 threads per core, enabling core scheduling
granularity would mean having 4 vCPUs in the scheduling units. But this
will still print "2-way core-scheduling", which I think would sound
confusing.
So I'd just go with "cpu", "core" and "socket" strings.
Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)
signature.asc
Description: This is a digitally signed message part
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
On Thu, Apr 16, 2020 at 06:28:33PM +0200, Dario Faggioli wrote:
> On Wed, 2020-04-15 at 14:06 +0100, Ian Jackson wrote:
> > Roger Pau Monne writes ("[PATCH OSSTEST 2/2] make-flight: add a core
> > scheduling job"):
> > > Run a simple core scheduling tests on a host that has SMT support.
> > > This is only enabled for Xen >= 4.13.
> > ...
> > > + # Core-scheduling tests are x86 only
> > > + if [ x$test_coresched = xy -a $xenarch = amd64 ]; then
> > > +job_create_test test-$xenarch$kern-coresched-$dom0arch-xl \
> > > +test-debian xl $xenarch $dom0arch
> > > $debian_runvars \
> > > +all_hostflags=$most_hostflags,smt \
> > > +xen_boot_append='sched-gran=core'
> > > +
> > > + fi
> >
> > This seems fine as far as it goes, but all it does is check that
> > things still work if sched-gran=core is passed.
> >
> Yep, and that's enough for enabling and starting using ore-scheduling.
> So, doing like this, core-scheduling should get the same amount and
> kind of testing that scheduling in general gets.
Well, we run a lot more tests without 'sched-gran=core', but I don't
think it's feasible to duplicate the matrix to run all tests with and
without core-scheduling.
> > I'm not sure whether
> > anything more sophisticated is needed, and in any case this is a step
> > in the right direction, so:
> >
> Indeed.
>
> One question, as my OSSTest-fu is a bit rusty... does this create
> "sched-gran=core" tests for all the schedulers? Or just one of them for
> th default scheduler?
Just for the default scheduler ATM, we can expand this if required.
The test also is very simple, as it just creates a Debian PV guest
and does some basic life cycle operations, it's exactly like the job
below but with 'sched-gran=core':
http://logs.test-lab.xenproject.org/osstest/logs/149667/test-amd64-amd64-xl/info.html
Roger.
Re: [PATCH OSSTEST 2/2] make-flight: add a core scheduling job
On Wed, 2020-04-15 at 14:06 +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH OSSTEST 2/2] make-flight: add a core
> scheduling job"):
> > Run a simple core scheduling tests on a host that has SMT support.
> > This is only enabled for Xen >= 4.13.
> ...
> > + # Core-scheduling tests are x86 only
> > + if [ x$test_coresched = xy -a $xenarch = amd64 ]; then
> > +job_create_test test-$xenarch$kern-coresched-$dom0arch-xl \
> > +test-debian xl $xenarch $dom0arch
> > $debian_runvars \
> > +all_hostflags=$most_hostflags,smt \
> > +xen_boot_append='sched-gran=core'
> > +
> > + fi
>
> This seems fine as far as it goes, but all it does is check that
> things still work if sched-gran=core is passed.
>
Yep, and that's enough for enabling and starting using ore-scheduling.
So, doing like this, core-scheduling should get the same amount and
kind of testing that scheduling in general gets.
> I'm not sure whether
> anything more sophisticated is needed, and in any case this is a step
> in the right direction, so:
>
Indeed.
One question, as my OSSTest-fu is a bit rusty... does this create
"sched-gran=core" tests for all the schedulers? Or just one of them for
th default scheduler?
Thanks and Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] sched: fix scheduler_disable() with core scheduling
On Thu, 2020-04-09 at 14:50 +0200, Jürgen Groß wrote: > On 09.04.20 11:41, Sergey Dyasli wrote: > > In core-scheduling mode, Xen might crash when entering ACPI S5 > > state. > > This happens in sched_slave() during is_idle_unit(next) check > > because > > next->vcpu_list is stale and points to an already freed memory. > > > > This situation happens shortly after scheduler_disable() is called > > if > > some CPU is still inside sched_slave() softirq. Current logic > > simply > > returns prev->next_task from sched_wait_rendezvous_in() which > > causes > > the described crash because next_task->vcpu_list has become > > invalid. > > > > Fix the crash by returning NULL from sched_wait_rendezvous_in() in > > the case when scheduler_disable() has been called. > > > > Signed-off-by: Sergey Dyasli > > Reviewed-by: Juergen Gross > Reviewed-by: Dario Faggioli Thanks and Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
Re: [Xen-devel] [PATCH] sched/core: Fix bug when moving a domain between cpupools
On Wed, 2020-04-15 at 11:08 +0200, Jürgen Groß wrote: > On 27.03.20 20:30, Jeff Kubascik wrote: > > For each UNIT, sched_set_affinity is called before unit->priv is > > updated > > to the new cpupool private UNIT data structure. The issue is > > sched_set_affinity will call the adjust_affinity method of the > > cpupool. > > If defined, the new cpupool may use unit->priv (e.g. credit), which > > at > > this point still references the old cpupool private UNIT data > > structure. > > > > This change fixes the bug by moving the switch of unit->priv earler > > in > > the function. > > > > Signed-off-by: Jeff Kubascik > > Reviewed-by: Juergen Gross > Acked-by: Dario Faggioli Sorry it took a while. And thanks Juergen for also having a look. Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
[PATCH 6/6] x86/mem-paging: consistently use gfn_t
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -278,7 +278,7 @@ static int set_mem_type(struct domain *d
if ( p2m_is_paging(t) )
{
put_gfn(d, pfn);
-p2m_mem_paging_populate(d, pfn);
+p2m_mem_paging_populate(d, _gfn(pfn));
return -EAGAIN;
}
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1968,7 +1968,7 @@ int hvm_hap_nested_page_fault(paddr_t gp
* locks in such circumstance.
*/
if ( paged )
-p2m_mem_paging_populate(currd, gfn);
+p2m_mem_paging_populate(currd, _gfn(gfn));
if ( sharing_enomem )
{
@@ -3199,7 +3199,7 @@ enum hvm_translation_result hvm_translat
if ( p2m_is_paging(p2mt) )
{
put_page(page);
-p2m_mem_paging_populate(v->domain, gfn_x(gfn));
+p2m_mem_paging_populate(v->domain, gfn);
return HVMTRANS_gfn_paged_out;
}
if ( p2m_is_shared(p2mt) )
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2151,16 +2151,17 @@ static int mod_l1_entry(l1_pgentry_t *pl
paging_mode_translate(pg_dom) )
{
p2m_type_t p2mt;
+unsigned long gfn = l1e_get_pfn(nl1e);
p2m_query_t q = l1e_get_flags(nl1e) & _PAGE_RW ?
P2M_ALLOC | P2M_UNSHARE : P2M_ALLOC;
-page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), &p2mt, q);
+page = get_page_from_gfn(pg_dom, gfn, &p2mt, q);
if ( p2m_is_paged(p2mt) )
{
if ( page )
put_page(page);
-p2m_mem_paging_populate(pg_dom, l1e_get_pfn(nl1e));
+p2m_mem_paging_populate(pg_dom, _gfn(gfn));
return -ENOENT;
}
@@ -3982,7 +3983,7 @@ long do_mmu_update(
put_page(page);
if ( p2m_is_paged(p2mt) )
{
-p2m_mem_paging_populate(pt_owner, gmfn);
+p2m_mem_paging_populate(pt_owner, _gfn(gmfn));
rc = -ENOENT;
}
else
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -68,7 +68,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
*pfec = PFEC_page_paged;
if ( top_page )
put_page(top_page);
-p2m_mem_paging_populate(p2m->domain, cr3 >> PAGE_SHIFT);
+p2m_mem_paging_populate(p2m->domain, _gfn(PFN_DOWN(cr3)));
return gfn_x(INVALID_GFN);
}
if ( p2m_is_shared(p2mt) )
@@ -109,7 +109,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
{
ASSERT(p2m_is_hostp2m(p2m));
*pfec = PFEC_page_paged;
-p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
+p2m_mem_paging_populate(p2m->domain, gfn);
return gfn_x(INVALID_GFN);
}
if ( p2m_is_shared(p2mt) )
--- a/xen/arch/x86/mm/mem_paging.c
+++ b/xen/arch/x86/mm/mem_paging.c
@@ -36,12 +36,11 @@
* released by the guest. The pager is supposed to drop its reference of the
* gfn.
*/
-void p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn,
- p2m_type_t p2mt)
+void p2m_mem_paging_drop_page(struct domain *d, gfn_t gfn, p2m_type_t p2mt)
{
vm_event_request_t req = {
.reason = VM_EVENT_REASON_MEM_PAGING,
-.u.mem_paging.gfn = gfn
+.u.mem_paging.gfn = gfn_x(gfn)
};
/*
@@ -89,16 +88,15 @@ void p2m_mem_paging_drop_page(struct dom
* already sent to the pager. In this case the caller has to try again until
the
* gfn is fully paged in again.
*/
-void p2m_mem_paging_populate(struct domain *d, unsigned long gfn_l)
+void p2m_mem_paging_populate(struct domain *d, gfn_t gfn)
{
struct vcpu *v = current;
vm_event_request_t req = {
.reason = VM_EVENT_REASON_MEM_PAGING,
-.u.mem_paging.gfn = gfn_l
+.u.mem_paging.gfn = gfn_x(gfn)
};
p2m_type_t p2mt;
p2m_access_t a;
-gfn_t gfn = _gfn(gfn_l);
mfn_t mfn;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int rc = vm_event_claim_slot(d, d->vm_event_paging);
@@ -107,7 +105,7 @@ void p2m_mem_paging_populate(struct doma
if ( rc == -EOPNOTSUPP )
{
gdprintk(XENLOG_ERR, "Dom%d paging gfn %lx yet no ring in place\n",
- d->domain_id, gfn_l);
+ d->domain_id, gfn_x(gfn));
/* Prevent the vcpu from faulting repeatedly on the same gfn */
if ( v->domain == d )
vcpu_pause_nosync(v);
@@ -218,13 +216,12 @@ void p2m_mem_paging_resume(struct domain
* Once the p2mt is changed the page is readonly for the guest. On success the
* pager can write the page contents to disk and later evict the page.
*/
-static int nominate(struct domain *d, unsigned long gfn_l)
+static int nominate(struct domain *d, gfn_t gfn)
{
struct page_info *page;
struct p2m_domain *p2m =
[PATCH 5/6] x86/mem-paging: move code to its dedicated source file
Do a little bit of style adjustment along the way, and drop the
"p2m_mem_paging_" prefixes from the now static functions.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/mm/mem_paging.c
+++ b/xen/arch/x86/mm/mem_paging.c
@@ -25,6 +25,421 @@
#include
#include
+#include "mm-locks.h"
+
+/*
+ * p2m_mem_paging_drop_page - Tell pager to drop its reference to a paged page
+ * @d: guest domain
+ * @gfn: guest page to drop
+ *
+ * p2m_mem_paging_drop_page() will notify the pager that a paged-out gfn was
+ * released by the guest. The pager is supposed to drop its reference of the
+ * gfn.
+ */
+void p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn,
+ p2m_type_t p2mt)
+{
+vm_event_request_t req = {
+.reason = VM_EVENT_REASON_MEM_PAGING,
+.u.mem_paging.gfn = gfn
+};
+
+/*
+ * We allow no ring in this unique case, because it won't affect
+ * correctness of the guest execution at this point. If this is the only
+ * page that happens to be paged-out, we'll be okay.. but it's likely the
+ * guest will crash shortly anyways.
+ */
+int rc = vm_event_claim_slot(d, d->vm_event_paging);
+
+if ( rc < 0 )
+return;
+
+/* Send release notification to pager */
+req.u.mem_paging.flags = MEM_PAGING_DROP_PAGE;
+
+/* Update stats unless the page hasn't yet been evicted */
+if ( p2mt != p2m_ram_paging_out )
+atomic_dec(&d->paged_pages);
+else
+/* Evict will fail now, tag this request for pager */
+req.u.mem_paging.flags |= MEM_PAGING_EVICT_FAIL;
+
+vm_event_put_request(d, d->vm_event_paging, &req);
+}
+
+/*
+ * p2m_mem_paging_populate - Tell pager to populate a paged page
+ * @d: guest domain
+ * @gfn: guest page in paging state
+ *
+ * p2m_mem_paging_populate() will notify the pager that a page in any of the
+ * paging states needs to be written back into the guest.
+ * This function needs to be called whenever gfn_to_mfn() returns any of the
p2m
+ * paging types because the gfn may not be backed by a mfn.
+ *
+ * The gfn can be in any of the paging states, but the pager needs only be
+ * notified when the gfn is in the paging-out path (paging_out or paged). This
+ * function may be called more than once from several vcpus. If the vcpu
belongs
+ * to the guest, the vcpu must be stopped and the pager notified that the vcpu
+ * was stopped. The pager needs to handle several requests for the same gfn.
+ *
+ * If the gfn is not in the paging-out path and the vcpu does not belong to the
+ * guest, nothing needs to be done and the function assumes that a request was
+ * already sent to the pager. In this case the caller has to try again until
the
+ * gfn is fully paged in again.
+ */
+void p2m_mem_paging_populate(struct domain *d, unsigned long gfn_l)
+{
+struct vcpu *v = current;
+vm_event_request_t req = {
+.reason = VM_EVENT_REASON_MEM_PAGING,
+.u.mem_paging.gfn = gfn_l
+};
+p2m_type_t p2mt;
+p2m_access_t a;
+gfn_t gfn = _gfn(gfn_l);
+mfn_t mfn;
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+int rc = vm_event_claim_slot(d, d->vm_event_paging);
+
+/* We're paging. There should be a ring. */
+if ( rc == -EOPNOTSUPP )
+{
+gdprintk(XENLOG_ERR, "Dom%d paging gfn %lx yet no ring in place\n",
+ d->domain_id, gfn_l);
+/* Prevent the vcpu from faulting repeatedly on the same gfn */
+if ( v->domain == d )
+vcpu_pause_nosync(v);
+domain_crash(d);
+return;
+}
+else if ( rc < 0 )
+return;
+
+/* Fix p2m mapping */
+gfn_lock(p2m, gfn, 0);
+mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL, NULL);
+/* Allow only nominated or evicted pages to enter page-in path */
+if ( p2mt == p2m_ram_paging_out || p2mt == p2m_ram_paged )
+{
+/* Evict will fail now, tag this request for pager */
+if ( p2mt == p2m_ram_paging_out )
+req.u.mem_paging.flags |= MEM_PAGING_EVICT_FAIL;
+
+rc = p2m_set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_in, a);
+}
+gfn_unlock(p2m, gfn, 0);
+if ( rc < 0 )
+goto out_cancel;
+
+/* Pause domain if request came from guest and gfn has paging type */
+if ( p2m_is_paging(p2mt) && v->domain == d )
+{
+vm_event_vcpu_pause(v);
+req.flags |= VM_EVENT_FLAG_VCPU_PAUSED;
+}
+/* No need to inform pager if the gfn is not in the page-out path */
+else if ( p2mt != p2m_ram_paging_out && p2mt != p2m_ram_paged )
+{
+/* gfn is already on its way back and vcpu is not paused */
+out_cancel:
+vm_event_cancel_slot(d, d->vm_event_paging);
+return;
+}
+
+/* Send request to pager */
+req.u.mem_paging.p2mt = p2mt;
+req.vcpu_id = v->vcpu_id;
+
+vm_event_put_request(d, d->vm_event_paging, &req);
+}
+
+/*
+ * p2m_mem_paging_resume - Resume guest gfn
+ * @d: guest domain
+ * @rsp: vm_e
[PATCH 3/6] x86/mem-paging: use guest handle for XENMEM_paging_op_prep
While it should have been this way from the beginning, not doing so will
become an actual problem with PVH Dom0. The interface change is binary
compatible, but requires tools side producers to be re-built.
Drop the bogus/unnecessary page alignment restriction on the input
buffer at the same time.
Signed-off-by: Jan Beulich
---
Is there really no way to avoid the buffer copying in libxc?
--- a/tools/libxc/xc_mem_paging.c
+++ b/tools/libxc/xc_mem_paging.c
@@ -26,15 +26,33 @@ static int xc_mem_paging_memop(xc_interf
unsigned int op, uint64_t gfn, void *buffer)
{
xen_mem_paging_op_t mpo;
+DECLARE_HYPERCALL_BOUNCE(buffer, XC_PAGE_SIZE,
+ XC_HYPERCALL_BUFFER_BOUNCE_IN);
+int rc;
memset(&mpo, 0, sizeof(mpo));
mpo.op = op;
mpo.domain = domain_id;
mpo.gfn = gfn;
-mpo.buffer = (unsigned long) buffer;
-return do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo));
+if ( buffer )
+{
+if ( xc_hypercall_bounce_pre(xch, buffer) )
+{
+PERROR("Could not bounce memory for XENMEM_paging_op %u", op);
+return -1;
+}
+
+set_xen_guest_handle(mpo.buffer, buffer);
+}
+
+rc = do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo));
+
+if ( buffer )
+xc_hypercall_bounce_post(xch, buffer);
+
+return rc;
}
int xc_mem_paging_enable(xc_interface *xch, uint32_t domain_id,
@@ -92,28 +110,13 @@ int xc_mem_paging_prep(xc_interface *xch
int xc_mem_paging_load(xc_interface *xch, uint32_t domain_id,
uint64_t gfn, void *buffer)
{
-int rc, old_errno;
-
errno = EINVAL;
if ( !buffer )
return -1;
-if ( ((unsigned long) buffer) & (XC_PAGE_SIZE - 1) )
-return -1;
-
-if ( mlock(buffer, XC_PAGE_SIZE) )
-return -1;
-
-rc = xc_mem_paging_memop(xch, domain_id,
- XENMEM_paging_op_prep,
- gfn, buffer);
-
-old_errno = errno;
-munlock(buffer, XC_PAGE_SIZE);
-errno = old_errno;
-
-return rc;
+return xc_mem_paging_memop(xch, domain_id, XENMEM_paging_op_prep,
+ gfn, buffer);
}
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1779,7 +1779,8 @@ void p2m_mem_paging_populate(struct doma
* mfn if populate was called for gfn which was nominated but not evicted. In
* this case only the p2mt needs to be forwarded.
*/
-int p2m_mem_paging_prep(struct domain *d, unsigned long gfn_l, uint64_t buffer)
+int p2m_mem_paging_prep(struct domain *d, unsigned long gfn_l,
+XEN_GUEST_HANDLE_PARAM(const_uint8) buffer)
{
struct page_info *page = NULL;
p2m_type_t p2mt;
@@ -1788,13 +1789,9 @@ int p2m_mem_paging_prep(struct domain *d
mfn_t mfn;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int ret, page_extant = 1;
-const void *user_ptr = (const void *) buffer;
-if ( user_ptr )
-/* Sanity check the buffer and bail out early if trouble */
-if ( (buffer & (PAGE_SIZE - 1)) ||
- (!access_ok(user_ptr, PAGE_SIZE)) )
-return -EINVAL;
+if ( !guest_handle_okay(buffer, PAGE_SIZE) )
+return -EINVAL;
gfn_lock(p2m, gfn, 0);
@@ -1812,7 +1809,7 @@ int p2m_mem_paging_prep(struct domain *d
/* If the user did not provide a buffer, we disallow */
ret = -EINVAL;
-if ( unlikely(user_ptr == NULL) )
+if ( unlikely(guest_handle_is_null(buffer)) )
goto out;
/* Get a free page */
ret = -ENOMEM;
@@ -1834,7 +1831,7 @@ int p2m_mem_paging_prep(struct domain *d
ASSERT( mfn_valid(mfn) );
guest_map = map_domain_page(mfn);
-ret = copy_from_user(guest_map, user_ptr, PAGE_SIZE);
+ret = copy_from_guest(guest_map, buffer, PAGE_SIZE);
unmap_domain_page(guest_map);
if ( ret )
{
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -741,7 +741,8 @@ void p2m_mem_paging_drop_page(struct dom
/* Start populating a paged out frame */
void p2m_mem_paging_populate(struct domain *d, unsigned long gfn);
/* Prepare the p2m for paging a frame in */
-int p2m_mem_paging_prep(struct domain *d, unsigned long gfn, uint64_t buffer);
+int p2m_mem_paging_prep(struct domain *d, unsigned long gfn,
+XEN_GUEST_HANDLE_PARAM(const_uint8) buffer);
/* Resume normal operation (in case a domain was paused) */
struct vm_event_st;
void p2m_mem_paging_resume(struct domain *d, struct vm_event_st *rsp);
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -396,10 +396,10 @@ struct xen_mem_paging_op {
uint8_t op; /* XENMEM_paging_op_* */
domid_t domain;
-/* PAGING_PREP IN: buffer to immediately fill page in */
-uint64_aligned_tbuffer;
-/* Other OPs */
-uint64_aligned_tgfn; /* IN:
[PATCH 4/6] x86/mem-paging: add minimal lock order enforcement to p2m_mem_paging_prep()
While full checking is impossible (as the lock is being acquired/ released down the call tree), perform at least a lock level check. Signed-off-by: Jan Beulich --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1813,6 +1813,7 @@ int p2m_mem_paging_prep(struct domain *d goto out; /* Get a free page */ ret = -ENOMEM; +page_alloc_mm_pre_lock(d); page = alloc_domheap_page(d, 0); if ( unlikely(page == NULL) ) goto out;
[PATCH 2/6] x86/mem-paging: correct p2m_mem_paging_prep()'s error handling
Communicating errors from p2m_set_entry() to the caller is not enough:
Neither the M2P nor the stats updates should occur in such a case.
Instead the allocated page needs to be freed again; for cleanliness
reasons also properly take into account _PGC_allocated there.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1781,7 +1781,7 @@ void p2m_mem_paging_populate(struct doma
*/
int p2m_mem_paging_prep(struct domain *d, unsigned long gfn_l, uint64_t buffer)
{
-struct page_info *page;
+struct page_info *page = NULL;
p2m_type_t p2mt;
p2m_access_t a;
gfn_t gfn = _gfn(gfn_l);
@@ -1816,9 +1816,19 @@ int p2m_mem_paging_prep(struct domain *d
goto out;
/* Get a free page */
ret = -ENOMEM;
-page = alloc_domheap_page(p2m->domain, 0);
+page = alloc_domheap_page(d, 0);
if ( unlikely(page == NULL) )
goto out;
+if ( unlikely(!get_page(page, d)) )
+{
+/*
+ * The domain can't possibly know about this page yet, so failure
+ * here is a clear indication of something fishy going on.
+ */
+domain_crash(d);
+page = NULL;
+goto out;
+}
mfn = page_to_mfn(page);
page_extant = 0;
@@ -1832,7 +1842,6 @@ int p2m_mem_paging_prep(struct domain *d
"Failed to load paging-in gfn %lx Dom%d bytes left %d\n",
gfn_l, d->domain_id, ret);
ret = -EFAULT;
-put_page(page); /* Don't leak pages */
goto out;
}
}
@@ -1843,13 +1852,24 @@ int p2m_mem_paging_prep(struct domain *d
ret = p2m_set_entry(p2m, gfn, mfn, PAGE_ORDER_4K,
paging_mode_log_dirty(d) ? p2m_ram_logdirty
: p2m_ram_rw, a);
-set_gpfn_from_mfn(mfn_x(mfn), gfn_l);
+if ( !ret )
+{
+set_gpfn_from_mfn(mfn_x(mfn), gfn_l);
-if ( !page_extant )
-atomic_dec(&d->paged_pages);
+if ( !page_extant )
+atomic_dec(&d->paged_pages);
+}
out:
gfn_unlock(p2m, gfn, 0);
+
+if ( page )
+{
+if ( ret )
+put_page_alloc_ref(page);
+put_page(page);
+}
+
return ret;
}
[PATCH 1/6] x86/mem-paging: fold p2m_mem_paging_prep()'s main if()-s
The condition of the second can be true only if the condition of the
first was met; the second half of the condition of the second then also
is redundant with an earlier check. Combine them, drop a pointless
local variable, and re-flow the affected gdprintk().
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1808,6 +1808,8 @@ int p2m_mem_paging_prep(struct domain *d
/* Allocate a page if the gfn does not have one yet */
if ( !mfn_valid(mfn) )
{
+void *guest_map;
+
/* If the user did not provide a buffer, we disallow */
ret = -EINVAL;
if ( unlikely(user_ptr == NULL) )
@@ -1819,22 +1821,16 @@ int p2m_mem_paging_prep(struct domain *d
goto out;
mfn = page_to_mfn(page);
page_extant = 0;
-}
-
-/* If we were given a buffer, now is the time to use it */
-if ( !page_extant && user_ptr )
-{
-void *guest_map;
-int rc;
ASSERT( mfn_valid(mfn) );
guest_map = map_domain_page(mfn);
-rc = copy_from_user(guest_map, user_ptr, PAGE_SIZE);
+ret = copy_from_user(guest_map, user_ptr, PAGE_SIZE);
unmap_domain_page(guest_map);
-if ( rc )
+if ( ret )
{
-gdprintk(XENLOG_ERR, "Failed to load paging-in gfn %lx domain %u "
- "bytes left %d\n", gfn_l, d->domain_id, rc);
+gdprintk(XENLOG_ERR,
+ "Failed to load paging-in gfn %lx Dom%d bytes left %d\n",
+ gfn_l, d->domain_id, ret);
ret = -EFAULT;
put_page(page); /* Don't leak pages */
goto out;
[PATCH 0/6] x86/mem-paging: misc cleanup
Repeatedly looking at varying parts of this code has lead to me accumulating a few adjustments. 1: fold p2m_mem_paging_prep()'s main if()-s 2: correct p2m_mem_paging_prep()'s error handling 3: use guest handle for XENMEM_paging_op_prep 4: add minimal lock order enforcement to p2m_mem_paging_prep() 5: move code to its dedicated source file 6: consistently use gfn_t Jan
[xen-unstable-smoke test] 149688: tolerable all pass - PUSHED
flight 149688 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/149688/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen a48e1323f9aa29f1ffb95594671b73de6bd7c1d4 baseline version: xen 615bfe42c6d183a0e54a0525ef82b58580d01619 Last test of basis 149686 2020-04-16 09:01:03 Z0 days Testing same since 149688 2020-04-16 12:01:12 Z0 days1 attempts People who touched revisions under test: Harsha Shamsundara Havanur Hongyan Xia Roger Pau Monné Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 615bfe42c6..a48e1323f9 a48e1323f9aa29f1ffb95594671b73de6bd7c1d4 -> smoke
Re: [XEN PATCH v4 14/18] xen,symbols: rework file symbols selection
On Thu, Apr 16, 2020 at 04:22:05PM +0200, Jan Beulich wrote:
> On 16.04.2020 14:44, Anthony PERARD wrote:
> > On Wed, Apr 08, 2020 at 02:54:35PM +0200, Jan Beulich wrote:
> >> On 31.03.2020 12:30, Anthony PERARD wrote:
> >>> We want to use the same rune to build mm/*/guest_*.o as the one use to
> >>> build every other *.o object. The consequence it that file symbols that
> >>> the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.
> >>>
> >>> (1) Currently we have those two file symbols:
> >>> guest_walk.c
> >>> guest_walk_2.o
> >>> (2) with CONFIG_ENFORCE_UNIQUE_SYMBOLS used on guest_walk.c, we will have:
> >>> arch/x86/mm/guest_walk.c
> >>> guest_walk_2.o
> >>>
> >>> The order in which those symbols are present may be different.
> >>>
> >>> Currently, in case (1) ./symbols chooses the *.o symbol (object file
> >>> name). But in case (2), may choose the *.c symbol (source file name with
> >>> path component) if it is first
> >>>
> >>> We want to have ./symbols choose the object file name symbol in both
> >>> cases.
> >>
> >> I guess the reason for wanting this is somehow connected to the
> >> statement at the beginning of the description, but I can't seem
> >> to be able to make the connection.
> >
> > I'm not sure I can explain it better.
> >
> > The "object file name" file symbol is used to distinguish between symbols
> > from all mm/*/guest_* objects. The other file symbol present in those
> > object is a "source file name without any path component symbol".
> >
> > But building those objects with the same rune as any other objects, and
> > having CONFIG_ENFORCE_UNIQUE_SYMBOLS=y, changes the file symbols present
> > in the resulting object. We still have the "object file name" symbol,
> > but now we also have "source file name with path components" symbol.
> > Unfortunately, all mm/*/guest_*.o in one directory are built from the
> > same source file, and thus have the same "source file name" symbol, but
> > have different "object file name" symbol. We still want to be able to
> > distinguish between guest_*.o in one dir, and the only way for that is
> > to use the "object file name" symbol.
>
> So where's the difference from how things work right now? The "same rune"
> aspect doesn't really change - right now we also build with effectively
> the same logic, just that -DGUEST_PAGING_LEVELS=... gets added. I guess
> it might help if you showed (for one particular example) how the set of
> file symbols changes from what we have now (with and without
> CONFIG_ENFORCE_UNIQUE_SYMBOLS=y) to what there would be with your changes
> to the symbols utility to what there will be with those changes.
The logic to build objects from C files changed in 81ecb38b83b0 ("build:
provide option to disambiguate symbol names"), with objects build with
__OBJECT_FILE__ explicitly left alone. So the logic is different now (at
least when CONFIG_ENFORCE_UNIQUE_SYMBOLS=y).
I did add the example of building arch/x86/mm/guest_walk_2.o to the
commit message, reworded below:
For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
this would be the difference of file symbol present in the object when
building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:
(1) Currently we have those two file symbols:
guest_walk.c
guest_walk_2.o
(2) When building with the same rune, we will have:
arch/x86/mm/guest_walk.c
guest_walk_2.o
The order in which those symbols are present may be different. Building
without CONFIG_ENFORCE_UNIQUE_SYMBOLS will result in (1).
> >>> So this patch changes that ./symbols prefer the "object file
> >>> name" symbol over the "source file name with path component" symbols.
> >>>
> >>> The new intended order of preference is:
> >>> - first object file name symbol
> >>> - first source file name with path components symbol
> >>> - last source file name without any path component symbol
> >>
> >> Isn't this going to lead to ambiguities again when
> >> CONFIG_ENFORCE_UNIQUE_SYMBOLS? Several object files (in different
> >> dirs) are named the same, after all. Static symbols with the same
> >> name in such objects would hence resolve to the same kallsyms
> >> name.
> >
> > "object file name" symbol are only present in mm/*/guest_*.o objects,
> > they all have different basenames. There is no ambiguity here.
>
> At least not right now, I see. Could you make this aspect more explicit
> by adding something like "(present only in object files produced from
> multiply compiled sources)" to the first bullet point?
Will do.
--
Anthony PERARD
[xen-4.10-testing test] 149676: tolerable trouble: fail/pass/starved - PUSHED
flight 149676 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/149676/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-xtf-amd64-amd64-5 51 xtf/test-hvm64-lbr-tsx-vmentry fail in 149650 pass in 149676 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-saverestore.2 fail pass in 149650 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail pass in 149650 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail in 149650 never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail like 146076 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 146101 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-thunderx 2 hosts-allocate starved n/a version targeted for testing: xen 24d62e126296b3f67dabdd49887d41d8ed69487f baseline version: xen 49a5d6e92317a7d9acbf0bdbd25b2809dfd84260 Last test of basis 146101 2020-01-15 04:00:32 Z 92 days Testing same since 149650 2020-04-14 13:35:36 Z2 days2 attempts Peop
Re: [XEN PATCH v4 18/18] build, include: rework compat-build-header.py
On 16.04.2020 16:17, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 03:56:02PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:31, Anthony PERARD wrote: >>> Replace a mix of shell script and python script by all python script. >>> >>> Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the >>> linemarkers generated by the preprocessor. But adding -P inhibit there >>> generation, thus the grep isn't needed anymore. >>> >>> gcc -E -P and clang -E -P have different behavior. While both don't >>> generates linemarkers, gcc also removes all empty lines while clang >>> keep them all. We don't need those empty lines, so we don't generates >>> them in the final compat/%.h headers. (This replace `uniq` which was >>> only de-duplicating empty line.) >>> >>> The only changes in the final generated headers it that they don't >>> have empty lines anymore. >> >> Making them harder to read? While typically no-one needs to look at >> their contents, in case of problems it helps if generated files are >> half way accessible to a human as well. > > I do think they are still readable. Those empty lines don't add much. > There are so many of them that a `uniq` is needed... > > For example, with dm_op.h, we have this: Let me take a different example, grant_table.h: Not all of the blank lines it has are useful, but I think the file would suffer if all of them got removed. Jan
Re: [XEN PATCH v4 16/18] build,xsm: Fix multiple call
On 16.04.2020 15:02, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 03:28:06PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:31, Anthony PERARD wrote: >>> Both script mkflask.sh and mkaccess_vector.sh generates multiple >>> files. Exploits the 'multi-target pattern rule' trick to call each >>> scripts only once. >> >> Isn't this a general fix, which may even want backporting? If so, >> this would better be at or near the beginning of the series. > > It is mostly a performance improvement, avoiding doing the same thing > several time. I don't think anything bad happens from concurrent calls, > or we would already have bug report I think. But I can try to move the > patch up. Up to three processes in parallel writing to the same file(s) is almost certainly a recipe for eventual / random breakage. Jan
Re: [XEN PATCH v4 15/18] xen/build: use if_changed to build guest_%.o
On 16.04.2020 14:57, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 03:02:21PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:30, Anthony PERARD wrote: >>> Use if_changed for building all guest_%.o objects, and make use of >>> command that already exist. >>> >>> This patch make a change to the way guest_%.o files are built, and now >>> run the same commands that enforce unique symbols. But with patch >>> "xen,symbols: rework file symbols selection", ./symbols should still >>> select the file symbols directive intended to be used for guest_%.o >>> objects. >> >> I'm having trouble making the connection between the change to the >> symbols tool and the adjustments made here: > > The change to symbol tool is to allow this change. I've been understanding the fact, but I still don't understand why the adjustment to that tool is necessary for the change here to be made. >>> --- a/xen/arch/x86/mm/Makefile >>> +++ b/xen/arch/x86/mm/Makefile >>> @@ -11,11 +11,13 @@ obj-y += p2m.o p2m-pt.o >>> obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o >>> obj-y += paging.o >>> >>> -guest_walk_%.o: guest_walk.c Makefile >>> - $(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ >> >> The original rules didn't do anything special to arrange for the >> resulting kallsyms names; these arrangements instead live at the >> top of the respective source files, in the form of asm()-s. > > They still are. I try to consolidate the number of location which have > command that build a target. Those guest_%.o object aren't special > enough to deserve to be built in a different way than every other > object. They do need a different make rule, but they can use the same > command. Again, I understand what the goal is, but not what it is that changes (and why) in the produced file symbols, making the utility adjustment necessary. I guess it's obvious to you, but it looks as if I was dense, sorry. Jan
Re: [XEN PATCH v4 14/18] xen,symbols: rework file symbols selection
On 16.04.2020 14:44, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 02:54:35PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:30, Anthony PERARD wrote: >>> We want to use the same rune to build mm/*/guest_*.o as the one use to >>> build every other *.o object. The consequence it that file symbols that >>> the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y. >>> >>> (1) Currently we have those two file symbols: >>> guest_walk.c >>> guest_walk_2.o >>> (2) with CONFIG_ENFORCE_UNIQUE_SYMBOLS used on guest_walk.c, we will have: >>> arch/x86/mm/guest_walk.c >>> guest_walk_2.o >>> >>> The order in which those symbols are present may be different. >>> >>> Currently, in case (1) ./symbols chooses the *.o symbol (object file >>> name). But in case (2), may choose the *.c symbol (source file name with >>> path component) if it is first >>> >>> We want to have ./symbols choose the object file name symbol in both >>> cases. >> >> I guess the reason for wanting this is somehow connected to the >> statement at the beginning of the description, but I can't seem >> to be able to make the connection. > > I'm not sure I can explain it better. > > The "object file name" file symbol is used to distinguish between symbols > from all mm/*/guest_* objects. The other file symbol present in those > object is a "source file name without any path component symbol". > > But building those objects with the same rune as any other objects, and > having CONFIG_ENFORCE_UNIQUE_SYMBOLS=y, changes the file symbols present > in the resulting object. We still have the "object file name" symbol, > but now we also have "source file name with path components" symbol. > Unfortunately, all mm/*/guest_*.o in one directory are built from the > same source file, and thus have the same "source file name" symbol, but > have different "object file name" symbol. We still want to be able to > distinguish between guest_*.o in one dir, and the only way for that is > to use the "object file name" symbol. So where's the difference from how things work right now? The "same rune" aspect doesn't really change - right now we also build with effectively the same logic, just that -DGUEST_PAGING_LEVELS=... gets added. I guess it might help if you showed (for one particular example) how the set of file symbols changes from what we have now (with and without CONFIG_ENFORCE_UNIQUE_SYMBOLS=y) to what there would be with your changes to the symbols utility to what there will be with those changes. >>> So this patch changes that ./symbols prefer the "object file >>> name" symbol over the "source file name with path component" symbols. >>> >>> The new intended order of preference is: >>> - first object file name symbol >>> - first source file name with path components symbol >>> - last source file name without any path component symbol >> >> Isn't this going to lead to ambiguities again when >> CONFIG_ENFORCE_UNIQUE_SYMBOLS? Several object files (in different >> dirs) are named the same, after all. Static symbols with the same >> name in such objects would hence resolve to the same kallsyms >> name. > > "object file name" symbol are only present in mm/*/guest_*.o objects, > they all have different basenames. There is no ambiguity here. At least not right now, I see. Could you make this aspect more explicit by adding something like "(present only in object files produced from multiply compiled sources)" to the first bullet point? Jan
Re: [XEN PATCH v4 18/18] build, include: rework compat-build-header.py
On Wed, Apr 08, 2020 at 03:56:02PM +0200, Jan Beulich wrote:
> On 31.03.2020 12:31, Anthony PERARD wrote:
> > Replace a mix of shell script and python script by all python script.
> >
> > Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the
> > linemarkers generated by the preprocessor. But adding -P inhibit there
> > generation, thus the grep isn't needed anymore.
> >
> > gcc -E -P and clang -E -P have different behavior. While both don't
> > generates linemarkers, gcc also removes all empty lines while clang
> > keep them all. We don't need those empty lines, so we don't generates
> > them in the final compat/%.h headers. (This replace `uniq` which was
> > only de-duplicating empty line.)
> >
> > The only changes in the final generated headers it that they don't
> > have empty lines anymore.
>
> Making them harder to read? While typically no-one needs to look at
> their contents, in case of problems it helps if generated files are
> half way accessible to a human as well.
I do think they are still readable. Those empty lines don't add much.
There are so many of them that a `uniq` is needed...
For example, with dm_op.h, we have this:
<<< before
#pragma pack(4)
typedef uint16_t ioservid_compat_t;
struct compat_dm_op_create_ioreq_server {
uint8_t handle_bufioreq;
uint8_t pad[3];
ioservid_compat_t id;
};
struct compat_dm_op_get_ioreq_server_info {
ioservid_compat_t id;
uint16_t flags;
evtchn_port_compat_t bufioreq_port;
uint64_t ioreq_gfn;
uint64_t bufioreq_gfn;
};
struct compat_dm_op_ioreq_server_range {
ioservid_compat_t id;
uint16_t pad;
uint32_t type;
uint64_t start, end;
};
===
#pragma pack(4)
typedef uint16_t ioservid_compat_t;
struct compat_dm_op_create_ioreq_server {
uint8_t handle_bufioreq;
uint8_t pad[3];
ioservid_compat_t id;
};
struct compat_dm_op_get_ioreq_server_info {
ioservid_compat_t id;
uint16_t flags;
evtchn_port_compat_t bufioreq_port;
uint64_t ioreq_gfn;
uint64_t bufioreq_gfn;
};
struct compat_dm_op_ioreq_server_range {
ioservid_compat_t id;
uint16_t pad;
uint32_t type;
uint64_t start, end;
};
>>> after
Thanks,
--
Anthony PERARD
[PATCH v10 0/3] x86/guest: use assisted TLB flush in guest mode
Hello, This is the remaining of the assisted TLB flush series. This last set of patches enable the usage of the Xen assisted flush when running nested on Xen. Thanks, Roger. Roger Pau Monne (3): x86/tlb: introduce a flush HVM ASIDs flag x86/tlb: allow disabling the TLB clock x86/tlb: use Xen L0 assisted TLB flush when available xen/arch/x86/flushtlb.c| 37 -- xen/arch/x86/guest/hypervisor.c| 14 ++ xen/arch/x86/guest/xen/xen.c | 6 + xen/arch/x86/mm/hap/hap.c | 8 +++--- xen/arch/x86/mm/hap/nested_hap.c | 2 +- xen/arch/x86/mm/p2m-pt.c | 5 ++-- xen/arch/x86/mm/paging.c | 2 +- xen/arch/x86/mm/shadow/common.c| 18 ++--- xen/arch/x86/mm/shadow/hvm.c | 2 +- xen/arch/x86/mm/shadow/multi.c | 16 +-- xen/arch/x86/mm/shadow/private.h | 6 + xen/arch/x86/smp.c | 7 + xen/include/asm-x86/flushtlb.h | 25 - xen/include/asm-x86/guest/hypervisor.h | 17 14 files changed, 130 insertions(+), 35 deletions(-) -- 2.26.0
[PATCH v10 1/3] x86/tlb: introduce a flush HVM ASIDs flag
Introduce a specific flag to request a HVM guest linear TLB flush,
which is an ASID/VPID tickle that forces a guest linear to guest
physical TLB flush for all HVM guests.
This was previously unconditionally done in each pre_flush call, but
that's not required: HVM guests not using shadow don't require linear
TLB flushes as Xen doesn't modify the guest page tables in that case
(ie: when using HAP). Note that shadow paging code already takes care
of issuing the necessary flushes when the shadow page tables are
modified.
In order to keep the previous behavior modify all shadow code TLB
flushes to also flush the guest linear to physical TLB if the guest is
HVM. I haven't looked at each specific shadow code TLB flush in order
to figure out whether it actually requires a guest TLB flush or not,
so there might be room for improvement in that regard.
Also perform ASID/VPID flushes when modifying the p2m tables as it's a
requirement for AMD hardware. Finally keep the flush in
switch_cr3_cr4, as it's not clear whether code could rely on
switch_cr3_cr4 also performing a guest linear TLB flush. A following
patch can remove the ASID/VPID tickle from switch_cr3_cr4 if found to
not be necessary.
Signed-off-by: Roger Pau Monné
---
Changes since v9:
- Introduce and use guest_flush_tlb_mask and sh_flush_local.
- Add a local domain variable to p2m_pt_change_entry_type_global.
Changes since v8:
- Don't flush host TLB on HAP changes.
- Introduce a helper for shadow changes that only flushes ASIDs/VPIDs
when the guest is HVM.
- Introduce a helper for HAP that only flushes ASIDs/VPIDs.
Changes since v7:
- Do not perform an ASID flush in filtered_flush_tlb_mask: the
requested flush is related to the page need_tlbflush field and not
to p2m changes (applies to both callers).
Changes since v6:
- Add ASID/VPID flushes when modifying the p2m.
- Keep the ASID/VPID flush in switch_cr3_cr4.
Changes since v5:
- Rename FLUSH_GUESTS_TLB to FLUSH_HVM_ASID_CORE.
- Clarify commit message.
- Define FLUSH_HVM_ASID_CORE to 0 when !CONFIG_HVM.
---
xen/arch/x86/flushtlb.c | 18 --
xen/arch/x86/mm/hap/hap.c| 8
xen/arch/x86/mm/hap/nested_hap.c | 2 +-
xen/arch/x86/mm/p2m-pt.c | 5 +++--
xen/arch/x86/mm/paging.c | 2 +-
xen/arch/x86/mm/shadow/common.c | 18 +-
xen/arch/x86/mm/shadow/hvm.c | 2 +-
xen/arch/x86/mm/shadow/multi.c | 16
xen/arch/x86/mm/shadow/private.h | 6 ++
xen/include/asm-x86/flushtlb.h | 8
10 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 03f92c23dc..7d261aef32 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -7,6 +7,7 @@
* Copyright (c) 2003-2006, K A Fraser
*/
+#include
#include
#include
#include
@@ -59,8 +60,6 @@ static u32 pre_flush(void)
raise_softirq(NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ);
skip_clocktick:
-hvm_flush_guest_tlbs();
-
return t2;
}
@@ -118,6 +117,7 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
local_irq_save(flags);
t = pre_flush();
+hvm_flush_guest_tlbs();
old_cr4 = read_cr4();
ASSERT(!(old_cr4 & X86_CR4_PCIDE) || !(old_cr4 & X86_CR4_PGE));
@@ -221,6 +221,9 @@ unsigned int flush_area_local(const void *va, unsigned int
flags)
do_tlb_flush();
}
+if ( flags & FLUSH_HVM_ASID_CORE )
+hvm_flush_guest_tlbs();
+
if ( flags & FLUSH_CACHE )
{
const struct cpuinfo_x86 *c = ¤t_cpu_data;
@@ -254,3 +257,14 @@ unsigned int flush_area_local(const void *va, unsigned int
flags)
return flags;
}
+
+void guest_flush_tlb_mask(const struct domain *d, const cpumask_t *mask)
+{
+unsigned int flags = (is_pv_domain(d) || paging_mode_shadow(d) ? FLUSH_TLB
+ : 0) |
+ (is_hvm_domain(d) && cpu_has_svm ? FLUSH_HVM_ASID_CORE
+ : 0);
+
+if ( flags )
+flush_mask(mask, flags);
+}
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 052ae35c6f..f7218a86d6 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -118,7 +118,7 @@ int hap_track_dirty_vram(struct domain *d,
p2m_change_type_range(d, begin_pfn, begin_pfn + nr,
p2m_ram_rw, p2m_ram_logdirty);
-flush_tlb_mask(d->dirty_cpumask);
+guest_flush_tlb_mask(d, d->dirty_cpumask);
memset(dirty_bitmap, 0xff, size); /* consider all pages dirty */
}
@@ -205,7 +205,7 @@ static int hap_enable_log_dirty(struct domain *d, bool_t
log_global)
* to be read-only, or via hardware-assisted log-dirty.
*/
p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_logdirty);
-flush_tlb_mask(d->dirty_cpumask);
+g
[PATCH v10 2/3] x86/tlb: allow disabling the TLB clock
The TLB clock is helpful when running Xen on bare metal because when
doing a TLB flush each CPU is IPI'ed and can keep a timestamp of the
last flush.
This is not the case however when Xen is running virtualized, and the
underlying hypervisor provides mechanism to assist in performing TLB
flushes: Xen itself for example offers a HVMOP_flush_tlbs hypercall in
order to perform a TLB flush without having to IPI each CPU. When
using such mechanisms it's no longer possible to keep a timestamp of
the flushes on each CPU, as they are performed by the underlying
hypervisor.
Offer a boolean in order to signal Xen that the timestamped TLB
shouldn't be used. This avoids keeping the timestamps of the flushes,
and also forces NEED_FLUSH to always return true.
No functional change intended, as this change doesn't introduce any
user that disables the timestamped TLB.
Signed-off-by: Roger Pau Monné
Reviewed-by: Wei Liu
Acked-by: Jan Beulich
---
xen/arch/x86/flushtlb.c| 19 +--
xen/include/asm-x86/flushtlb.h | 17 -
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 7d261aef32..e1b1e98c23 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -33,6 +33,9 @@
u32 tlbflush_clock = 1U;
DEFINE_PER_CPU(u32, tlbflush_time);
+/* Signals whether the TLB flush clock is in use. */
+bool __read_mostly tlb_clk_enabled = true;
+
/*
* pre_flush(): Increment the virtual TLB-flush clock. Returns new clock value.
*
@@ -83,12 +86,13 @@ static void post_flush(u32 t)
static void do_tlb_flush(void)
{
unsigned long flags, cr4;
-u32 t;
+u32 t = 0;
/* This non-reentrant function is sometimes called in interrupt context. */
local_irq_save(flags);
-t = pre_flush();
+if ( tlb_clk_enabled )
+t = pre_flush();
if ( use_invpcid )
invpcid_flush_all();
@@ -100,7 +104,8 @@ static void do_tlb_flush(void)
else
write_cr3(read_cr3());
-post_flush(t);
+if ( tlb_clk_enabled )
+post_flush(t);
local_irq_restore(flags);
}
@@ -108,7 +113,7 @@ static void do_tlb_flush(void)
void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
{
unsigned long flags, old_cr4;
-u32 t;
+u32 t = 0;
/* Throughout this function we make this assumption: */
ASSERT(!(cr4 & X86_CR4_PCIDE) || !(cr4 & X86_CR4_PGE));
@@ -116,7 +121,8 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
/* This non-reentrant function is sometimes called in interrupt context. */
local_irq_save(flags);
-t = pre_flush();
+if ( tlb_clk_enabled )
+t = pre_flush();
hvm_flush_guest_tlbs();
old_cr4 = read_cr4();
@@ -169,7 +175,8 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
if ( cr4 & X86_CR4_PCIDE )
invpcid_flush_all_nonglobals();
-post_flush(t);
+if ( tlb_clk_enabled )
+post_flush(t);
local_irq_restore(flags);
}
diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index 50df468c16..d5ca4bad57 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -21,10 +21,21 @@ extern u32 tlbflush_clock;
/* Time at which each CPU's TLB was last flushed. */
DECLARE_PER_CPU(u32, tlbflush_time);
-#define tlbflush_current_time() tlbflush_clock
+/* TLB clock is in use. */
+extern bool tlb_clk_enabled;
+
+static inline uint32_t tlbflush_current_time(void)
+{
+/* Returning 0 from tlbflush_current_time will always force a flush. */
+return tlb_clk_enabled ? tlbflush_clock : 0;
+}
static inline void page_set_tlbflush_timestamp(struct page_info *page)
{
+/* Avoid the write if the TLB clock is disabled. */
+if ( !tlb_clk_enabled )
+return;
+
/*
* Prevent storing a stale time stamp, which could happen if an update
* to tlbflush_clock plus a subsequent flush IPI happen between the
@@ -67,6 +78,10 @@ static inline void tlbflush_filter(cpumask_t *mask, uint32_t
page_timestamp)
{
unsigned int cpu;
+/* Short-circuit: there's no need to iterate if the clock is disabled. */
+if ( !tlb_clk_enabled )
+return;
+
for_each_cpu ( cpu, mask )
if ( !NEED_FLUSH(per_cpu(tlbflush_time, cpu), page_timestamp) )
__cpumask_clear_cpu(cpu, mask);
--
2.26.0
[PATCH v10 3/3] x86/tlb: use Xen L0 assisted TLB flush when available
Use Xen's L0 HVMOP_flush_tlbs hypercall in order to perform flushes.
This greatly increases the performance of TLB flushes when running
with a high amount of vCPUs as a Xen guest, and is specially important
when running in shim mode.
The following figures are from a PV guest running `make -j32 xen` in
shim mode with 32 vCPUs and HAP.
Using x2APIC and ALLBUT shorthand:
real4m35.973s
user4m35.110s
sys 36m24.117s
Using L0 assisted flush:
real1m2.596s
user4m34.818s
sys 5m16.374s
The implementation adds a new hook to hypervisor_ops so other
enlightenments can also implement such assisted flush just by filling
the hook.
Note that the Xen implementation completely ignores the dirty CPU mask
and the linear address passed in, and always performs a global TLB
flush on all vCPUs. This is a limitation of the hypercall provided by
Xen. Also note that local TLB flushes are not performed using the
assisted TLB flush, only remote ones.
Signed-off-by: Roger Pau Monné
Reviewed-by: Wei Liu
Reviewed-by: Jan Beulich
---
Changes since v5:
- Clarify commit message.
- Test for assisted flush at setup, do this for all hypervisors.
- Return EOPNOTSUPP if assisted flush is not available.
Changes since v4:
- Adjust order calculation.
Changes since v3:
- Use an alternative call for the flush hook.
Changes since v1:
- Add a L0 assisted hook to hypervisor ops.
---
xen/arch/x86/guest/hypervisor.c| 14 ++
xen/arch/x86/guest/xen/xen.c | 6 ++
xen/arch/x86/smp.c | 7 +++
xen/include/asm-x86/guest/hypervisor.h | 17 +
4 files changed, 44 insertions(+)
diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c
index 647cdb1367..e46de42ded 100644
--- a/xen/arch/x86/guest/hypervisor.c
+++ b/xen/arch/x86/guest/hypervisor.c
@@ -18,6 +18,7 @@
*
* Copyright (c) 2019 Microsoft.
*/
+#include
#include
#include
@@ -51,6 +52,10 @@ void __init hypervisor_setup(void)
{
if ( ops.setup )
ops.setup();
+
+/* Check if assisted flush is available and disable the TLB clock if so. */
+if ( !hypervisor_flush_tlb(cpumask_of(smp_processor_id()), NULL, 0) )
+tlb_clk_enabled = false;
}
int hypervisor_ap_setup(void)
@@ -73,6 +78,15 @@ void __init hypervisor_e820_fixup(struct e820map *e820)
ops.e820_fixup(e820);
}
+int hypervisor_flush_tlb(const cpumask_t *mask, const void *va,
+ unsigned int order)
+{
+if ( ops.flush_tlb )
+return alternative_call(ops.flush_tlb, mask, va, order);
+
+return -EOPNOTSUPP;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
index e74fd1e995..3bc01c8723 100644
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -324,12 +324,18 @@ static void __init e820_fixup(struct e820map *e820)
pv_shim_fixup_e820(e820);
}
+static int flush_tlb(const cpumask_t *mask, const void *va, unsigned int order)
+{
+return xen_hypercall_hvm_op(HVMOP_flush_tlbs, NULL);
+}
+
static const struct hypervisor_ops __initconstrel ops = {
.name = "Xen",
.setup = setup,
.ap_setup = ap_setup,
.resume = resume,
.e820_fixup = e820_fixup,
+.flush_tlb = flush_tlb,
};
const struct hypervisor_ops *__init xg_probe(void)
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index bcead5d01b..1d9fec65de 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -268,6 +269,12 @@ void flush_area_mask(const cpumask_t *mask, const void
*va, unsigned int flags)
if ( (flags & ~FLUSH_ORDER_MASK) &&
!cpumask_subset(mask, cpumask_of(cpu)) )
{
+if ( cpu_has_hypervisor &&
+ !(flags & ~(FLUSH_TLB | FLUSH_TLB_GLOBAL | FLUSH_VA_VALID |
+ FLUSH_ORDER_MASK)) &&
+ !hypervisor_flush_tlb(mask, va, (flags - 1) & FLUSH_ORDER_MASK) )
+return;
+
spin_lock(&flush_lock);
cpumask_and(&flush_cpumask, mask, &cpu_online_map);
cpumask_clear_cpu(cpu, &flush_cpumask);
diff --git a/xen/include/asm-x86/guest/hypervisor.h
b/xen/include/asm-x86/guest/hypervisor.h
index ade10e74ea..77a1d21824 100644
--- a/xen/include/asm-x86/guest/hypervisor.h
+++ b/xen/include/asm-x86/guest/hypervisor.h
@@ -19,6 +19,8 @@
#ifndef __X86_HYPERVISOR_H__
#define __X86_HYPERVISOR_H__
+#include
+
#include
struct hypervisor_ops {
@@ -32,6 +34,8 @@ struct hypervisor_ops {
void (*resume)(void);
/* Fix up e820 map */
void (*e820_fixup)(struct e820map *e820);
+/* L0 assisted TLB flush */
+int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int
order);
};
#ifdef CONFIG_GUEST
@@ -41,6 +45,14 @@ void hypervisor_setup(void);
int hypervisor_ap_setup(void);
void hypervisor_resume(void);
void hypervisor_e820_fixup(struct e820m
Re: [PATCH] mini-os: allow 4096 event channels for 64-bit mini-os
On Thu, Apr 16, 2020 at 02:29:18PM +0200, Samuel Thibault wrote: > Juergen Gross, le jeu. 16 avril 2020 14:27:00 +0200, a ecrit: > > Limiting the number of event channels to 1024 is fine for 32-bit > > builds, but not for 64-bit ones. This might be a problem when using > > Xenstore-stubdom as the number of domains which can be supported is > > then limited to a little bit more than 1000. > > > > So raise the number of event channels to 4096 in 64-bit builds. > > > > Signed-off-by: Juergen Gross > > Reviewed-by: Samuel Thibault Applied.
Re: [PATCH v2] mini-os: use -m elf_i386 for final linking
On Thu, Apr 16, 2020 at 03:17:15PM +0200, Juergen Gross wrote: > Using the standard -m elf_x86_64 for 64-bit mini-os results in the > first section (.text) to start only at offset 2MB in the binary file. > > Quoting Andrew Cooper on that topic: > > Specifically, 64bit emulation appears to include "align primary > sections to 2M so your OS can make better use of superpages even when > mmap()ing", with no way I can spot to override this in the linker > file. > > Using -m elf_i386 avoids that problem without any visible disadvantage. > > Signed-off-by: Juergen Gross Applied with Samuel's Rb from v1. Wei.
Re: [PATCH] mini-os: provide binary without debug information
On Thu, Apr 16, 2020 at 02:30:02PM +0200, Samuel Thibault wrote: > Juergen Gross, le jeu. 16 avril 2020 14:27:48 +0200, a ecrit: > > Provide a mini-os binary stripped from debug information in order to > > have a smaller resulting kernel file. The binary with debug > > information is kept with the suffix "-debug". > > > > Signed-off-by: Juergen Gross > > Reviewed-by: Samuel Thibault > Applied.
Re: [PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string
The changes in the OCaml C stubs look good to me. They don't really touch the interface but are mostly concerned with types on the C side by adding casts, const, and so on. The extended error handling is an improvement. -- Christian -- Acked-by: Christian Lindig From: Julien Grall Sent: 16 April 2020 12:25 To: xen-devel@lists.xenproject.org; Christian Lindig; David Scott Cc: dfaggi...@suse.com; Julien Grall; Stefano Stabellini; Volodymyr Babchuk; Jan Beulich; Andrew Cooper; Wei Liu; Roger Pau Monne; George Dunlap; Ian Jackson Subject: Re: [PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string Hi, Gentle ping. I am missing reviews for the OCaml part. Cheers, On 30/03/2020 20:21, Julien Grall wrote: > From: Julien Grall > > Hi all, > > This series is meant to solve the build issue reported by Dario when > using recent version of OCaml and -safe-string. > > I took the opportunity to harden a bit more the code by using const more > often. > > This series was only build tested. > > Cheers, > > Julien Grall (8): >xen/guest_access: Harden copy_to_guest_offset to prevent const dest > operand >xen/public: sysctl: set_parameter.params and debug.keys should be > const >tools/libxc: misc: Mark const the parameter 'keys' of > xc_send_debug_keys() >tools/libxc: misc: Mark const the parameter 'params' of > xc_set_parameters() >tools/ocaml: libxc: Check error return in stub_xc_vcpu_context_get() >tools/ocaml: libxb: Harden stub_header_of_string() >tools/ocaml: libxb: Avoid to use String_val() when value is bytes >tools/ocaml: Fix stubs build when OCaml has been compiled with > -safe-string > > tools/libxc/include/xenctrl.h | 4 ++-- > tools/libxc/xc_misc.c | 8 > tools/libxc/xc_private.h| 8 > tools/ocaml/libs/xb/xenbus_stubs.c | 6 +++--- > tools/ocaml/libs/xb/xs_ring_stubs.c | 12 ++-- > tools/ocaml/libs/xc/xenctrl_stubs.c | 6 -- > xen/include/asm-arm/guest_access.h | 2 +- > xen/include/asm-x86/guest_access.h | 2 +- > xen/include/public/sysctl.h | 4 ++-- > 9 files changed, 35 insertions(+), 17 deletions(-) > -- Julien Grall
[PATCH v2] mini-os: use -m elf_i386 for final linking
Using the standard -m elf_x86_64 for 64-bit mini-os results in the first section (.text) to start only at offset 2MB in the binary file. Quoting Andrew Cooper on that topic: Specifically, 64bit emulation appears to include "align primary sections to 2M so your OS can make better use of superpages even when mmap()ing", with no way I can spot to override this in the linker file. Using -m elf_i386 avoids that problem without any visible disadvantage. Signed-off-by: Juergen Gross --- arch/x86/arch.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/arch.mk b/arch/x86/arch.mk index c87885f..00028a0 100644 --- a/arch/x86/arch.mk +++ b/arch/x86/arch.mk @@ -26,3 +26,5 @@ ifeq ($(CONFIG_PARAVIRT),n) ARCH_LDFLAGS_FINAL := --oformat=elf32-i386 ARCH_AS_DEPS += x86_hvm.S endif + +ARCH_LDFLAGS_FINAL += -m elf_i386 -- 2.16.4
[xen-unstable test] 149667: tolerable FAIL - PUSHED
flight 149667 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/149667/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 16 guest-localmigrate fail REGR. vs. 149648 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 149648 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 149648 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 149648 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 149648 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 149648 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 149648 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 149648 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 149648 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 149648 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass version targeted for testing: xen fcd06227f83643194f8018f8dd37adce57763a61 baseline version: xen 0dbc112e727f6c17f306c864950bdf83dece5cd5 Last test of basis 149648 2020-04-14 13:06:14 Z2 days Testing same since 149667 2020-04-15 07:26:21 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Julien Grall Pawel Wieczorkiewicz Ross Lagerwall jobs: build-amd64-xsm
Re: [PATCH] mini-os: use -m elf_i386 for final linking
Jürgen Groß, le jeu. 16 avril 2020 14:58:34 +0200, a ecrit: > Specifically, 64bit emulation appears to include "align primary > sections to 2M so your OS can make better use of superpages even when > mmap()ing", with no way I can spot to override this in the linker > file." Ok, I see, I had indeed guessed that the 2M rounding probably had something to do with 2M pages. I'm afraid it may bite us back some day, but I'd say it is fine enough for now to include it, so Reviewed-by: Samuel Thibault with the information quoted above put in the changelog, please :) Samuel
Re: [XEN PATCH v4 17/18] build, include: rework compat-build-source.py
On Wed, Apr 08, 2020 at 03:53:01PM +0200, Jan Beulich wrote: > On 31.03.2020 12:31, Anthony PERARD wrote: > > Improvement are: > > - give the path to xlat.lst as argument > > - include `grep -v` in compat-build-source.py script, we don't need to > > write this in several scripted language. > > - have 'xlat.lst' path as a variable. > > The change looks okay, but I'm unsure whether it's really worthwhile. > I specifically dislike the last point above, as it makes things less > easy to read. I might be willing to ack a patch with this part taken > out again; faod I'm not meaning to nak the patch in its current form, > but I guess I'm also not going to ack it. I'll remove the last point from this patch. Thanks, -- Anthony PERARD
Re: [XEN PATCH v4 16/18] build,xsm: Fix multiple call
On Wed, Apr 08, 2020 at 03:28:06PM +0200, Jan Beulich wrote: > On 31.03.2020 12:31, Anthony PERARD wrote: > > Both script mkflask.sh and mkaccess_vector.sh generates multiple > > files. Exploits the 'multi-target pattern rule' trick to call each > > scripts only once. > > Isn't this a general fix, which may even want backporting? If so, > this would better be at or near the beginning of the series. It is mostly a performance improvement, avoiding doing the same thing several time. I don't think anything bad happens from concurrent calls, or we would already have bug report I think. But I can try to move the patch up. > > --- a/xen/xsm/flask/Makefile > > +++ b/xen/xsm/flask/Makefile > > @@ -26,14 +26,14 @@ mkflask := policy/mkflask.sh > > quiet_cmd_mkflask = MKFLASK $@ > > cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND) > > > > -$(FLASK_H_FILES): $(FLASK_H_DEPEND) $(mkflask) FORCE > > +$(patsubst include/%,\%/%,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) > > FORCE > > Since what $(FLASK_H_FILES) contains is well under our control, > how about the simpler > > $(subst include/,%/,$(FLASK_H_FILES)): ... > > ? Preferably with this and preferably with it moved ahead > Reviewed-by: Jan Beulich I'll do that, thanks, -- Anthony PERARD
Re: [PATCH] mini-os: use -m elf_i386 for final linking
On 16.04.20 14:46, Samuel Thibault wrote:
Juergen Gross, le jeu. 16 avril 2020 14:27:31 +0200, a ecrit:
Using the standard -m elf_x86_64 for 64-bit mini-os results in the
first section (.text) to start only at offset 2MB in the binary file.
? I'm not seeing this on my system:
0 .text 0001933a 1000 2**12
CONTENTS, ALLOC, LOAD, READONLY, CODE
# readelf -S mini-os
There are 19 section headers, starting at offset 0x245e88:
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[ 0] NULL
0 0 0
[ 1] .text PROGBITS 0020
00017d07 AX 0 0 4096
[ 2] .rodata PROGBITS 00017d20 00217d20
so only 4K offset in the file, the file ends up being 135K big after
stripping.
Lucky you. :-)
Might be specific to the linker used.
Using -m elf_i386 avoids that problem without any visible disadvantage.
Using a 32bit emulation for a 64bit binary? This looks very odd to me?
(and probably fragile)
That is only the final linking process, the option must not be used
earlier (I had linking errors in that case).
I'd like to know more where this 2MB binary file offset is coming from,
since AIUI it'd basically impact all binaries built by the toolchain of
your system, not just mini-os, and I don't think the maintainers of your
system want that :)
Andrew Cooper gave me the hint how to solve the problem. He has seen it
as well and told me (via IRC):
"I actually figured that out while hacking up a KVM-friendly version of
XTF for Andy Luto. The linking -m elf_i386/elf_x86_64 option sets the
"target emulation" which is more than just "what this is compiled for".
I haven't yet cleaned up the patch for XTF (which also suffers the same
problem), but linking an ELF64 using -m elf_i386 will DTRT with no
other ill effects.
Sadly, LD's documentation about details like this (and the linker
script for that matter) are poor at best. Specifically, 64bit emulation
appears to include "align primary sections to 2M so your OS can make
better use of superpages even when mmap()ing", with no way I can spot
to override this in the linker file."
Juergen
Re: [XEN PATCH v4 15/18] xen/build: use if_changed to build guest_%.o
On Wed, Apr 08, 2020 at 03:02:21PM +0200, Jan Beulich wrote: > On 31.03.2020 12:30, Anthony PERARD wrote: > > Use if_changed for building all guest_%.o objects, and make use of > > command that already exist. > > > > This patch make a change to the way guest_%.o files are built, and now > > run the same commands that enforce unique symbols. But with patch > > "xen,symbols: rework file symbols selection", ./symbols should still > > select the file symbols directive intended to be used for guest_%.o > > objects. > > I'm having trouble making the connection between the change to the > symbols tool and the adjustments made here: The change to symbol tool is to allow this change. > > --- a/xen/arch/x86/mm/Makefile > > +++ b/xen/arch/x86/mm/Makefile > > @@ -11,11 +11,13 @@ obj-y += p2m.o p2m-pt.o > > obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o > > obj-y += paging.o > > > > -guest_walk_%.o: guest_walk.c Makefile > > - $(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ > > The original rules didn't do anything special to arrange for the > resulting kallsyms names; these arrangements instead live at the > top of the respective source files, in the form of asm()-s. They still are. I try to consolidate the number of location which have command that build a target. Those guest_%.o object aren't special enough to deserve to be built in a different way than every other object. They do need a different make rule, but they can use the same command. Thanks, -- Anthony PERARD
Re: [PATCH] mini-os: use -m elf_i386 for final linking
Juergen Gross, le jeu. 16 avril 2020 14:27:31 +0200, a ecrit: > Using the standard -m elf_x86_64 for 64-bit mini-os results in the > first section (.text) to start only at offset 2MB in the binary file. ? I'm not seeing this on my system: 0 .text 0001933a 1000 2**12 CONTENTS, ALLOC, LOAD, READONLY, CODE so only 4K offset in the file, the file ends up being 135K big after stripping. > Using -m elf_i386 avoids that problem without any visible disadvantage. Using a 32bit emulation for a 64bit binary? This looks very odd to me? (and probably fragile) I'd like to know more where this 2MB binary file offset is coming from, since AIUI it'd basically impact all binaries built by the toolchain of your system, not just mini-os, and I don't think the maintainers of your system want that :) Samuel
Re: [XEN PATCH v4 14/18] xen,symbols: rework file symbols selection
On Wed, Apr 08, 2020 at 02:54:35PM +0200, Jan Beulich wrote: > On 31.03.2020 12:30, Anthony PERARD wrote: > > We want to use the same rune to build mm/*/guest_*.o as the one use to > > build every other *.o object. The consequence it that file symbols that > > the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y. > > > > (1) Currently we have those two file symbols: > > guest_walk.c > > guest_walk_2.o > > (2) with CONFIG_ENFORCE_UNIQUE_SYMBOLS used on guest_walk.c, we will have: > > arch/x86/mm/guest_walk.c > > guest_walk_2.o > > > > The order in which those symbols are present may be different. > > > > Currently, in case (1) ./symbols chooses the *.o symbol (object file > > name). But in case (2), may choose the *.c symbol (source file name with > > path component) if it is first > > > > We want to have ./symbols choose the object file name symbol in both > > cases. > > I guess the reason for wanting this is somehow connected to the > statement at the beginning of the description, but I can't seem > to be able to make the connection. I'm not sure I can explain it better. The "object file name" file symbol is used to distinguish between symbols from all mm/*/guest_* objects. The other file symbol present in those object is a "source file name without any path component symbol". But building those objects with the same rune as any other objects, and having CONFIG_ENFORCE_UNIQUE_SYMBOLS=y, changes the file symbols present in the resulting object. We still have the "object file name" symbol, but now we also have "source file name with path components" symbol. Unfortunately, all mm/*/guest_*.o in one directory are built from the same source file, and thus have the same "source file name" symbol, but have different "object file name" symbol. We still want to be able to distinguish between guest_*.o in one dir, and the only way for that is to use the "object file name" symbol. > > So this patch changes that ./symbols prefer the "object file > > name" symbol over the "source file name with path component" symbols. > > > > The new intended order of preference is: > > - first object file name symbol > > - first source file name with path components symbol > > - last source file name without any path component symbol > > Isn't this going to lead to ambiguities again when > CONFIG_ENFORCE_UNIQUE_SYMBOLS? Several object files (in different > dirs) are named the same, after all. Static symbols with the same > name in such objects would hence resolve to the same kallsyms > name. "object file name" symbol are only present in mm/*/guest_*.o objects, they all have different basenames. There is no ambiguity here. Thanks, -- Anthony PERARD
Re: [PATCH] mini-os: provide binary without debug information
Juergen Gross, le jeu. 16 avril 2020 14:27:48 +0200, a ecrit: > Provide a mini-os binary stripped from debug information in order to > have a smaller resulting kernel file. The binary with debug > information is kept with the suffix "-debug". > > Signed-off-by: Juergen Gross Reviewed-by: Samuel Thibault > --- > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 6a05de6..be640cd 100644 > --- a/Makefile > +++ b/Makefile > @@ -167,7 +167,9 @@ $(OBJ_DIR)/arch/x86/minios-x86%.lds: > arch/x86/minios-x86.lds.S > $(OBJ_DIR)/$(TARGET): $(OBJS) $(APP_O) arch_lib > $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds > $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(APP_O) $(OBJS) $(LDARCHLIB) $(LDLIBS) > -o $@.o > $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o > - $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@ > + $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug > + strip -s $@-debug -o $@ > + gzip -n -f -9 -c $@-debug >$@-debug.gz > gzip -n -f -9 -c $@ >$@.gz > > .PHONY: config > -- > 2.16.4 >
Re: [PATCH] mini-os: allow 4096 event channels for 64-bit mini-os
Juergen Gross, le jeu. 16 avril 2020 14:27:00 +0200, a ecrit:
> Limiting the number of event channels to 1024 is fine for 32-bit
> builds, but not for 64-bit ones. This might be a problem when using
> Xenstore-stubdom as the number of domains which can be supported is
> then limited to a little bit more than 1000.
>
> So raise the number of event channels to 4096 in 64-bit builds.
>
> Signed-off-by: Juergen Gross
Reviewed-by: Samuel Thibault
> ---
> events.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/events.c b/events.c
> index 342aead..cdae90f 100644
> --- a/events.c
> +++ b/events.c
> @@ -23,7 +23,7 @@
> #include
> #include
>
> -#define NR_EVS 1024
> +#define NR_EVS EVTCHN_2L_NR_CHANNELS
>
> /* this represents a event handler. Chaining or sharing is not allowed */
> typedef struct _ev_action_t {
> --
> 2.16.4
>
[PATCH] mini-os: provide binary without debug information
Provide a mini-os binary stripped from debug information in order to have a smaller resulting kernel file. The binary with debug information is kept with the suffix "-debug". Signed-off-by: Juergen Gross --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6a05de6..be640cd 100644 --- a/Makefile +++ b/Makefile @@ -167,7 +167,9 @@ $(OBJ_DIR)/arch/x86/minios-x86%.lds: arch/x86/minios-x86.lds.S $(OBJ_DIR)/$(TARGET): $(OBJS) $(APP_O) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(APP_O) $(OBJS) $(LDARCHLIB) $(LDLIBS) -o $@.o $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o - $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@ + $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug + strip -s $@-debug -o $@ + gzip -n -f -9 -c $@-debug >$@-debug.gz gzip -n -f -9 -c $@ >$@.gz .PHONY: config -- 2.16.4
[PATCH] mini-os: use -m elf_i386 for final linking
Using the standard -m elf_x86_64 for 64-bit mini-os results in the first section (.text) to start only at offset 2MB in the binary file. Using -m elf_i386 avoids that problem without any visible disadvantage. Signed-off-by: Juergen Gross --- arch/x86/arch.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/arch.mk b/arch/x86/arch.mk index c87885f..00028a0 100644 --- a/arch/x86/arch.mk +++ b/arch/x86/arch.mk @@ -26,3 +26,5 @@ ifeq ($(CONFIG_PARAVIRT),n) ARCH_LDFLAGS_FINAL := --oformat=elf32-i386 ARCH_AS_DEPS += x86_hvm.S endif + +ARCH_LDFLAGS_FINAL += -m elf_i386 -- 2.16.4
[PATCH] mini-os: allow 4096 event channels for 64-bit mini-os
Limiting the number of event channels to 1024 is fine for 32-bit
builds, but not for 64-bit ones. This might be a problem when using
Xenstore-stubdom as the number of domains which can be supported is
then limited to a little bit more than 1000.
So raise the number of event channels to 4096 in 64-bit builds.
Signed-off-by: Juergen Gross
---
events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/events.c b/events.c
index 342aead..cdae90f 100644
--- a/events.c
+++ b/events.c
@@ -23,7 +23,7 @@
#include
#include
-#define NR_EVS 1024
+#define NR_EVS EVTCHN_2L_NR_CHANNELS
/* this represents a event handler. Chaining or sharing is not allowed */
typedef struct _ev_action_t {
--
2.16.4
RE: [PATCH v6 03/10] x86emul: support MOVDIR{I,64B} insns
> -Original Message-
> From: Jan Beulich
> Sent: 14 April 2020 12:45
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper ; Wei Liu ; Roger
> Pau Monne
> ; Paul Durrant
> Subject: [PATCH v6 03/10] x86emul: support MOVDIR{I,64B} insns
>
> Introduce a new blk() hook, paralleling the rmw() one in a certain way,
> but being intended for larger data sizes, and hence its HVM intermediate
> handling function doesn't fall back to splitting the operation if the
> requested virtual address can't be mapped.
>
> Note that SDM revision 071 doesn't specify exception behavior for
> ModRM.mod == 0b11; assuming #UD here.
>
> Signed-off-by: Jan Beulich
hvm/emulate part...
Reviewed-by: Paul Durrant
> ---
> v6: Fold MOVDIRI and MOVDIR64B changes again. Use blk() for both. All
> tags dropped.
> v5: Introduce/use ->blk() hook. Correct asm() operands.
> v4: Split MOVDIRI and MOVDIR64B and move this one ahead. Re-base.
> v3: Update description.
> ---
> (SDE: -tnt)
>
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -652,6 +652,18 @@ static int cmpxchg(
> return X86EMUL_OKAY;
> }
>
> +static int blk(
> +enum x86_segment seg,
> +unsigned long offset,
> +void *p_data,
> +unsigned int bytes,
> +uint32_t *eflags,
> +struct x86_emulate_state *state,
> +struct x86_emulate_ctxt *ctxt)
> +{
> +return x86_emul_blk((void *)offset, p_data, bytes, eflags, state, ctxt);
> +}
> +
> static int read_segment(
> enum x86_segment seg,
> struct segment_register *reg,
> @@ -2339,6 +2351,54 @@ int main(int argc, char **argv)
> goto fail;
> printf("okay\n");
>
> +emulops.blk = blk;
> +
> +printf("%-40s", "Testing movdiri %edx,(%ecx)...");
> +if ( stack_exec && cpu_has_movdiri )
> +{
> +instr[0] = 0x0f; instr[1] = 0x38; instr[2] = 0xf9; instr[3] = 0x11;
> +
> +regs.eip = (unsigned long)&instr[0];
> +regs.ecx = (unsigned long)memset(res, -1, 16);
> +regs.edx = 0x44332211;
> +
> +rc = x86_emulate(&ctxt, &emulops);
> +if ( (rc != X86EMUL_OKAY) ||
> + (regs.eip != (unsigned long)&instr[4]) ||
> + res[0] != 0x44332211 || ~res[1] )
> +goto fail;
> +printf("okay\n");
> +}
> +else
> +printf("skipped\n");
> +
> +printf("%-40s", "Testing movdir64b 144(%edx),%ecx...");
> +if ( stack_exec && cpu_has_movdir64b )
> +{
> +instr[0] = 0x66; instr[1] = 0x0f; instr[2] = 0x38; instr[3] = 0xf8;
> +instr[4] = 0x8a; instr[5] = 0x90; instr[8] = instr[7] = instr[6] = 0;
> +
> +regs.eip = (unsigned long)&instr[0];
> +for ( i = 0; i < 64; ++i )
> +res[i] = i - 20;
> +regs.edx = (unsigned long)res;
> +regs.ecx = (unsigned long)(res + 16);
> +
> +rc = x86_emulate(&ctxt, &emulops);
> +if ( (rc != X86EMUL_OKAY) ||
> + (regs.eip != (unsigned long)&instr[9]) ||
> + res[15] != -5 || res[32] != 12 )
> +goto fail;
> +for ( i = 16; i < 32; ++i )
> +if ( res[i] != i )
> +goto fail;
> +printf("okay\n");
> +}
> +else
> +printf("skipped\n");
> +
> +emulops.blk = NULL;
> +
> printf("%-40s", "Testing movq %mm3,(%ecx)...");
> if ( stack_exec && cpu_has_mmx )
> {
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -154,6 +154,8 @@ static inline bool xcr0_mask(uint64_t ma
> #define cpu_has_avx512_vnni (cp.feat.avx512_vnni && xcr0_mask(0xe6))
> #define cpu_has_avx512_bitalg (cp.feat.avx512_bitalg && xcr0_mask(0xe6))
> #define cpu_has_avx512_vpopcntdq (cp.feat.avx512_vpopcntdq &&
> xcr0_mask(0xe6))
> +#define cpu_has_movdiricp.feat.movdiri
> +#define cpu_has_movdir64b cp.feat.movdir64b
> #define cpu_has_avx512_4vnniw (cp.feat.avx512_4vnniw && xcr0_mask(0xe6))
> #define cpu_has_avx512_4fmaps (cp.feat.avx512_4fmaps && xcr0_mask(0xe6))
> #define cpu_has_avx512_bf16 (cp.feat.avx512_bf16 && xcr0_mask(0xe6))
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -250,12 +250,13 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
> # sure we pick up changes when the compiler used has changed.)
> ifeq ($(MAKECMDGOALS),asm-offsets.s)
>
> -as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
> +as-ISA-list := CLWB EPT FSGSBASE INVPCID MOVDIR RDRAND RDSEED SSE4_2 VMX
> XSAVEOPT
>
> CLWB-insn:= clwb (%rax)
> EPT-insn := invept (%rax),%rax
> FSGSBASE-insn:= rdfsbase %rax
> INVPCID-insn := invpcid (%rax),%rax
> +MOVDIR-insn := movdiri %rax,(%rax)
> RDRAND-insn := rdrand %eax
> RDSEED-insn := rdseed %eax
> SSE4_2-insn := crc32 %eax,%eax
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1409,6 +1409,44 @@ static int hvmemul_rmw(
> return rc;
> }
>
> +static int hvmemul_blk(
> +enum x86_segment se
Re: [[PATCH v3]] xen/guest_access: Harden *copy_to_guest_offset() to prevent const dest operand
On 16.04.2020 13:24, Julien Grall wrote:
> From: Julien Grall
>
> At the moment, *copy_to_guest_offset() will allow the hypervisor to copy
> data to guest handle marked const.
>
> Thankfully, no users of the helper will do that. Rather than hoping this
> can be caught during review, harden copy_to_guest_offset() so the build
> will fail if such users are introduced.
>
> There is no easy way to check whether a const is NULL in C99. The
> approach used is to introduce an unused variable that is non-const and
> assign the handle. If the handle were const, this would fail at build
> because without an explicit cast, it is not possible to assign a const
> variable to a non-const variable.
>
> Suggested-by: Jan Beulich
> Signed-off-by: Julien Grall
Reviewed-by: Jan Beulich
with one further remark:
> --- a/xen/include/asm-x86/guest_access.h
> +++ b/xen/include/asm-x86/guest_access.h
> @@ -87,6 +87,8 @@
> #define copy_to_guest_offset(hnd, off, ptr, nr) ({ \
> const typeof(*(ptr)) *_s = (ptr); \
> char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \
> +/* Check if the handle is not const */ \
> +void *__maybe_unused _t = (hnd).p; \
Not being a native speaker, to me "if" doesn't look appropriate
here. I'd use "that" instead, but you may want to confirm this.
Overall then maybe "Check that the handle is not for a const type"?
Jan
[PATCH] x86emul: SYSRET must change CPL
The special AMD behavior of leaving SS mostly alone wasn't really complete: We need to adjust CPL aka SS.DPL. Signed-off-by: Jan Beulich --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -6022,6 +6022,8 @@ x86_emulate( /* There's explicitly no RPL adjustment here. */ sreg.sel = (msr_val >> 48) + 8; +/* But DPL needs adjustment, for the new CPL to be correct. */ +sreg.dpl = 3; } #ifdef __x86_64__
Re: [XTF 4/4] setup: Setup PV console for HVM guests on xen >4.2
> On 16. Apr 2020, at 12:36, Andrew Cooper wrote:
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
>
>
>
> On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote:
>> @@ -272,9 +274,23 @@ void arch_setup(void)
>>
>> init_hypercalls();
>>
>> -if ( !is_initdomain() )
>> +xen_version = hypercall_xen_version(XENVER_version, NULL);
>> +if ( version )
>> +*version = xen_version;
>> +
>> +/*
>> + * The setup_pv_console function registers a writing function
>> + * that makes hvm guests crash on Xen 4.2
>
> This comment in particular is rather concerning. Even if there is a
> configuration issue in 4.2 stopping the PV console from being wired up
> properly, nothing involved in transmitting on the console should crash
> the guest.
>
I am again a little short on details here. Maybe Paul could chime in.
But, I vagualy remember it was something about the init_pv_console()’s:
if ( port >= (sizeof(shared_info.evtchn_pending) * CHAR_BIT) )
panic("evtchn %u out of evtchn_pending[] range\n", port);
> ~Andrew
Best Regards,
Pawel Wieczorkiewicz
wipa...@amazon.com
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Re: [XTF 3/4] Enabled serial writing for hvm guests
> On 16. Apr 2020, at 12:32, Andrew Cooper wrote:
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
>
>
>
> On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote:
>> From: Paul Semel
>>
>> setup.c: PV console writing is not working in Xen 4.2 for hvm
>> guests,
>
> What is not working about it?
>
Honestly I am little short on details here, I would have to ask Paul or refresh
my memory.
>> so we make xtf write to COM1 serial port to get its output
>>
>> Signed-off-by: Paul Semel
>> Signed-off-by: Pawel Wieczorkiewicz
>> ---
>> arch/x86/setup.c | 14 ++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
>> index 3c84e96..f6fa4df 100644
>> --- a/arch/x86/setup.c
>> +++ b/arch/x86/setup.c
>> @@ -238,6 +238,13 @@ static void qemu_console_write(const char *buf, size_t
>> len)
>> : "d" (0x12));
>> }
>>
>> +static void com1_write(const char *buf, size_t len)
>> +{
>> +asm volatile("rep; outsb"
>> + : "+S" (buf), "+c" (len)
>> + : "d" (0x3f8));
>
> Despite being 0x3f8, this really isn't uart-compatible COM1. I presume
> it only works because Qemu doesn't have any real THR delays in its
> emulation.
>
That can be. Nevertheless, it works for me[tm].
>> +}
>> +
>> static void xen_console_write(const char *buf, size_t len)
>> {
>> hypercall_console_write(buf, len);
>> @@ -246,7 +253,14 @@ static void xen_console_write(const char *buf, size_t
>> len)
>> void arch_setup(void)
>> {
>> if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
>> +{
>> register_console_callback(qemu_console_write);
>> +}
>> +
>> +if ( IS_DEFINED(CONFIG_HVM) )
>> +{
>> +register_console_callback(com1_write);
>
> This wires up 0x3f8 even for PVH guests, which I'm guessing isn't
> intentional? This should be part of the previous if(), but does beg the
> question what is wrong with the existing qemu console?
>
It turns out that both PVH and HVM guests are PVH ABI compatible,
but PVH does not need qemu console. As per:
01e16ceb arch/x86/hvm/head.S (Andrew Cooper2018-01-26 16:39:15
+ 36) /* All HVM XTF guests are compatible with the PVH ABI. */
In order to get serial console via qemu working, I always register com1
handler for both HVM and PVH. Register qemu console only for HVM guests.
> ~Andrew
I use the com1 to make qemu write console output to a file via:
serial=“file:/tmp/…”
Best Regards,
Pawel Wieczorkiewicz
wipa...@amazon.com
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Re: [XTF 2/4] lib: always append CR after LF in vsnprintf()
> On 16. Apr 2020, at 12:18, Andrew Cooper wrote: > > CAUTION: This email originated from outside of the organization. Do not click > links or open attachments unless you can confirm the sender and know the > content is safe. > > > > On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote: >> The explicit LFCR sequence guarantees proper line by line formatting >> in the output. >> The '\n' character alone on some terminals is not automatically >> converted to LFCR. >> >> Signed-off-by: Pawel Wieczorkiewicz > > Up until now, all console destinations have expected POSIX text semantics. > > I presume this is due to the COM1 use presented in the next patch? > No, this is not about that. > Unfortunately, this comes with collateral damage. > > # ./xtf-runner hvm64 example > Executing 'xl create -p tests/example/test-hvm64-example.cfg' > Executing 'xl console test-hvm64-example' > Executing 'xl unpause test-hvm64-example' > --- Xen Test Framework --- > > Found Xen: 4.14 > > Environment: HVM 64bit (Long mode 4 levels) > > Hello World > > Test result: SUCCESS > > > Combined test results: > test-hvm64-example CRASH > I never use xtf-runner script to execute tests. I do it the old fashion way: # xl create -c test-hvm64-example.cfg Parsing config from test-hvm64-example.cfg Guest cpuid information Native cpuid: : -> 000d:756e6547:6c65746e:49656e69 0001: -> 000306e4:00400800:f7ba2203:1fcbfbff 0002: -> 76036301:00f0b2ff::00ca 0003: -> ::: 0004: -> 7c000121:01c0003f:003f: 0004:0001 -> 7c000122:01c0003f:003f: 0004:0002 -> 7c000143:01c0003f:01ff: 0004:0003 -> 7c000163:04c0003f:4fff:0006 0004:0004 -> ::: 0005: -> 0040:0040:0003:1120 0006: -> 0077:0002:0009: 0007: -> :0281::9c000400 0008: -> ::: 0009: -> ::: 000a: -> 07300403:::0603 000b: -> ::: 000c: -> ::: 000d: -> 0007:0240:0340: 000d:0001 -> 0001::: 000d:0002 -> 0100:0240:: 4000: -> 4005:566e6558:65584d4d:4d4d566e
Re: [PATCH 01/17] xen/x86: Introduce helpers to generate/convert the CR3 from/to a MFN/GFN
Hi Jan,
On 30/03/2020 08:38, Jan Beulich wrote:
Maybe some variation thereof:
- hvm_cr3_to_gfn()/hvm_gfn_to_cr3(): Handle the CR3 for HVM guest
- {pv,compat}_mfn_to_cr3()/{pv,compat}_cr3_to_mfn(): Handle the CR3 for PV
guest
- cr3_to_mfn()/mfn_to_cr3(): To handle the host cr3
? This is because I'd prefer to avoid host_ prefixes (albeit I'm
not entirely opposed to such), and I'd also prefer to use xen_
prefixes as they're generally ambiguous as to what aspect of "Xen"
they actually mean.
I am happy with your suggested naming. I will have a look to see how
they fit in the tree and respin the series.
Cheers,
--
Julien Grall
[xen-unstable-smoke test] 149686: tolerable all pass - PUSHED
flight 149686 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/149686/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 615bfe42c6d183a0e54a0525ef82b58580d01619 baseline version: xen fcd06227f83643194f8018f8dd37adce57763a61 Last test of basis 149654 2020-04-14 16:01:51 Z1 days Testing same since 149686 2020-04-16 09:01:03 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Julien Grall jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git fcd06227f8..615bfe42c6 615bfe42c6d183a0e54a0525ef82b58580d01619 -> smoke
Re: [PATCH 0/8] Fix build with using OCaml 4.06.1 and -safe-string
Hi, Gentle ping. I am missing reviews for the OCaml part. Cheers, On 30/03/2020 20:21, Julien Grall wrote: From: Julien Grall Hi all, This series is meant to solve the build issue reported by Dario when using recent version of OCaml and -safe-string. I took the opportunity to harden a bit more the code by using const more often. This series was only build tested. Cheers, Julien Grall (8): xen/guest_access: Harden copy_to_guest_offset to prevent const dest operand xen/public: sysctl: set_parameter.params and debug.keys should be const tools/libxc: misc: Mark const the parameter 'keys' of xc_send_debug_keys() tools/libxc: misc: Mark const the parameter 'params' of xc_set_parameters() tools/ocaml: libxc: Check error return in stub_xc_vcpu_context_get() tools/ocaml: libxb: Harden stub_header_of_string() tools/ocaml: libxb: Avoid to use String_val() when value is bytes tools/ocaml: Fix stubs build when OCaml has been compiled with -safe-string tools/libxc/include/xenctrl.h | 4 ++-- tools/libxc/xc_misc.c | 8 tools/libxc/xc_private.h| 8 tools/ocaml/libs/xb/xenbus_stubs.c | 6 +++--- tools/ocaml/libs/xb/xs_ring_stubs.c | 12 ++-- tools/ocaml/libs/xc/xenctrl_stubs.c | 6 -- xen/include/asm-arm/guest_access.h | 2 +- xen/include/asm-x86/guest_access.h | 2 +- xen/include/public/sysctl.h | 4 ++-- 9 files changed, 35 insertions(+), 17 deletions(-) -- Julien Grall
[[PATCH v3]] xen/guest_access: Harden *copy_to_guest_offset() to prevent const dest operand
From: Julien Grall
At the moment, *copy_to_guest_offset() will allow the hypervisor to copy
data to guest handle marked const.
Thankfully, no users of the helper will do that. Rather than hoping this
can be caught during review, harden copy_to_guest_offset() so the build
will fail if such users are introduced.
There is no easy way to check whether a const is NULL in C99. The
approach used is to introduce an unused variable that is non-const and
assign the handle. If the handle were const, this would fail at build
because without an explicit cast, it is not possible to assign a const
variable to a non-const variable.
Suggested-by: Jan Beulich
Signed-off-by: Julien Grall
---
Changes in v3:
- Reduce the length of the comments and move it within the
macro.
Changes in v2:
- Use a void * variable to check the handle is not const.
---
xen/include/asm-arm/guest_access.h | 4
xen/include/asm-x86/guest_access.h | 4
2 files changed, 8 insertions(+)
diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 8997a1cbfe..97cf3384f1 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -78,6 +78,8 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa,
void *buf,
#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \
const typeof(*(ptr)) *_s = (ptr); \
char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \
+/* Check if the handle is not const */ \
+void *__maybe_unused _t = (hnd).p; \
((void)((hnd).p == (ptr))); \
raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \
})
@@ -127,6 +129,8 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t
ipa, void *buf,
#define __copy_to_guest_offset(hnd, off, ptr, nr) ({\
const typeof(*(ptr)) *_s = (ptr); \
char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \
+/* Check if the handle is not const */ \
+void *__maybe_unused _t = (hnd).p; \
((void)((hnd).p == (ptr))); \
__raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
})
diff --git a/xen/include/asm-x86/guest_access.h
b/xen/include/asm-x86/guest_access.h
index ca700c959a..6f5107951c 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -87,6 +87,8 @@
#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \
const typeof(*(ptr)) *_s = (ptr); \
char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \
+/* Check if the handle is not const */ \
+void *__maybe_unused _t = (hnd).p; \
((void)((hnd).p == (ptr))); \
raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \
})
@@ -137,6 +139,8 @@
#define __copy_to_guest_offset(hnd, off, ptr, nr) ({\
const typeof(*(ptr)) *_s = (ptr); \
char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \
+/* Check if the handle is not const */ \
+void *__maybe_unused _t = (hnd).p; \
((void)((hnd).p == (ptr))); \
__raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
})
--
2.17.1
Re: [XTF 4/4] setup: Setup PV console for HVM guests on xen >4.2
On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote: > @@ -272,9 +274,23 @@ void arch_setup(void) > > init_hypercalls(); > > -if ( !is_initdomain() ) > +xen_version = hypercall_xen_version(XENVER_version, NULL); > +if ( version ) > +*version = xen_version; > + > +/* > + * The setup_pv_console function registers a writing function > + * that makes hvm guests crash on Xen 4.2 This comment in particular is rather concerning. Even if there is a configuration issue in 4.2 stopping the PV console from being wired up properly, nothing involved in transmitting on the console should crash the guest. ~Andrew
Re: [XTF 3/4] Enabled serial writing for hvm guests
On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote:
> From: Paul Semel
>
> setup.c: PV console writing is not working in Xen 4.2 for hvm
> guests,
What is not working about it?
> so we make xtf write to COM1 serial port to get its output
>
> Signed-off-by: Paul Semel
> Signed-off-by: Pawel Wieczorkiewicz
> ---
> arch/x86/setup.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
> index 3c84e96..f6fa4df 100644
> --- a/arch/x86/setup.c
> +++ b/arch/x86/setup.c
> @@ -238,6 +238,13 @@ static void qemu_console_write(const char *buf, size_t
> len)
> : "d" (0x12));
> }
>
> +static void com1_write(const char *buf, size_t len)
> +{
> +asm volatile("rep; outsb"
> + : "+S" (buf), "+c" (len)
> + : "d" (0x3f8));
Despite being 0x3f8, this really isn't uart-compatible COM1. I presume
it only works because Qemu doesn't have any real THR delays in its
emulation.
> +}
> +
> static void xen_console_write(const char *buf, size_t len)
> {
> hypercall_console_write(buf, len);
> @@ -246,7 +253,14 @@ static void xen_console_write(const char *buf, size_t
> len)
> void arch_setup(void)
> {
> if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
> +{
> register_console_callback(qemu_console_write);
> +}
> +
> +if ( IS_DEFINED(CONFIG_HVM) )
> +{
> +register_console_callback(com1_write);
This wires up 0x3f8 even for PVH guests, which I'm guessing isn't
intentional? This should be part of the previous if(), but does beg the
question what is wrong with the existing qemu console?
~Andrew
Re: [XTF 2/4] lib: always append CR after LF in vsnprintf()
On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote:
> The explicit LFCR sequence guarantees proper line by line formatting
> in the output.
> The '\n' character alone on some terminals is not automatically
> converted to LFCR.
>
> Signed-off-by: Pawel Wieczorkiewicz
Up until now, all console destinations have expected POSIX text semantics.
I presume this is due to the COM1 use presented in the next patch?
Unfortunately, this comes with collateral damage.
# ./xtf-runner hvm64 example
Executing 'xl create -p tests/example/test-hvm64-example.cfg'
Executing 'xl console test-hvm64-example'
Executing 'xl unpause test-hvm64-example'
--- Xen Test Framework ---
Found Xen: 4.14
Environment: HVM 64bit (Long mode 4 levels)
Hello World
Test result: SUCCESS
Combined test results:
test-hvm64-example CRASH
which I believe is due to xenconsoled (or the intervening pty) also
expanding \n to \r\n (and "Test result:" no longer being on the final
line from xtf-runner's point of view). Xen also expands \n to \r\n for
the console, so ends up emitting \r\r\n.
Would it be better to have the com1 console handler do the expansion
locally, to avoid interfering with the semantics of every other
destination? That said...
> ---
> common/libc/vsnprintf.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c
> index a49fd30..3202137 100644
> --- a/common/libc/vsnprintf.c
> +++ b/common/libc/vsnprintf.c
> @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt,
> va_list args)
> if ( *fmt != '%' )
> {
> PUT(*fmt);
> +
> +/*
> + * The '\n' character alone on some terminals is not
> automatically
> + * converted to LFCR.
> + * The explicit LFCR sequence guarantees proper line by line
> + * formatting in the output.
> + */
> +if ( *fmt == '\n' && str < end )
> +PUT('\r');
... doesn't this end up putting out \n\r ?
~Andrew
> +
> continue;
> }
>
Re: [XEN PATCH v4 12/18] xen/build: factorise generation of the linker scripts
On Thu, Apr 16, 2020 at 09:36:15AM +0200, Jan Beulich wrote:
> On 15.04.2020 18:58, Anthony PERARD wrote:
> > On Wed, Apr 08, 2020 at 02:46:42PM +0200, Jan Beulich wrote:
> >> On 31.03.2020 12:30, Anthony PERARD wrote:
> >>> - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line
> >>> is
> >>> still mandatory for if_changed (or cmd) macro to work.
> >>
> >> I still don't believe in there being a need for "; \" there. This
> >> actually breaks things, after all:
> >>
> >>> --- a/xen/Rules.mk
> >>> +++ b/xen/Rules.mk
> >>> @@ -236,6 +236,12 @@ cmd_s_S = $(CPP) $(filter-out
> >>> -Wa$(comma)%,$(a_flags)) $< -o $@
> >>> %.s: %.S FORCE
> >>> $(call if_changed,cpp_s_S)
> >>>
> >>> +# Linker scripts, .lds.S -> .lds
> >>> +quiet_cmd_cc_lds_S = LDS $@
> >>> +cmd_cc_lds_S = $(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<;
> >>> \
> >>> +sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d
> >>> >$(dot-target).d.new; \
> >>> +mv -f $(dot-target).d.new $(dot-target).d
> >>
> >> if $(CPP) or sed fail, previously the whole rule would have failed,
> >> which no longer is the case with your use of semicolons. There
> >> ought to be a solution to this, ideally one better than adding
> >> "set -e" as the first command ("define" would at least deal with
> >> the multi-line make issue, but without it being clear to me why the
> >> semicolons would be needed I don't think I can suggest anything
> >> there at the moment).
> >
> > The only macro that will consumes cmd_cc_lds_S (and other cmd_*) is
> > "cmd", it is defined as:
> > cmd = @set -e; $(echo-cmd) $(cmd_$(1))
> > So, "set -e" is already there, and using semicolons in commands is
> > equivalent to using "&&".
> >
> > With "cmd" alone, multi-line command would work as expected (unless
> > $(echo-cmd) is is trying to print the command line).
> >
> > It's "if_changed" macro that doesn't work with multi-line commands.
> > It does:
> > $(cmd); printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
> > With a multiple line command, $(make-cmd) get's expanded to multiple
> > line, so the second argument of "printf" is going to be spread over
> > multiple line in make, and thus multiple shell. We run into this error:
> > /bin/sh: -c: line 0: unexpected EOF while looking for matching `''
> > /bin/sh: -c: line 1: syntax error: unexpected end of file
> >
> > This is why we need to have commands on a single line.
> >
> > I hope the explanation is clear enough.
>
> Yes, thanks. One question remains though: Why do we need multiple
> commands here in the first place, when Linux gets away with one?
Actually, Linux also has multiple commands as well. After running CPP,
it runs ./fixdep (via if_change_dep) which does at least the same thing
as our sed command. We can't use fixdep yet, but I'm working toward it.
> Two other remarks: For one the command's name, aiui, ought to be
> cmd_cpp_lds_S (see Linux). And there ought to be cpp_flags, which
> would then also be used by e.g. cmd_s_S (instead of both having
> $(filter-out -Wa$(comma)%,$(a_flags)) open-coded).
When switching to use CPP instead of CC, I forgot to rename the command,
so I'll fix that.
I'll look at introducing cpp_flags.
Thanks,
--
Anthony PERARD
[XTF 4/4] setup: Setup PV console for HVM guests on xen >4.2
From: Paul Semel
Xen 4.2 requires a workaround that does not setup PV console
for HVM guests. However, newer Xen versions do not have that
limitation and should always have the PV console setup.
In arch_setup() detects Xen version by issuing xen_version hypercall
and optionally passes the version to main_xtf().
Signed-off-by: Paul Semel
Signed-off-by: Pawel Wieczorkiewicz
---
arch/x86/setup.c| 20 ++--
common/setup.c | 6 +-
include/xtf/framework.h | 2 +-
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index f6fa4df..e3f74e6 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -250,8 +250,10 @@ static void xen_console_write(const char *buf, size_t len)
hypercall_console_write(buf, len);
}
-void arch_setup(void)
+void arch_setup(int *version)
{
+int xen_version;
+
if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
{
register_console_callback(qemu_console_write);
@@ -272,9 +274,23 @@ void arch_setup(void)
init_hypercalls();
-if ( !is_initdomain() )
+xen_version = hypercall_xen_version(XENVER_version, NULL);
+if ( version )
+*version = xen_version;
+
+/*
+ * The setup_pv_console function registers a writing function
+ * that makes hvm guests crash on Xen 4.2
+ */
+if ( (!IS_DEFINED(CONFIG_HVM) ||
+ (XEN_MAJOR(xen_version) >= 4 && XEN_MINOR(xen_version) > 2)) &&
+ !is_initdomain() )
{
setup_pv_console();
+}
+
+if ( !is_initdomain() )
+{
setup_xenbus();
}
diff --git a/common/setup.c b/common/setup.c
index 932fc09..1d3da15 100644
--- a/common/setup.c
+++ b/common/setup.c
@@ -19,9 +19,13 @@
*/
void __noreturn xtf_main(void)
{
-arch_setup();
+int xen_version;
+
+arch_setup(&xen_version);
printk("--- Xen Test Framework ---\n");
+printk("Found Xen: %d.%d\n", XEN_MAJOR(xen_version),
+ XEN_MINOR(xen_version));
printk("Environment: %s\n", environment_description);
printk("%s\n", test_title);
diff --git a/include/xtf/framework.h b/include/xtf/framework.h
index a71bf39..6664733 100644
--- a/include/xtf/framework.h
+++ b/include/xtf/framework.h
@@ -2,7 +2,7 @@
#define XTF_FRAMEWORK_H
/* To be implemented by each arch */
-void arch_setup(void);
+void arch_setup(int *);
void test_setup(void);
/* Single line summary of execution environment. */
--
2.16.6
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
[XTF 0/4] Small fixes and improvements
This is the first series of XTF patches I intend to send. It covers some relatively small fixes to handling of PV console by HVM guests, as well as adding serial consol support. Paul Semel (2): Enabled serial writing for hvm guests setup: Setup PV console for HVM guests on xen >4.2 Pawel Wieczorkiewicz (2): lib: Add XEN_MAJOR() and XEN_MINOR() macros lib: always append CR after LF in vsnprintf() arch/x86/setup.c| 34 -- common/libc/vsnprintf.c | 10 ++ common/setup.c | 6 +- include/xtf/framework.h | 2 +- include/xtf/lib.h | 3 +++ tests/xsa-213/main.c| 4 ++-- 6 files changed, 53 insertions(+), 6 deletions(-) -- 2.16.6 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
[XTF 2/4] lib: always append CR after LF in vsnprintf()
The explicit LFCR sequence guarantees proper line by line formatting
in the output.
The '\n' character alone on some terminals is not automatically
converted to LFCR.
Signed-off-by: Pawel Wieczorkiewicz
---
common/libc/vsnprintf.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c
index a49fd30..3202137 100644
--- a/common/libc/vsnprintf.c
+++ b/common/libc/vsnprintf.c
@@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt,
va_list args)
if ( *fmt != '%' )
{
PUT(*fmt);
+
+/*
+ * The '\n' character alone on some terminals is not automatically
+ * converted to LFCR.
+ * The explicit LFCR sequence guarantees proper line by line
+ * formatting in the output.
+ */
+if ( *fmt == '\n' && str < end )
+PUT('\r');
+
continue;
}
--
2.16.6
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
[XTF 1/4] lib: Add XEN_MAJOR() and XEN_MINOR() macros
These are just a simple macros obtaining major, minor values as
returned by xen_version hypercall.
Signed-off-by: Pawel Wieczorkiewicz
---
include/xtf/lib.h| 3 +++
tests/xsa-213/main.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/xtf/lib.h b/include/xtf/lib.h
index 3348464..40e5731 100644
--- a/include/xtf/lib.h
+++ b/include/xtf/lib.h
@@ -20,6 +20,9 @@
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+#define XEN_MAJOR(v) (((v) >> 16) & 0x)
+#define XEN_MINOR(v) ((v) & 0x)
+
void __noreturn panic(const char *fmt, ...) __printf(1, 2);
#define ASSERT(cond)\
diff --git a/tests/xsa-213/main.c b/tests/xsa-213/main.c
index 64e7065..0353168 100644
--- a/tests/xsa-213/main.c
+++ b/tests/xsa-213/main.c
@@ -121,8 +121,8 @@ void test_main(void)
{
long rc, xen_version = hypercall_xen_version(XENVER_version, NULL);
-printk("Found Xen %ld.%ld\n",
- (xen_version >> 16) & 0x, xen_version & 0x);
+printk("Found Xen %ld.%ld\n", XEN_MAJOR(xen_version),
+ XEN_MINOR(xen_version));
xtf_set_idte(X86_VEC_AVAIL, &idte);
--
2.16.6
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
[XTF 3/4] Enabled serial writing for hvm guests
From: Paul Semel
setup.c: PV console writing is not working in Xen 4.2 for hvm
guests, so we make xtf write to COM1 serial port to get its output
Signed-off-by: Paul Semel
Signed-off-by: Pawel Wieczorkiewicz
---
arch/x86/setup.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index 3c84e96..f6fa4df 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -238,6 +238,13 @@ static void qemu_console_write(const char *buf, size_t len)
: "d" (0x12));
}
+static void com1_write(const char *buf, size_t len)
+{
+asm volatile("rep; outsb"
+ : "+S" (buf), "+c" (len)
+ : "d" (0x3f8));
+}
+
static void xen_console_write(const char *buf, size_t len)
{
hypercall_console_write(buf, len);
@@ -246,7 +253,14 @@ static void xen_console_write(const char *buf, size_t len)
void arch_setup(void)
{
if ( IS_DEFINED(CONFIG_HVM) && !pvh_start_info )
+{
register_console_callback(qemu_console_write);
+}
+
+if ( IS_DEFINED(CONFIG_HVM) )
+{
+register_console_callback(com1_write);
+}
register_console_callback(xen_console_write);
--
2.16.6
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Re: [PATCH] sched: print information about scheduler granularity
On 16/04/2020 10:25, Jürgen Groß wrote: > On 16.04.20 11:20, Sergey Dyasli wrote: >> On 16/04/2020 09:57, Jürgen Groß wrote: >>> On 16.04.20 10:33, Sergey Dyasli wrote: Currently it might be not obvious which scheduling mode is being used by the scheduler. Alleviate this by printing additional information about the selected granularity. Messages now look like these: 1. boot (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler (credit) in core-scheduling mode 2. xl debug-keys r (XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-way core-scheduling mode Signed-off-by: Sergey Dyasli >>> >>> Hmm, do we need that? >>> >>> The xen commandline ins part of the boot messages and is contained >>> in the "xl info" output. >> >> It's true that you can see "sched-gran=core" in "xl info" output. But that's >> just the switch - not the end result. A user might want to verify that he did >> everything correctly and core-scheduling mode has indeed been enabled. > > I'm planning to add this information in the pending hypfs (per cpupool). hypfs is certainly nice, but I doubt it'll be available for Xen 4.13. > I'm not opposed to your patch, but as soon as we have per-cpupool > granularity it should be reverted again. "xl debug-keys r" already prints the granularity information per cpupool. It's just opt_sched_granularity is currently a single global variable. Once per-cpupool granularity is implemented, sched_gran_str() should simply gain granularity as a parameter. -- Thanks, Sergey
Re: [PATCH] sched: print information about scheduler granularity
On 16.04.20 11:20, Sergey Dyasli wrote: On 16/04/2020 09:57, Jürgen Groß wrote: On 16.04.20 10:33, Sergey Dyasli wrote: Currently it might be not obvious which scheduling mode is being used by the scheduler. Alleviate this by printing additional information about the selected granularity. Messages now look like these: 1. boot (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler (credit) in core-scheduling mode 2. xl debug-keys r (XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-way core-scheduling mode Signed-off-by: Sergey Dyasli Hmm, do we need that? The xen commandline ins part of the boot messages and is contained in the "xl info" output. It's true that you can see "sched-gran=core" in "xl info" output. But that's just the switch - not the end result. A user might want to verify that he did everything correctly and core-scheduling mode has indeed been enabled. I'm planning to add this information in the pending hypfs (per cpupool). I'm not opposed to your patch, but as soon as we have per-cpupool granularity it should be reverted again. Juergen
Re: [PATCH] sched: print information about scheduler granularity
On 16/04/2020 09:57, Jürgen Groß wrote: > On 16.04.20 10:33, Sergey Dyasli wrote: >> Currently it might be not obvious which scheduling mode is being used >> by the scheduler. Alleviate this by printing additional information >> about the selected granularity. Messages now look like these: >> >> 1. boot >> (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler (credit) in >> core-scheduling mode >> >> 2. xl debug-keys r >> (XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-way >> core-scheduling mode >> >> Signed-off-by: Sergey Dyasli > > Hmm, do we need that? > > The xen commandline ins part of the boot messages and is contained > in the "xl info" output. It's true that you can see "sched-gran=core" in "xl info" output. But that's just the switch - not the end result. A user might want to verify that he did everything correctly and core-scheduling mode has indeed been enabled. -- Thanks, Sergey
Re: [PATCH 0/12] direct-map DomUs
On 15/04/2020 02:02, Stefano Stabellini wrote: Hi all, This series adds support for 1:1 mapping (guest physical == physical) the memory of dom0less domUs. The memory ranges assigned to a domU can be explicitly chosen by the user at boot time. This is desirable in cases where an IOMMU is not present in the system, or it cannot be used. For instance, it might not be usable because it doesn't cover a specific device, or because it doesn't have enough bandwidth, or because it adds too much latency. In these cases, the user should use a MPU to protect the memory in the system (e.g. the Xilinx XMPU), configuring it with the chosen address ranges. Cheers, Stefano The following changes since commit 7372466b21c3b6c96bb7a52754e432bac883a1e3: x86/mem_sharing: Fix build with !CONFIG_XSM (2020-04-10 15:20:10 +0100) are available in the Git repository at: http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git direct-map-1 for you to fetch changes up to 43503720ab6851a28a66fdd067f592d5354ae83a: xen/arm: call iomem_permit_access for passthrough devices (2020-04-14 17:42:21 -0700) Stefano Stabellini (12): xen: introduce xen_dom_flags xen/arm: introduce arch_xen_dom_flags and direct_map xen/arm: introduce 1:1 mapping for domUs xen: split alloc_heap_pages in two halves for reusability xen: introduce reserve_heap_pages xen/arm: reserve 1:1 memory for direct_map domUs xen/arm: new vgic: rename vgic_cpu/dist_base to c/dbase xen/arm: if is_domain_direct_mapped use native addresses for GICv2 xen/arm: if is_domain_direct_mapped use native addresses for GICv3 xen/arm: if is_domain_direct_mapped use native UART address for vPL011 The 3 patches above cover addresses but not interrupts. Why? Cheers, -- Julien Grall
Re: [PATCH] sched: print information about scheduler granularity
On 16.04.20 10:33, Sergey Dyasli wrote: Currently it might be not obvious which scheduling mode is being used by the scheduler. Alleviate this by printing additional information about the selected granularity. Messages now look like these: 1. boot (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler (credit) in core-scheduling mode 2. xl debug-keys r (XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-way core-scheduling mode Signed-off-by: Sergey Dyasli Hmm, do we need that? The xen commandline ins part of the boot messages and is contained in the "xl info" output. Juergen
Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
On 16/04/2020 09:35, Jan Beulich wrote: > On 16.04.2020 10:19, Andrew Cooper wrote: >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -679,7 +679,11 @@ handle_exception_saved: >> 1: movq %rsp,%rdi >> movzbl UREGS_entry_vector(%rsp),%eax >> leaq exception_table(%rip),%rdx >> - PERFC_INCR(exceptions, %rax, %rbx) >> +#ifdef CONFIG_PERF_COUNTERS >> + lea per_cpu__perfcounters(%rip), %rdi >> + add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rdi >> + incl ASM_PERFC_exceptions * 4(%rdi, %rax, 4) >> +#endif >> mov (%rdx, %rax, 8), %rdx >> INDIRECT_CALL %rdx >> mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) >> >> >> If you're happy with that? > I'm afraid I'm not - you can't use %rdi here, it already holds the > called function's argument. I'd be fine with %rcx used instead. Bah - serves me right for being lazy and not retesting. %rcx it is. ~Andrew
Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
On 16.04.2020 10:19, Andrew Cooper wrote: > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -679,7 +679,11 @@ handle_exception_saved: > 1: movq %rsp,%rdi > movzbl UREGS_entry_vector(%rsp),%eax > leaq exception_table(%rip),%rdx > - PERFC_INCR(exceptions, %rax, %rbx) > +#ifdef CONFIG_PERF_COUNTERS > + lea per_cpu__perfcounters(%rip), %rdi > + add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rdi > + incl ASM_PERFC_exceptions * 4(%rdi, %rax, 4) > +#endif > mov (%rdx, %rax, 8), %rdx > INDIRECT_CALL %rdx > mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) > > > If you're happy with that? I'm afraid I'm not - you can't use %rdi here, it already holds the called function's argument. I'd be fine with %rcx used instead. Jan
Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
Hi Andrew, On 15/04/2020 18:39, Andrew Cooper wrote: The PERFC_INCR() macro uses current->processor, but current is not valid during early boot. This causes the following crash to occur if e.g. rdmsr_safe() has to recover from a #GP fault. (XEN) Early fatal page fault at e008:82d0803b1a39 (cr2=0004, ec=) (XEN) [ Xen-4.14-unstable x86_64 debug=y Not tainted ] (XEN) CPU:0 (XEN) RIP:e008:[] x86_64/entry.S#handle_exception_saved+0x64/0xb8 ... (XEN) Xen call trace: (XEN)[] R x86_64/entry.S#handle_exception_saved+0x64/0xb8 (XEN)[] F __start_xen+0x2cd/0x2980 (XEN)[] F __high_start+0x4c/0x4e Furthermore, the PERFC_INCR() macro is wildly inefficient. There has been a single caller for many releases now, so inline it and delete the macro completely. For the assembly, move entry_vector from %eax into %ecx. There is no encoding benefit for movzbl, and it frees up %eax to be used inside the CONFIG_PERF_COUNTERS block where there is an encoding benefit. There is no need to reference current at all. What is actually needed is the per_cpu_offset which can be obtained directly from the top-of-stack block. This simplifies the counter handling to 3 instructions and no spilling to the stack at all. The same breakage from above is now handled properly: (XEN) traps.c:1591: GPF (): 82d0806394fe [__start_xen+0x2cd/0x2980] -> 82d0803b3bfb Reported-by: Julien Grall Signed-off-by: Andrew Cooper I can confirm the crash I have seen has now disappeared. Tested-by: Julien Grall Cheers, -- Julien Grall
[PATCH] sched: print information about scheduler granularity
Currently it might be not obvious which scheduling mode is being used
by the scheduler. Alleviate this by printing additional information
about the selected granularity. Messages now look like these:
1. boot
(XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler (credit) in
core-scheduling mode
2. xl debug-keys r
(XEN) [ 45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-way
core-scheduling mode
Signed-off-by: Sergey Dyasli
---
CC: Juergen Gross
CC: Dario Faggioli
CC: George Dunlap
CC: Jan Beulich
---
xen/common/sched/core.c| 10 --
xen/common/sched/cpupool.c | 30 +-
xen/common/sched/private.h | 2 ++
3 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index d4a6489929..b1b09a159b 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2883,6 +2883,7 @@ void scheduler_enable(void)
void __init scheduler_init(void)
{
struct domain *idle_domain;
+char sched_gran[20];
int i;
scheduler_enable();
@@ -2937,7 +2938,9 @@ void __init scheduler_init(void)
BUG();
register_cpu_notifier(&cpu_schedule_nfb);
-printk("Using scheduler: %s (%s)\n", ops.name, ops.opt_name);
+printk("Using scheduler: %s (%s) in %s-scheduling mode\n",
+ ops.name, ops.opt_name,
+ sched_gran_str(sched_gran, sizeof(sched_gran)));
if ( sched_init(&ops) )
panic("scheduler returned error on init\n");
@@ -3267,6 +3270,7 @@ void schedule_dump(struct cpupool *c)
unsigned int i, j;
struct scheduler *sched;
cpumask_t*cpus;
+char sched_gran[20];
/* Locking, if necessary, must be handled withing each scheduler */
@@ -3276,7 +3280,9 @@ void schedule_dump(struct cpupool *c)
{
sched = c->sched;
cpus = c->res_valid;
-printk("Scheduler: %s (%s)\n", sched->name, sched->opt_name);
+printk("Scheduler: %s (%s) in %s-scheduling mode\n",
+ sched->name, sched->opt_name,
+ sched_gran_str(sched_gran, sizeof(sched_gran)));
sched_dump_settings(sched);
}
else
diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index d40345b585..a37b97f4c2 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -38,7 +38,35 @@ static cpumask_t cpupool_locked_cpus;
static DEFINE_SPINLOCK(cpupool_lock);
static enum sched_gran __read_mostly opt_sched_granularity = SCHED_GRAN_cpu;
-static unsigned int __read_mostly sched_granularity = 1;
+static unsigned int __read_mostly sched_granularity;
+
+char *sched_gran_str(char *str, size_t size)
+{
+char *mode = "";
+
+switch ( opt_sched_granularity )
+{
+case SCHED_GRAN_cpu:
+mode = "cpu";
+break;
+case SCHED_GRAN_core:
+mode = "core";
+break;
+case SCHED_GRAN_socket:
+mode = "socket";
+break;
+default:
+ASSERT_UNREACHABLE();
+break;
+}
+
+if ( sched_granularity )
+snprintf(str, size, "%u-way %s", sched_granularity, mode);
+else
+snprintf(str, size, "%s", mode);
+
+return str;
+}
#ifdef CONFIG_HAS_SCHED_GRANULARITY
static int __init sched_select_granularity(const char *str)
diff --git a/xen/common/sched/private.h b/xen/common/sched/private.h
index 367811a12f..fd49f545cb 100644
--- a/xen/common/sched/private.h
+++ b/xen/common/sched/private.h
@@ -30,6 +30,8 @@ enum sched_gran {
SCHED_GRAN_socket
};
+char *sched_gran_str(char *str, size_t size);
+
/*
* In order to allow a scheduler to remap the lock->cpu mapping,
* we have a per-cpu pointer, along with a pre-allocated set of
--
2.17.1
Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
On 16/04/2020 08:19, Jan Beulich wrote: > On 15.04.2020 19:39, Andrew Cooper wrote: >> The PERFC_INCR() macro uses current->processor, but current is not valid >> during early boot. This causes the following crash to occur if >> e.g. rdmsr_safe() has to recover from a #GP fault. >> >> (XEN) Early fatal page fault at e008:82d0803b1a39 >> (cr2=0004, ec=) >> (XEN) [ Xen-4.14-unstable x86_64 debug=y Not tainted ] >> (XEN) CPU:0 >> (XEN) RIP:e008:[] >> x86_64/entry.S#handle_exception_saved+0x64/0xb8 >> ... >> (XEN) Xen call trace: >> (XEN)[] R >> x86_64/entry.S#handle_exception_saved+0x64/0xb8 >> (XEN)[] F __start_xen+0x2cd/0x2980 >> (XEN)[] F __high_start+0x4c/0x4e >> >> Furthermore, the PERFC_INCR() macro is wildly inefficient. There has been a >> single caller for many releases now, so inline it and delete the macro >> completely. >> >> For the assembly, move entry_vector from %eax into %ecx. There is no >> encoding >> benefit for movzbl, and it frees up %eax to be used inside the >> CONFIG_PERF_COUNTERS block where there is an encoding benefit. > I don't mind the change in register use, but I'd certainly like to > understand the supposed "encoding benefit". Afaics ... > >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -677,10 +677,14 @@ handle_exception_saved: >> #endif /* CONFIG_PV */ >> sti >> 1: movq %rsp,%rdi >> -movzbl UREGS_entry_vector(%rsp),%eax >> +movzbl UREGS_entry_vector(%rsp), %ecx >> leaq exception_table(%rip),%rdx >> -PERFC_INCR(exceptions, %rax, %rbx) >> -mov (%rdx, %rax, 8), %rdx >> +#ifdef CONFIG_PERF_COUNTERS >> +lea per_cpu__perfcounters(%rip), %rax >> +add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rax >> +incl ASM_PERFC_exceptions * 4(%rax, %rcx, 4) >> +#endif > ... all insns inside the block use a ModR/M byte, and there's also > no special SIB encoding form used (i.e. one where the disp size > would increase because of register choice). Hmm - I suppose it is stale, written for an older version of the logic before I spotted a further optimisation. > With this clarified, i.e. possibly the commit message updated > accordingly, and possibly even code churn reduced by undoing the > change of register used, Leaving %eax as was, and using %rdi for the single temporary looks like: diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 997c481ecb..1984bb7948 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -679,7 +679,11 @@ handle_exception_saved: 1: movq %rsp,%rdi movzbl UREGS_entry_vector(%rsp),%eax leaq exception_table(%rip),%rdx - PERFC_INCR(exceptions, %rax, %rbx) +#ifdef CONFIG_PERF_COUNTERS + lea per_cpu__perfcounters(%rip), %rdi + add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rdi + incl ASM_PERFC_exceptions * 4(%rdi, %rax, 4) +#endif mov (%rdx, %rax, 8), %rdx INDIRECT_CALL %rdx mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) If you're happy with that? ~Andrew
Re: [XEN PATCH v4 12/18] xen/build: factorise generation of the linker scripts
On 15.04.2020 18:58, Anthony PERARD wrote:
> On Wed, Apr 08, 2020 at 02:46:42PM +0200, Jan Beulich wrote:
>> On 31.03.2020 12:30, Anthony PERARD wrote:
>>> - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line is
>>> still mandatory for if_changed (or cmd) macro to work.
>>
>> I still don't believe in there being a need for "; \" there. This
>> actually breaks things, after all:
>>
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -236,6 +236,12 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags))
>>> $< -o $@
>>> %.s: %.S FORCE
>>> $(call if_changed,cpp_s_S)
>>>
>>> +# Linker scripts, .lds.S -> .lds
>>> +quiet_cmd_cc_lds_S = LDS $@
>>> +cmd_cc_lds_S = $(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \
>>> +sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new;
>>> \
>>> +mv -f $(dot-target).d.new $(dot-target).d
>>
>> if $(CPP) or sed fail, previously the whole rule would have failed,
>> which no longer is the case with your use of semicolons. There
>> ought to be a solution to this, ideally one better than adding
>> "set -e" as the first command ("define" would at least deal with
>> the multi-line make issue, but without it being clear to me why the
>> semicolons would be needed I don't think I can suggest anything
>> there at the moment).
>
> The only macro that will consumes cmd_cc_lds_S (and other cmd_*) is
> "cmd", it is defined as:
> cmd = @set -e; $(echo-cmd) $(cmd_$(1))
> So, "set -e" is already there, and using semicolons in commands is
> equivalent to using "&&".
>
> With "cmd" alone, multi-line command would work as expected (unless
> $(echo-cmd) is is trying to print the command line).
>
> It's "if_changed" macro that doesn't work with multi-line commands.
> It does:
> $(cmd); printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
> With a multiple line command, $(make-cmd) get's expanded to multiple
> line, so the second argument of "printf" is going to be spread over
> multiple line in make, and thus multiple shell. We run into this error:
> /bin/sh: -c: line 0: unexpected EOF while looking for matching `''
> /bin/sh: -c: line 1: syntax error: unexpected end of file
>
> This is why we need to have commands on a single line.
>
> I hope the explanation is clear enough.
Yes, thanks. One question remains though: Why do we need multiple
commands here in the first place, when Linux gets away with one?
Two other remarks: For one the command's name, aiui, ought to be
cmd_cpp_lds_S (see Linux). And there ought to be cpp_flags, which
would then also be used by e.g. cmd_s_S (instead of both having
$(filter-out -Wa$(comma)%,$(a_flags)) open-coded).
Jan
Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
On 15.04.2020 19:39, Andrew Cooper wrote: > The PERFC_INCR() macro uses current->processor, but current is not valid > during early boot. This causes the following crash to occur if > e.g. rdmsr_safe() has to recover from a #GP fault. > > (XEN) Early fatal page fault at e008:82d0803b1a39 > (cr2=0004, ec=) > (XEN) [ Xen-4.14-unstable x86_64 debug=y Not tainted ] > (XEN) CPU:0 > (XEN) RIP:e008:[] > x86_64/entry.S#handle_exception_saved+0x64/0xb8 > ... > (XEN) Xen call trace: > (XEN)[] R > x86_64/entry.S#handle_exception_saved+0x64/0xb8 > (XEN)[] F __start_xen+0x2cd/0x2980 > (XEN)[] F __high_start+0x4c/0x4e > > Furthermore, the PERFC_INCR() macro is wildly inefficient. There has been a > single caller for many releases now, so inline it and delete the macro > completely. > > For the assembly, move entry_vector from %eax into %ecx. There is no encoding > benefit for movzbl, and it frees up %eax to be used inside the > CONFIG_PERF_COUNTERS block where there is an encoding benefit. I don't mind the change in register use, but I'd certainly like to understand the supposed "encoding benefit". Afaics ... > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -677,10 +677,14 @@ handle_exception_saved: > #endif /* CONFIG_PV */ > sti > 1: movq %rsp,%rdi > -movzbl UREGS_entry_vector(%rsp),%eax > +movzbl UREGS_entry_vector(%rsp), %ecx > leaq exception_table(%rip),%rdx > -PERFC_INCR(exceptions, %rax, %rbx) > -mov (%rdx, %rax, 8), %rdx > +#ifdef CONFIG_PERF_COUNTERS > +lea per_cpu__perfcounters(%rip), %rax > +add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rax > +incl ASM_PERFC_exceptions * 4(%rax, %rcx, 4) > +#endif ... all insns inside the block use a ModR/M byte, and there's also no special SIB encoding form used (i.e. one where the disp size would increase because of register choice). With this clarified, i.e. possibly the commit message updated accordingly, and possibly even code churn reduced by undoing the change of register used, Reviewed-by: Jan Beulich Jan
Re: [PATCH v4 4/4] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
On 15.04.2020 20:37, Hongyan Xia wrote: > From: Wei Liu > > Signed-off-by: Wei Liu > Signed-off-by: Hongyan Xia Reviewed-by: Jan Beulich
Re: [PATCH v4 1/4] x86/shim: map and unmap page tables in replace_va_mapping
On 15.04.2020 20:37, Hongyan Xia wrote: > From: Wei Liu > > Also, introduce lYe_from_lXe() macros which do not rely on the direct > map when walking page tables. Unfortunately, they cannot be inline > functions due to the header dependency on domain_page.h, so keep them as > macros just like map_lYt_from_lXe(). > > Signed-off-by: Wei Liu > Signed-off-by: Hongyan Xia Reviewed-by: Jan Beulich
