Re: [Xen-devel] Criteria / validation proposal: drop Xen
Hi all, I am going to step in here with my hat as Xen Project community manager. We had a discussion about this issue as part of last week's community call. I CC'ed a number of stake-holders, which probably should have been on the thread such as ITL (which builds QubesOS on top of Fedora) and Michael A Young (the Xen package manager). First of all apologies that this issue has lingered so long. Key members of the community were not aware of the issues raised in this thread, otherwise we would have acted earlier. With this in mind, please in future raise issues with me, on xen-devel@, committers@ or the Xen-Fedora package manager. The Xen Community would like to see Fedora running as guest: in fact it would be somewhat odd if there was a Xen-Dom0 package and guest support didn't work. And there are some downstreams such as QubesOS, which depend on this support. > On 6 Jul 2017, at 13:45, Adam Williamson wrote: > > On Thu, 2017-07-06 at 15:13 -0400, Konrad Rzeszutek Wilk wrote: >> On Thu, Jul 06, 2017 at 11:59:01AM -0700, Adam Williamson wrote: >>> Hi, folks! A while ago, Xen virtualization functionality was added to >>> the criteria and the validation test case set, on the understanding >>> that Oracle would provide testing for it (and help fix bugs as they >>> arose). >>> >>> For the last couple of releases we really have not had any such testing >> >> We had been doing the testing, it just that we (or rather me and >> Dariof) seem to get a wind of this at the last minute. Not sure exactly >> how to fix that thought. > > Well, I mean, every few *days* a compose gets nominated for validation > testing, and a mail is sent to test-announce. Just check your test- > announce archives for mails with "nominated for testing" in their > subject lines, and you'll see dozens. Is this not sufficient > notification? We discussed this at the community call and came to the conclusion that we can run regular tests of Fedora RC's as part of our OSSTEST infrastructure. Ian Jackson volunteered to implement this, but there are some questions on a) The installer (which we can handle ourselves) b) When we would trigger a test - aka is there some trigger other than the c) How would results best be reported back to Fedora Apologies, I am not very familiar with how the Fedora Test group works. Is there some documentation which would help integrate what you to with the test system of another open source project? >>> from Oracle. On that basis, I'm proposing we remove this Final >>> criterion: >> >> s/Oracle/Xen Project/ I believe? > > Perhaps, it's just that it always seemed to be you doing the testing, > so they got a bit conflated :) Can we come to some arrangement, by which such issues get communicated to the Xen Project earlier? Aka me, xen-devel@ or committers@ >>> "The release must boot successfully as Xen DomU with releases providing >>> a functional, supported Xen Dom0 and widely used cloud providers >>> utilizing Xen." >>> >>> and change the 'milestone' for the test case - >>> https://fedoraproject.org/wiki/QA:Testcase_Boot_Methods_Xen_Para_Virt - >>> from Final to Optional. >>> >>> Thoughts? Comments? Thanks! >> >> I would prefer for it to remain as it is. > > This is only practical if it's going to be tested, and tested regularly > - not *only* on the final release candidate, right before we sign off > on the release. It needs to be tested regularly throughout the release > cycle, on the composes that are "nominated for testing". Would the proposal above work for you? I think it satisfies what you are looking for. We would also have someone who monitors these test results pro-actively. Then, there are the specific grub issues that need resolving [A1] https://bugzilla.redhat.com/show_bug.cgi?id=1486002 (and a recently filed duplicate @ https://bugzilla.redhat.com/show_bug.cgi?id=1691559) caused by [A2]) [A2] https://bugzilla.redhat.com/show_bug.cgi?id=1703700 [B1] https://bugzilla.redhat.com/show_bug.cgi?id=1264103 The first makes it harder to boot Fedora _dom0_ (but workarounds exist). While it is unpleasant, it doesn't break the release criterion, but probably has deterred people from testing. The second one [B1] is about Fedora _domU_, which breaks the release criterion. Marek and Michael had a discussion about these and there was also a summary from Daniel. == On [A1]/[A2] == Lack of GRUB2 multiboot2/module2 commands in Fedora/RH which does not allow you load Xen as dom0 on EFI platforms with or without secure boot. Here are some relevant snippets from the discussions: "In general both modules were dropped due to CVE-2015-5281 (grub2: modules built in on EFI builds that allow loading arbitrary code, circumventing secure boot) [A3][A4]. Of course this makes sense because we do not want to break UEFI secure boot. But this means that you cannot boot Xen dom0 on UEFI platforms. The Multiboot2 protocol support is required to do that. Potentially you can use xen.efi directly but
[Xen-devel] [qemu-upstream-4.11-testing test] 136057: regressions - FAIL
flight 136057 qemu-upstream-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/136057/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken in 134594 build-arm64 broken in 134594 build-arm64-xsm broken in 134594 build-arm64-xsm4 host-install(4) broken in 134594 REGR. vs. 125575 build-arm64-pvops 4 host-install(4) broken in 134594 REGR. vs. 125575 build-arm644 host-install(4) broken in 134594 REGR. vs. 125575 test-arm64-arm64-libvirt-xsm 7 xen-boot fail REGR. vs. 125575 test-arm64-arm64-xl-credit2 7 xen-boot fail REGR. vs. 125575 test-arm64-arm64-xl-xsm 7 xen-boot fail REGR. vs. 125575 test-arm64-arm64-xl 7 xen-boot fail REGR. vs. 125575 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qcow217 guest-localmigrate/x10 fail pass in 134594 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit1 1 build-check(1) blocked in 134594 n/a test-arm64-arm64-xl 1 build-check(1) blocked in 134594 n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked in 134594 n/a build-arm64-libvirt 1 build-check(1) blocked in 134594 n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked in 134594 n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked in 134594 n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-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-arm64-arm64-xl-credit1 7 xen-boot fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-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 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 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-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-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuu2871355a6957f1b3c16f858e3143e0fff0737b6a baseline version: qemuu20c76f9a5fbf16d58c6add2ace2ff0fabd785926 Last test of basis 125575 2018-07-25 18:53:54 Z 292 days Testing same since
Re: [Xen-devel] Anyone using blktap2?
On Mon, May 13, 2019 at 04:34:14PM +0100, Wei Liu wrote: > Hello > > Seeing that you were the last people who changed blktap2 in a meaningful > way: do you use it at all? > > I'm thinking about dropping it (again). Fine with me too. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 136179: tolerable all pass - PUSHED
flight 136179 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/136179/ 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 99bb45e684283b3bc621dbc99b1b93c856b4dd1c baseline version: xen 004299fba49747ccd614c9abcae9772ec8845e20 Last test of basis 136178 2019-05-13 13:00:48 Z0 days Testing same since 136179 2019-05-13 16:02:31 Z0 days1 attempts People who touched revisions under test: Andrew Cooper George Dunlap Jan Beulich Olaf Hering Paul Durrant Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-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 004299fba4..99bb45e684 99bb45e684283b3bc621dbc99b1b93c856b4dd1c -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-upstream-4.10-testing test] 136051: FAIL
flight 136051 qemu-upstream-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/136051/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken in 134580 build-arm64-pvopsbroken in 134580 build-arm64 broken in 134580 build-arm64-xsm4 host-install(4) broken in 134580 REGR. vs. 124921 build-arm64-pvops 4 host-install(4) broken in 134580 REGR. vs. 124921 build-arm644 host-install(4) broken in 134580 REGR. vs. 124921 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qcow217 guest-localmigrate/x10 fail pass in 134580 test-amd64-amd64-xl-xsm 7 xen-boot fail pass in 135948 test-amd64-i386-xl-qemuu-ws16-amd64 15 guest-saverestore.2 fail pass in 135948 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked in 134580 n/a build-arm64-libvirt 1 build-check(1) blocked in 134580 n/a test-arm64-arm64-xl 1 build-check(1) blocked in 134580 n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked in 134580 n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked in 134580 n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked in 134580 n/a test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail in 135948 never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail like 124921 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 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-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-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never
[Xen-devel] [ovmf test] 136056: all pass - PUSHED
flight 136056 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/136056/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf f684c3f5eef4be691e137ae64e7d00521ec201de baseline version: ovmf cd5147734cbe82f0c1665eb232608d75643944b0 Last test of basis 135978 2019-05-10 04:21:26 Z3 days Testing same since 136056 2019-05-12 02:12:34 Z1 days1 attempts People who touched revisions under test: "Tien Hock, Loh" Bob Feng Fan, ZhijuX Guo Dong Liu, Zhiguang Maurice Ma Tien Hock, Loh Zhiguang Liu Zhiju.Fan 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 cd5147734c..f684c3f5ee f684c3f5eef4be691e137ae64e7d00521ec201de -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [VMI] Possible race-condition in altp2m APIs
On 5/13/19 7:18 PM, Mathieu Tarral wrote: > Le vendredi, mai 10, 2019 5:21 PM, Andrew Cooper > a écrit : > >> On 10/05/2019 16:17, Mathieu Tarral wrote: >> >>> Le jeudi, mai 9, 2019 6:42 PM, Andrew Cooper andrew.coop...@citrix.com a >>> écrit : >>> Therefore, the conclusion to draw is that it is a logical bug somewhere. >>> The bug is still here, so we can exclude a microcode issue. >> >> Good - that is one further angle excluded. Always make sure you are >> running with up-to-date microcode, but it looks like we back to >> investigating a logical bug in libvmi or Xen. > > I played with tool/tests/xen-access this afternoon. > > The tool is working, i could intercept breakpoints, cpuid, write and exec mem > accesses, etc.. > > However, using altp2m related intercepts leads to a guest crash sometimes: > > Windows 7 x64, 4 VCPUs > - altp2m_write: crash > - altp2m_exec: crash > - altp2m_write_no_gpt: frozen > > Windows 7 x64, 1 VCPU > - altp2m_write: crash > - altp2m_exec: OK > - altp2m_write_no_gpt: frozen > > "frozen" means that xen-access receives VMI events, bug the guest is frozen > until I decide to stop xen-access. > I'm wondering what kind of exec events it received because they are not the > same, so it's not looping > over the same RIP over and over. (?) I think you're simply tripping some OS timer because you're slowing the guest down in the crash case, and simply keep the guest too busy handling events in the "freeze" case. Remember that there's quite a delay running each offending instruction: one vm_event saying you've got a violation, a reply saying "put this VCPU in single-step mode _and_ switch to the unrestricted EPT view", another vm_event saying "instruction executed", followed by anoher reply saying "switch back to the restricted EPT _and_ take the VCPU out of single-step mode". Restricting the whole of the guest's memory (and so doing this dance for _every_ instruction causing a fault) is practically guaranteed to upset the OS. A little EPT restricting goes a long way. Of course, if this could be improved so that even stress-tests (which is basically what xen-access is) leave the guest running smoothly, that'd be fantastic. Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] docs/xl: Clarify documentation for mem-max and mem-set
On 13/05/2019, 08:28, "Wei Liu" wrote: On Mon, May 13, 2019 at 02:59:30PM +0100, Wei Liu wrote: > On Wed, May 01, 2019 at 03:59:41PM +0100, George Dunlap wrote: > > On 4/8/19 12:09 PM, George Dunlap wrote: > > > mem-set is the primary command that users will need to use and > > > understand. Move it first, and clarify the wording; also specify that > > > you can't set the target higher than maxmem from the domain config. > > > > > > mem-max is actually a pretty useless command at the moment. Clarify > > > that users are not expected to use it; and document all of its quirky > > > behavior. > > > > > > Signed-off-by: George Dunlap > > > > Wei / Ian: Ping? > > > > Juergen replied to my "review note" comment, not to anything actionable > > in the patch (or commit message) itself. > > Acked-by: Wei Liu > > (I already said this looked okay to me in v1) I believe Lars' Rb from v1 still stands. Yes Lars ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] x86/mm: subsume set_gpfn_from_mfn() into guest_physmap_add_page()
On 5/9/19 2:45 PM, Jan Beulich wrote: > The two callers in common/memory.c currently call set_gpfn_from_mfn() > themselves, so moving the call into guest_physmap_add_page() helps > tidy their code. > > The two callers in common/grant_table.c fail to make that call alongside > the one to guest_physmap_add_page(), so will actually get fixed by the > change. > > Other (x86) callers are HVM only and are hence unaffected by a change > to the function's !paging_mode_translate() part. > > Sadly this isn't enough yet to drop Arm's dummy macro, as there's one > more use in page_alloc.c. > > Signed-off-by: Jan Beulich > Acked-by: Julien Grall > --- > v2: Re-base over added earlier patch. Re-write description. Thanks: Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/mm: make guest_physmap_add_entry() HVM-only
On 5/9/19 2:44 PM, Jan Beulich wrote: > Lift its !paging_mode_translate() part into guest_physmap_add_page() > (which is what common code calls), eliminating the dummy use of a > (HVM-only really) P2M type in the PV case. > > Suggested-by: George Dunlap > Signed-off-by: Jan Beulich Thanks, looks good: Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [VMI] Possible race-condition in altp2m APIs
Le vendredi, mai 10, 2019 5:21 PM, Andrew Cooper a écrit : > On 10/05/2019 16:17, Mathieu Tarral wrote: > > > Le jeudi, mai 9, 2019 6:42 PM, Andrew Cooper andrew.coop...@citrix.com a > > écrit : > > > > > Therefore, the conclusion to draw is that it is a logical bug somewhere. > > The bug is still here, so we can exclude a microcode issue. > > Good - that is one further angle excluded. Always make sure you are > running with up-to-date microcode, but it looks like we back to > investigating a logical bug in libvmi or Xen. I played with tool/tests/xen-access this afternoon. The tool is working, i could intercept breakpoints, cpuid, write and exec mem accesses, etc.. However, using altp2m related intercepts leads to a guest crash sometimes: Windows 7 x64, 4 VCPUs - altp2m_write: crash - altp2m_exec: crash - altp2m_write_no_gpt: frozen Windows 7 x64, 1 VCPU - altp2m_write: crash - altp2m_exec: OK - altp2m_write_no_gpt: frozen "frozen" means that xen-access receives VMI events, bug the guest is frozen until I decide to stop xen-access. I'm wondering what kind of exec events it received because they are not the same, so it's not looping over the same RIP over and over. (?) Here is an example output I have when I run sudo ./xen-access altp2m_write ... Got event from Xen Singlestep: rip=f800026e60dc, vcpu 1, altp2m 0 Switching altp2m to view 1! Error -1 getting mem_access event Singlestep: rip=f800026e6054, vcpu 3, altp2m 0 Switching altp2m to view 1! Singlestep: rip=f800026d64c5, vcpu 0, altp2m 0 Switching altp2m to view 1! xenaccess shutting down on signal -1 Got event from Xen PAGE ACCESS: rw- for GFN 21cef (offset 000fb8) gla f88002039fb8 (valid: y; fault in gpt: n; fault with gla: y) (vcpu 1 [p], altp2m view 1) Switching back to default view! PAGE ACCESS: rw- for GFN 1debc (offset 0004b0) gla f880022ed4b0 (valid: y; fault in gpt: n; fault with gla: y) (vcpu 3 [p], altp2m view 1) Switching back to default view! PAGE ACCESS: rw- for GFN b9a (offset 000ae8) gla f8b9aae8 (valid: y; fault in gpt: n; fault with gla: y) (vcpu 0 [p], altp2m view 1) Switching back to default view! xenaccess shut down on signal -1 xenaccess exit code -1 @Tamas: if you added support for altp2m in xen-access, did you remember crashing your guest ? Or was it working at the time you tested ? Mathieu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/mm: short-circuit HVM-only mode flags when !HVM
On 5/9/19 2:42 PM, Jan Beulich wrote: > #define-ing them to zero allows better code generation in this case, > and paves the way for more DCE, allowing to leave certain functions just > declared, but not defined. > > Signed-off-by: Jan Beulich Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Anyone using blktap2?
Am Mon, 13 May 2019 16:34:14 +0100 schrieb Wei Liu : > Seeing that you were the last people who changed blktap2 in a meaningful > way: do you use it at all? The default for --enable-blktap2 in tools/configure.ac is off, and nothing seems to pass --enable-blktap2 to configure. So I'm ok to remove the code. Olaf pgprksBzG1B9W.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/5] iommu: move iommu_get_ops() into common code
>>> On 08.05.19 at 15:24, wrote:
> Currently x86 and ARM differ in their implementation for no good reason.
> This patch moves the ARM variant of iommu_get/set_ops() helpers into
> common code and modifies them so they deal with the __initconstrel
> ops structures used by the x86 IOMMU vendor implementations (adding
> __initconstrel to the SMMU code to bring it in line). Consequently, a lack
> of init() method is now taken to mean uninitialized iommu_ops. Also, the
> printk warning in iommu_set_ops() now becomes an ASSERT.
When having submitted the indirect call overhead reduction series
including IOMMU changes for the first time, I was told that the Arm
folks would like to retain the ability to eventually support
heterogeneous IOMMUs (and hence I shouldn't provide patching
infrastructure there). A single global iommu_[gs]et_ops() is sort of
getting in the way of this as well, I think, and hence I'm not sure it
is a desirable step to make this so far Arm-specific arrangement
the general model. At least it would further complicate Arm side
changes towards that (mid / long term?) goal.
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -21,6 +21,21 @@
> #include
> #include
>
> +static struct iommu_ops __read_mostly iommu_ops;
> +
> +const struct iommu_ops *iommu_get_ops(void)
> +{
> +return _ops;
> +}
> +
> +void __init iommu_set_ops(const struct iommu_ops *ops)
> +{
> +BUG_ON(!ops);
> +
> +ASSERT(!iommu_ops.init || iommu_ops.init == ops->init);
> +iommu_ops = *ops;
> +}
I realize that you merely move (and slightly re-arrange) what has
been there, but now that I look at it again I think ops->init should
also be verified to be non-NULL, or else installing such a set of
hooks would effectively revert back to the "no hooks yet" state.
> @@ -33,11 +32,7 @@ int __init iommu_hardware_setup(void)
> if ( !iommu_init_ops )
> return -ENODEV;
>
> -if ( !iommu_ops.init )
> -iommu_ops = *iommu_init_ops->ops;
> -else
> -/* x2apic setup may have previously initialised the struct. */
> -ASSERT(iommu_ops.init == iommu_init_ops->ops->init);
> +iommu_set_ops(iommu_init_ops->ops);
I was specifically asked to add the comment that you get rid of.
While mentioning x2APIC in common code may no be appropriate,
I'm sure this could be worded in a more general way and attached
to the moved check.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 13.05.19 18:45, Julien Grall wrote: I was speaking about dd process. But Dom0 vCPUs might also be a good idea. I see. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hi. On 5/13/19 4:42 PM, Andrii Anisov wrote: On 13.05.19 18:40, Julien Grall wrote: From my understanding, if you want consistency, then setting the affinity will definitely help. Otherwise, you may have the scheduler to kick up and also balancing. Sorry, do you mean setting affinity for dd processes, or Dom0 VCPUs, or both? I was speaking about dd process. But Dom0 vCPUs might also be a good idea. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Anyone using blktap2?
On 13/05/2019 17:34, Wei Liu wrote: > Hello > > Seeing that you were the last people who changed blktap2 in a meaningful > way: do you use it at all? Not me. I was only changing it to comply with the rest of the build (adding pkg-config file). I SUSE builds (SLE, openSUSE) it is not configured. > I'm thinking about dropping it (again). +1 Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 13.05.19 18:40, Julien Grall wrote: From my understanding, if you want consistency, then setting the affinity will definitely help. Otherwise, you may have the scheduler to kick up and also balancing. Sorry, do you mean setting affinity for dd processes, or Dom0 VCPUs, or both? -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 5/13/19 4:38 PM, Andrii Anisov wrote: On 13.05.19 18:31, Julien Grall wrote: So, are you running 4 dd (one for each core) in parallel? Are they pinned? Yes, sure I run 4 dd's in parallel to get all VCPUs loaded. No they are not pinned. From my understanding, if you want consistency, then setting the affinity will definitely help. Otherwise, you may have the scheduler to kick up and also balancing. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] xen/watchdog: Drop all locked operations on the watchdog_inuse_map
>>> On 13.05.19 at 17:17, wrote:
> On 13/05/2019 16:01, Jan Beulich wrote:
> On 10.05.19 at 20:28, wrote:
>>> All modifications to the watchdog_inuse_map happen with d->watchdog_lock
>>> held,
>>> so there are no concurrency problems to deal with.
>> But concurrency problems can also occur for readers. While
>> not a problem afaict, dump_domains() actually has such an
>> example (and hence might be worth mentioning here).
>
> Its only debugging, and would need to take the spinlock anyway to avoid
> a TOCTOU race between watchdog_inuse_map and d->watchdog_timer[i].expires
Right, hence my suggestion to just mention it here.
>>> --- a/xen/common/schedule.c
>>> +++ b/xen/common/schedule.c
>>> @@ -1068,17 +1068,15 @@ static long domain_watchdog(struct domain *d,
>>> uint32_t id, uint32_t timeout)
>>> }
>>> else /* Allocate the next available timer. */
>>> {
>>> -for ( id = 0; id < NR_DOMAIN_WATCHDOG_TIMERS; id++ )
>>> -{
>>> -if ( test_and_set_bit(id, >watchdog_inuse_map) )
>>> -continue;
>>> -break;
>>> -}
>>> -if ( id == NR_DOMAIN_WATCHDOG_TIMERS )
>>> +id = ffs(~d->watchdog_inuse_map) - 1;
>> I'm surprised we have no (universally available) ffz(). I wonder
>> though whether find_first_zero_bit() wouldn't be the better
>> choice here anyway, as the result wouldn't be undefined in
>> case NR_DOMAIN_WATCHDOG_TIMERS grew to 32.
>
> Sadly no - find_first_zero_bit() takes unsigned long *, so its use here
> a) requires a (void *) cast to compile, and b) is definitely UB.
Oh, right, it works with a pointer. Would you mind adding a
BUILD_BUG_ON() then to exclude the UB case of ffs()? With
that then
Reviewed-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 13.05.19 18:31, Julien Grall wrote: So, are you running 4 dd (one for each core) in parallel? Are they pinned? Yes, sure I run 4 dd's in parallel to get all VCPUs loaded. No they are not pinned. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/5] iommu / x86: move call to scan_pci_devices() out of vendor code
>>> On 08.05.19 at 15:24, wrote: > It's not vendor specific so it shouldn't really be there. Perhaps, but this needs better justification: > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -2372,10 +2372,6 @@ static int __init vtd_setup(void) > P(iommu_hap_pt_share, "Shared EPT tables"); > #undef P > > -ret = scan_pci_devices(); > -if ( ret ) > -goto error; > - > ret = init_vtd_hw(); Even after some looking around, it's not obvious to me that the latter call doesn't depend on PCI devices being known, more specifically segment 0's bus2bridge[] having been filled. Nor can I tell whether there would be some noticeable misbehavior (prior to any guests starting) if there was a dependency and it got broken by the re- ordering. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 136178: tolerable all pass - PUSHED
flight 136178 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/136178/ 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 004299fba49747ccd614c9abcae9772ec8845e20 baseline version: xen 1e31c150f6b0efac59df1824e9881b3eb00b01b5 Last test of basis 136170 2019-05-13 09:00:34 Z0 days Testing same since 136178 2019-05-13 13:00:48 Z0 days1 attempts People who touched revisions under test: Amit Singh Tomar Andrew Cooper Anthony PERARD Brian Woods Daniel De Graaf Dario Faggioli David Woodhouse Doug Goldstein Eslam Elnikety Ian Jackson Jan Beulich Julien Grall Julien Grall Kevin Tian Konrad Rzeszutek Wilk Marek Marczykowski-Górecki Olaf Hering Oleksandr Tyshchenko Roger Pau Monne Roger Pau Monné Stefano Stabellini Tamas K Lengyel Tim Deegan 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 1e31c150f6..004299fba4 004299fba49747ccd614c9abcae9772ec8845e20 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Anyone using blktap2?
Hello Seeing that you were the last people who changed blktap2 in a meaningful way: do you use it at all? I'm thinking about dropping it (again). (Obv. the wider community is welcome to voice their opinion as well.) Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hi, On 5/13/19 4:29 PM, Andrii Anisov wrote: On 13.05.19 17:34, Julien Grall wrote: My point of my message is to understand the exact workload/setup you are using. "dd" was not an entirely obvious choice for CPUBurn and Google didn't provide a lot of website backing this information. Anyway, now I understand a bit more the workload, a couple of more questions: - How many vCPUs are you using in each domain? - What scheduler are you using? - What is the affinity? For the test with glmark2: Dom0 (4 VCPUs), DomD (4 VCPUs), 4 PCPUs, no affinity specified. So, are you running 4 dd (one for each core) in parallel? Are they pinned? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 13.05.19 17:34, Julien Grall wrote: My point of my message is to understand the exact workload/setup you are using. "dd" was not an entirely obvious choice for CPUBurn and Google didn't provide a lot of website backing this information. Anyway, now I understand a bit more the workload, a couple of more questions: - How many vCPUs are you using in each domain? - What scheduler are you using? - What is the affinity? For the test with glmark2: Dom0 (4 VCPUs), DomD (4 VCPUs), 4 PCPUs, no affinity specified. For the test with TBM: Dom0 (2 VCPUs) pinned to PCPUs 0 and 1, and TBM domain with one VCPU pinned to PCPU 2. The scheduler is defaut (credit2). -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] how to disable build of pv-shim?
On Mon, May 13, 2019 at 05:20:05PM +0200, Roger Pau Monné wrote: > On Mon, May 13, 2019 at 04:53:21PM +0200, Olaf Hering wrote: > > What is the recommended way to disable CONFIG_PV_SHIM, which is set in > > tools/firmware/Makefile? From my understanding there is no way to influence > > its value from outside, which means the build always enters xen-dir/. > > I think the following should do the trick. > > Let me know if that works for you and I will submit it formally. > > Thanks! > > ---8<--- > diff --git a/config/Tools.mk.in b/config/Tools.mk.in > index 98245f63c9..84ddb1a542 100644 > --- a/config/Tools.mk.in > +++ b/config/Tools.mk.in > @@ -75,3 +75,5 @@ TINFO_LIBS := @TINFO_LIBS@ > ARGP_LDFLAGS:= @argp_ldflags@ > > FILE_OFFSET_BITS:= @FILE_OFFSET_BITS@ > + > +CONFIG_PV_SHIM := @pvshim@ > diff --git a/tools/configure.ac b/tools/configure.ac > index c9fd69ddfa..8df2fd604b 100644 > --- a/tools/configure.ac > +++ b/tools/configure.ac > @@ -492,4 +492,15 @@ AC_ARG_ENABLE([9pfs], > > AC_SUBST(ninepfs) > > +AC_ARG_ENABLE([pvshim], > +AS_HELP_STRING([--disable-pvshim], [Disable pvshim build (x86 only, > enabled by default)]), > +[AS_IF([test "x$enable_pvshim" = "xno"], [pvshim=n], [pvshim=y])], [ > +case "$host_cpu" in > +i[[3456]]86|x86_64) > + pvshim="y";; Since xen doesn't build on 32bit anymore you may want to exclude i[3456]86 here? Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/watchdog: Fold exit paths to have a single unlock
On 5/10/19 7:28 PM, Andrew Cooper wrote: > This is mostly to simplify future logical changes, but it does come with a > modest redunction in compiled code size (-55, 345 => 290). > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/5] iommu: trivial re-organisation to avoid unnecessary test
>>> On 08.05.19 at 15:23, wrote: > An 'if ( !iommu_enabled )' followed by an 'if ( iommu_enabled )' with > only a printk() in between seems a little silly. Move the printk() and > use 'else' instead. > > Signed-off-by: Paul Durrant Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/12] x86/IRQ: consolidate use of ->arch.cpu_mask
>>> On 13.05.19 at 13:32, wrote:
> On Wed, May 08, 2019 at 07:10:29AM -0600, Jan Beulich wrote:
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -680,7 +680,7 @@ void /*__init*/ setup_ioapic_dest(void)
>> continue;
>> irq = pin_2_irq(irq_entry, ioapic, pin);
>> desc = irq_to_desc(irq);
>> -BUG_ON(cpumask_empty(desc->arch.cpu_mask));
>> +BUG_ON(!cpumask_intersects(desc->arch.cpu_mask,
>> _online_map));
>
> I wonder if maybe you could instead do:
>
> if ( cpumask_intersects(desc->arch.cpu_mask, _online_map) )
> set_ioapic_affinity_irq(desc, desc->arch.cpu_mask);
> else
> ASSERT_UNREACHABLE();
>
> I guess if the IRQ is in use by Xen itself the failure ought to be
> fatal.
And imo also when it's another one (used by Dom0). Iirc we get
here only during Dom0 boot (the commented out __init serving as
a hint). Hence I think BUG_ON() is better in this case than any
for of assertion.
>> @@ -2232,11 +2231,17 @@ int io_apic_set_pci_routing (int ioapic,
>> return vector;
>> entry.vector = vector;
>>
>> -cpumask_copy(, TARGET_CPUS);
>> -/* Don't chance ending up with an empty mask. */
>> -if (cpumask_intersects(, desc->arch.cpu_mask))
>> -cpumask_and(, , desc->arch.cpu_mask);
>> -SET_DEST(entry, logical, cpu_mask_to_apicid());
>> +if (cpumask_intersects(desc->arch.cpu_mask, TARGET_CPUS)) {
>> +cpumask_t *mask = this_cpu(scratch_cpumask);
>> +
>> +cpumask_and(mask, desc->arch.cpu_mask, TARGET_CPUS);
>> +SET_DEST(entry, logical, cpu_mask_to_apicid(mask));
>> +} else {
>> +printk(XENLOG_ERR "IRQ%d: no target CPU (%*pb vs %*pb)\n",
>> + irq, nr_cpu_ids, cpumask_bits(desc->arch.cpu_mask),
>> + nr_cpu_ids, cpumask_bits(TARGET_CPUS));
>> +desc->status |= IRQ_DISABLED;
>> +}
>
> Hm, part of this file doesn't seem to use Xen coding style, but the
> chunk you add below does use it. And there are functions (like
> mask_and_ack_level_ioapic_irq that seem to use a mix of coding
> styles).
>
> I'm not sure what's the policy here, should new chunks follow Xen's
> coding style?
Well, I've decided to match surrounding code's style, until the file
gets morphed into consistent shape.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] how to disable build of pv-shim?
On Mon, May 13, 2019 at 04:53:21PM +0200, Olaf Hering wrote: > What is the recommended way to disable CONFIG_PV_SHIM, which is set in > tools/firmware/Makefile? From my understanding there is no way to influence > its value from outside, which means the build always enters xen-dir/. I think the following should do the trick. Let me know if that works for you and I will submit it formally. Thanks! ---8<--- diff --git a/config/Tools.mk.in b/config/Tools.mk.in index 98245f63c9..84ddb1a542 100644 --- a/config/Tools.mk.in +++ b/config/Tools.mk.in @@ -75,3 +75,5 @@ TINFO_LIBS := @TINFO_LIBS@ ARGP_LDFLAGS:= @argp_ldflags@ FILE_OFFSET_BITS:= @FILE_OFFSET_BITS@ + +CONFIG_PV_SHIM := @pvshim@ diff --git a/tools/configure.ac b/tools/configure.ac index c9fd69ddfa..8df2fd604b 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -492,4 +492,15 @@ AC_ARG_ENABLE([9pfs], AC_SUBST(ninepfs) +AC_ARG_ENABLE([pvshim], +AS_HELP_STRING([--disable-pvshim], [Disable pvshim build (x86 only, enabled by default)]), +[AS_IF([test "x$enable_pvshim" = "xno"], [pvshim=n], [pvshim=y])], [ +case "$host_cpu" in +i[[3456]]86|x86_64) + pvshim="y";; +*) pvshim="n";; +esac + ]) +AC_SUBST(pvshim) + AC_OUTPUT() diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile index cf304fc578..809a5fd025 100644 --- a/tools/firmware/Makefile +++ b/tools/firmware/Makefile @@ -1,10 +1,6 @@ XEN_ROOT = $(CURDIR)/../.. include $(XEN_ROOT)/tools/Rules.mk -ifneq ($(XEN_TARGET_ARCH),x86_32) -CONFIG_PV_SHIM := y -endif - # hvmloader is a 32-bit protected mode binary. TARGET := hvmloader/hvmloader INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] xen/watchdog: Drop all locked operations on the watchdog_inuse_map
On 13/05/2019 16:01, Jan Beulich wrote:
On 10.05.19 at 20:28, wrote:
>> All modifications to the watchdog_inuse_map happen with d->watchdog_lock
>> held,
>> so there are no concurrency problems to deal with.
> But concurrency problems can also occur for readers. While
> not a problem afaict, dump_domains() actually has such an
> example (and hence might be worth mentioning here).
Its only debugging, and would need to take the spinlock anyway to avoid
a TOCTOU race between watchdog_inuse_map and d->watchdog_timer[i].expires
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -1068,17 +1068,15 @@ static long domain_watchdog(struct domain *d,
>> uint32_t id, uint32_t timeout)
>> }
>> else /* Allocate the next available timer. */
>> {
>> -for ( id = 0; id < NR_DOMAIN_WATCHDOG_TIMERS; id++ )
>> -{
>> -if ( test_and_set_bit(id, >watchdog_inuse_map) )
>> -continue;
>> -break;
>> -}
>> -if ( id == NR_DOMAIN_WATCHDOG_TIMERS )
>> +id = ffs(~d->watchdog_inuse_map) - 1;
> I'm surprised we have no (universally available) ffz(). I wonder
> though whether find_first_zero_bit() wouldn't be the better
> choice here anyway, as the result wouldn't be undefined in
> case NR_DOMAIN_WATCHDOG_TIMERS grew to 32.
Sadly no - find_first_zero_bit() takes unsigned long *, so its use here
a) requires a (void *) cast to compile, and b) is definitely UB.
I didn't fancy extending d->watchdog_inuse_map to unsigned long just to
make this work, nor do I think it is likely for this interface to gain
more timers. From a usability point of view, it is rather terrible.
A far more useful interface (from a guests point of view) would be a
mechanism with a per-vcpu timer which injects an NMI on timeout, which
permits the guest far more flexibility about how to handle the timeout,
which might including dumping state or sending out a positive "I'm
fencing" broadcast. For some HA scenarios, this is preferable to
forcing everyone else to wait for the system timeout before declaring
the dead entity to have fenced.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] memory: restrict XENMEM_remove_from_physmap to translated guests
>>> On 13.05.19 at 16:35, wrote:
> On 3/5/19 1:28 PM, Jan Beulich wrote:
>> The commit re-introducing it (14eb3b41d0 ["xen: reinstate previously
>> unused XENMEM_remove_from_physmap hypercall"]) as well as the one having
>> originally introduced it (d818f3cb7c ["hvm: Use main memory for video
>> memory"]) and the one then purging it again (78c3097e4f ["Remove unused
>> XENMEM_remove_from_physmap"]) make clear that this operation is intended
>> for use on HVM (i.e. translated) guests only. Restrict it at least as
>> much, because for PV guests documentation (in the public header) does
>> not even match the implementation: It talks about GPFN as input, but
>> get_page_from_gfn() assumes a GMFN in the non-translated case (and hands
>> back the value passed in).
>>
>> Also lift the check in XENMEM_add_to_physmap{,_batch} handling up
>> directly into top level hypercall handling, and clarify things in the
>> public header accordingly.
>>
>> Take the liberty and also replace a pointless use of "current" with a
>> more efficient use of an existing local variable (or function parameter
>> to be precise).
>>
>> Signed-off-by: Jan Beulich
>
> Looks good, sorry for the delay:
>
> Reviewed-by: George Dunlap
Thanks.
> A couple of comments:
>
>> ---
>> TBD: Is using P2M_ALLOC here really appropriate? It means e.g.
>> pointlessly populating a PoD slot just to unpopulate it again right
>> away, with the page then free floating, i.e. no longer available
>> for use to replace another PoD slot, and (afaict) no longer
>> accessible by the guest in any way.
>
> Looks like the P2M_ALLOC was introduced in c/s 06e7bfed206. I can't
> immediately see any reason to do the allocation -- I think it just must
> have been C without much thought given as to what was going to happen
> next.
The question is: If we remove it, what is the -ENOENT condition
going to be?
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4470,9 +4470,6 @@ int xenmem_add_to_physmap_one(
>> mfn_t mfn = INVALID_MFN;
>> p2m_type_t p2mt;
>>
>> -if ( !paging_mode_translate(d) )
>> -return -EACCES;
>> -
>> switch ( space )
>> {
>> case XENMAPSPACE_shared_info:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -815,6 +815,8 @@ int xenmem_add_to_physmap(struct domain
>> long rc = 0;
>> union xen_add_to_physmap_batch_extra extra;
>>
>> +ASSERT(paging_mode_translate(d));
>
> So, just trying to think through the principles behind these two. We
> know that this is never going to be called w/o first calling
> xatp_permission_check(); if we assume that such a change will be tested
> (i.e., that something with paging_mode_translate() will call this
> hypercall before a release), then a single ASSERT() should be enough to
> make sure that both functions are updated properly?
Yes, that's my expectation.
> I guess that's good enough. (It's hard not to start to get paranoid
> when you ask yourself these sorts of questions.)
Good. (Indeed.)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] how to disable build of pv-shim?
On Mon, May 13, 2019 at 04:53:21PM +0200, Olaf Hering wrote: > What is the recommended way to disable CONFIG_PV_SHIM, which is set in > tools/firmware/Makefile? From my understanding there is no way to influence > its value from outside, which means the build always enters xen-dir/. > There isn't a way to disable it yet. I would suggest you add a --disable-pv-shim or something to tools' configure script. Wei. > > Olaf > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/12] x86/IRQ: fix locking around vector management
>>> On 13.05.19 at 16:45, wrote: > On Mon, May 13, 2019 at 08:19:04AM -0600, Jan Beulich wrote: >> >>> On 13.05.19 at 15:48, wrote: >> > On Wed, May 08, 2019 at 07:10:59AM -0600, Jan Beulich wrote: >> >> --- a/xen/drivers/passthrough/vtd/iommu.c >> >> +++ b/xen/drivers/passthrough/vtd/iommu.c >> >> @@ -2134,11 +2134,16 @@ static void adjust_irq_affinity(struct a >> >> unsigned int node = rhsa ? pxm_to_node(rhsa->proximity_domain) >> >> : NUMA_NO_NODE; >> >> const cpumask_t *cpumask = _online_map; >> >> +struct irq_desc *desc; >> >> >> >> if ( node < MAX_NUMNODES && node_online(node) && >> >> cpumask_intersects(_to_cpumask(node), cpumask) ) >> >> cpumask = _to_cpumask(node); >> >> -dma_msi_set_affinity(irq_to_desc(drhd->iommu->msi.irq), cpumask); >> >> + >> >> +desc = irq_to_desc(drhd->iommu->msi.irq); >> >> +spin_lock_irq(>lock); >> > >> > I would use the irqsave/irqrestore variants here for extra safety. >> >> Hmm, maybe. But I think we're in bigger trouble if IRQs indeed >> ended up enabled at any of the two points where this function >> gets called. > > I think I'm misreading the above, but if you expect > adjust_irq_affinity to always be called with interrupts disabled using > spin_unlock_irq is wrong as it unconditionally enables interrupts. Oops - s/enabled/disabled/ in my earlier reply. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] xen/watchdog: Drop all locked operations on the watchdog_inuse_map
>>> On 10.05.19 at 20:28, wrote:
> All modifications to the watchdog_inuse_map happen with d->watchdog_lock held,
> so there are no concurrency problems to deal with.
But concurrency problems can also occur for readers. While
not a problem afaict, dump_domains() actually has such an
example (and hence might be worth mentioning here).
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1068,17 +1068,15 @@ static long domain_watchdog(struct domain *d,
> uint32_t id, uint32_t timeout)
> }
> else /* Allocate the next available timer. */
> {
> -for ( id = 0; id < NR_DOMAIN_WATCHDOG_TIMERS; id++ )
> -{
> -if ( test_and_set_bit(id, >watchdog_inuse_map) )
> -continue;
> -break;
> -}
> -if ( id == NR_DOMAIN_WATCHDOG_TIMERS )
> +id = ffs(~d->watchdog_inuse_map) - 1;
I'm surprised we have no (universally available) ffz(). I wonder
though whether find_first_zero_bit() wouldn't be the better
choice here anyway, as the result wouldn't be undefined in
case NR_DOMAIN_WATCHDOG_TIMERS grew to 32.
Also while this looks to be logically independent of patch 2, it
doesn't look like it would apply on its own.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
On Mon, May 13, 2019 at 04:45:21PM +0200, Olaf Hering wrote: > Am Mon, 13 May 2019 16:37:33 +0200 > schrieb Roger Pau Monné : > > > FTR I would have preferred a pre-patch that did the move of this chunk > > of code into libxl__domain_set_device_model without any functional > > change, and then a second patch that introduces the new functionality. > > If that needs to be done, I can do it. Iff you have to resend anyway, and it's not too much fuss then I would say yes. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
On Mon, May 13, 2019 at 05:39:55PM +0300, Viktor Mitin wrote: > Hi Wei and Julien, > > Thank you for the hints provided. It is appeared that it was Yocto Xen > receipt issue and not Xen coverage feature issue. > We see that xencov* tools are built anyway, even in the case when > CONFIG_COVERAGE is not enabled in hypervisor Kconfig. > Is there a reason for this? Though the tools and hypervisor live in one repository, their build systems aren't really that connected. It wouldn't hurt to build the xencov tool in any case. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
Am Mon, 13 May 2019 16:37:33 +0200 schrieb Roger Pau Monné : > FTR I would have preferred a pre-patch that did the move of this chunk > of code into libxl__domain_set_device_model without any functional > change, and then a second patch that introduces the new functionality. If that needs to be done, I can do it. What could be done in addition is to add an assert to libxl__domain_build_info_setdefault to check if device_model_version is really set. Olaf pgpH6VCfDKBKg.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/12] x86/IRQ: fix locking around vector management
On Mon, May 13, 2019 at 08:19:04AM -0600, Jan Beulich wrote: > >>> On 13.05.19 at 15:48, wrote: > > On Wed, May 08, 2019 at 07:10:59AM -0600, Jan Beulich wrote: > >> --- a/xen/drivers/passthrough/vtd/iommu.c > >> +++ b/xen/drivers/passthrough/vtd/iommu.c > >> @@ -2134,11 +2134,16 @@ static void adjust_irq_affinity(struct a > >> unsigned int node = rhsa ? pxm_to_node(rhsa->proximity_domain) > >> : NUMA_NO_NODE; > >> const cpumask_t *cpumask = _online_map; > >> +struct irq_desc *desc; > >> > >> if ( node < MAX_NUMNODES && node_online(node) && > >> cpumask_intersects(_to_cpumask(node), cpumask) ) > >> cpumask = _to_cpumask(node); > >> -dma_msi_set_affinity(irq_to_desc(drhd->iommu->msi.irq), cpumask); > >> + > >> +desc = irq_to_desc(drhd->iommu->msi.irq); > >> +spin_lock_irq(>lock); > > > > I would use the irqsave/irqrestore variants here for extra safety. > > Hmm, maybe. But I think we're in bigger trouble if IRQs indeed > ended up enabled at any of the two points where this function > gets called. I think I'm misreading the above, but if you expect adjust_irq_affinity to always be called with interrupts disabled using spin_unlock_irq is wrong as it unconditionally enables interrupts. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] how to disable build of pv-shim?
What is the recommended way to disable CONFIG_PV_SHIM, which is set in tools/firmware/Makefile? From my understanding there is no way to influence its value from outside, which means the build always enters xen-dir/. Olaf pgpFnjX5_1j1z.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
Hi, On 5/13/19 3:39 PM, Viktor Mitin wrote: Hi Wei and Julien, --- Xen 4.13 has not been checked yet with the patch. Currently, xen 4.13 staging fails to boot due to unknown reason... it worked some days ago. It hangs after the next log currently: (XEN) Failed to bring up CPU 7 (error -5) (XEN) Brought up 4 CPUs (XEN) P2M: 44-bit IPA with 44-bit PA and 8-bit VMID (XEN) P2M: 4 levels with order-0 root, VTCR 0x80043594 (XEN) I/O virtualisation disabled (XEN) build-id: f4ea2c93ff09225beed05f629a3813b4e31c420d (XEN) alternatives: Patching with alt table 00343d58 -> 00344418 Which commit are you using? Do you remember which one worked? If so, can you bisect it? --- Julien, are you going to integrate the patch? For that someone needs to send a patch so it can be reviewed. I was hoping you could do that as the reporter of the problem. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
On Mon, May 13, 2019 at 04:37:33PM +0200, Roger Pau Monné wrote:
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index cb4702fd7a..7d75bd3850 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -106,6 +106,7 @@ libxl_device_model_version =
> > Enumeration("device_model_version", [
> > (0, "UNKNOWN"),
> > (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model
> > (qemu-dm)
> > (2, "QEMU_XEN"), # Upstream based qemu-xen device model
> > +(3, "NONE_REQUIRED"),
>
> Plain "NONE" would also be fine here IMO.
Also please add a LIBXL_HAVE macro to libxl.h
Roger, thanks for reviewing this patch.
Wei.
>
> Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] README: document that `python` is required
On Mon, May 13, 2019 at 03:38:24PM +0100, George Dunlap wrote: > On 5/13/19 2:47 PM, Wei Liu wrote: > > The hypervisor build system requires `python`. To avoid putting too > > much (confusing) information in README, mandate availability of > > `python`. > > > > Signed-off-by: Wei Liu > > --- > > README | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/README b/README > > index 23e4f7c3dc..a60ccf6e9c 100644 > > --- a/README > > +++ b/README > > @@ -181,6 +181,10 @@ Various tools, such as pygrub, have the following > > runtime dependencies: > >URL:http://www.python.org/ > >Debian: python > > > > +Note that the build system expects `python` to be available. If your system > > +only has `python2` or `python3` but not `python` (as in Linux From > > Scratch), > > +you will need to create a symlink for it. > > Since we're not in a release crunch any more, it seems like updating > ./configure to look for ""python3", "python2", and "python" (probably in > that order) would be a better solution here. No, it is not about tools build. Hypervisor build depends on `python` but we don't want it to depend on configure. See "Hypervisor build can't work without `python` (Was: Re: Xen commit 9b0bc91b3 possibly removed too much info from README)" Wei. > > -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
Hi Wei and Julien, Thank you for the hints provided. It is appeared that it was Yocto Xen receipt issue and not Xen coverage feature issue. We see that xencov* tools are built anyway, even in the case when CONFIG_COVERAGE is not enabled in hypervisor Kconfig. Is there a reason for this? To summarize, there is no need to enable coverage feature in two places, only in Kconfig, as mentioned in documentation. It has been rechecked with Xen 4.12, and works well as expected. --- Xen 4.13 has not been checked yet with the patch. Currently, xen 4.13 staging fails to boot due to unknown reason... it worked some days ago. It hangs after the next log currently: (XEN) Failed to bring up CPU 7 (error -5) (XEN) Brought up 4 CPUs (XEN) P2M: 44-bit IPA with 44-bit PA and 8-bit VMID (XEN) P2M: 4 levels with order-0 root, VTCR 0x80043594 (XEN) I/O virtualisation disabled (XEN) build-id: f4ea2c93ff09225beed05f629a3813b4e31c420d (XEN) alternatives: Patching with alt table 00343d58 -> 00344418 --- Julien, are you going to integrate the patch? Thanks On Mon, May 13, 2019 at 1:43 PM Wei Liu wrote: > > On Mon, May 13, 2019 at 01:29:12PM +0300, Viktor Mitin wrote: > > > > aarch64-poky-linux-gcc -DBUILD_ID -fno-strict-aliasing -std=gnu99 > > > > -Wall -Wstrict-prototypes -Wdeclaration-after-statement > > > > -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2 > > > > -fomit-frame-pointer > > > > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF > > > > .handlereg.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror > > > > -Wmissing-prototypes -I./include > > > > -I/home/c/w/rcar_h3_ubuntu1604_yocto/build/tmp/work/aarch64-poky-linux/xen/4.12.0.0+gitAUTOINC+fd2a34c965-r0/git/tools/libs/toolcore/../../../tools/include > > > > -c -o handlereg.o handlereg.c > > > > > > ... this looks like a tool building error when I only touch the > > > hypervisor part. Are you certain this is my patch and not another error > > > in Xen 4.12 (or any patch you have on top)? > > > > Julien, you are right, it was local environment build issue (sorry for > > that). > > Xen GCC coverage feature works well with Aarch64 with this patch. > > Checked both commands, xencov read and xencov reset - both work well > > (no crashes anymore). > > > > Please also note that use case mentioned in Xen documentation > > (xencov_split) is also ok with generated coverage.dat input: > > xen.git/xen$ ssh root@host xencov read > coverage.dat > > xen.git/xen$ ../tools/xencov_split coverage.dat --output-dir=/ > > xen.git/xen$ geninfo . -o cov.info > > xen.git/xen$ genhtml cov.info -o cov/ > > xen.git/xen$ $BROWSER cov/index.html > > > > > > Minor observation about coverage build procedure. Documentation states: > > "To build with coverage support, enable CONFIG_COVERAGE in Kconfig." > > However, to build it properly, it needs to enable coverage feature in > > two places - main xen make command line and hypervisor .config file. > > Is it expected way to build xen with coverage feature? If yes, > > probably we should improve (or at least document) it some day... > > What does your make command line look like? > > Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] README: document that `python` is required
On 5/13/19 2:47 PM, Wei Liu wrote: > The hypervisor build system requires `python`. To avoid putting too > much (confusing) information in README, mandate availability of > `python`. > > Signed-off-by: Wei Liu > --- > README | 4 > 1 file changed, 4 insertions(+) > > diff --git a/README b/README > index 23e4f7c3dc..a60ccf6e9c 100644 > --- a/README > +++ b/README > @@ -181,6 +181,10 @@ Various tools, such as pygrub, have the following > runtime dependencies: >URL:http://www.python.org/ >Debian: python > > +Note that the build system expects `python` to be available. If your system > +only has `python2` or `python3` but not `python` (as in Linux From Scratch), > +you will need to create a symlink for it. Since we're not in a release crunch any more, it seems like updating ./configure to look for ""python3", "python2", and "python" (probably in that order) would be a better solution here. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
On Fri, May 10, 2019 at 05:20:47PM +0200, Olaf Hering wrote:
> If a domU has a qemu-xen instance attached, it is required to call qemus
> "xen-save-devices-state" method. Without it, the receiving side of a PV or
> PVH migration may be unable to lock the image:
>
> xen be: qdisk-51712: xen be: qdisk-51712: error: Failed to get "write" lock
> error: Failed to get "write" lock
> xen be: qdisk-51712: xen be: qdisk-51712: initialise() failed
> initialise() failed
>
> To fix this bug, libxl__domain_suspend_device_model() and
> libxl__domain_resume_device_model() have to be called not only for HVM,
> but also if the active device_model is QEMU_XEN.
>
> Unfortunately, libxl__domain_build_info_setdefault() hardcodes
> b_info->device_model_version to QEMU_XEN if it does not know it any
> better. As a result libxl__device_model_version_running() will return
> incorrect values. This breaks domUs without a device_model.
> libxl__qmp_stop() would wait 10 seconds in qmp_open() for a qemu that
> will never appear. During this long timeframe the domU remains in state
> paused on the sending side. As a result network connections may be
> dropped. Once this bug is fixed as well, by just removing the assumption
> that every domU has a QEMU_XEN, there is no code to actually initialise
> b_info->device_model_version.
>
> There is a helper function libxl__need_xenpv_qemu(), which is used in
> various places to decide if a device_model has to be spawned. This
> function can not be used as is, just to fill device_model_version,
> because store_libxl_entry() was already called earlier.
>
> Create a new function to set device_model_version. Move existing code
> from libxl__domain_build_info_setdefault() to cover the HVM case. Add
> new code to cover non-HVM case, use libxl__need_xenpv_qemu() to set
> device_model_version.
> Move also initialization for device_model_stubdomain to the new function.
>
> Update libxl__spawn_stub_dm() and initiate_domain_create() to call the
> new function prior libxl__domain_build_info_setdefault() because
> device_mode_version is expected to be initialzed.
> libxl_domain_need_memory() needs no update because it does not have a
> d_config available anyway, and the callers provide a populated b_info.
>
> Introduce LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED for PV and PVH that
> have no need for a device_model to make the state explicit.
>
> v03:
> - rearrange code to make sure device_model_version is initialized before
> store_libxl_entry() is called
> v02:
> - update wording in a comment
> - remove stale goto in domcreate_launch_dm
> - initialize ret in libxl__need_xenpv_qemu
>
> Signed-off-by: Olaf Hering
> Cc: Roger Pau Monné
> Cc: Anthony PERARD
LGTM:
Reviewed-by: Roger Pau Monné
> ---
> tools/libxl/libxl_create.c | 99
> ++---
> tools/libxl/libxl_dm.c | 2 +
> tools/libxl/libxl_dom_suspend.c | 8 +++-
> tools/libxl/libxl_internal.h| 2 +
> tools/libxl/libxl_types.idl | 1 +
> 5 files changed, 75 insertions(+), 37 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 89fe80fc9c..bc4613a296 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -27,6 +27,64 @@
>
> #include
>
> +int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config
> *d_config)
> +{
> +libxl_domain_build_info *b_info = _config->b_info;
> +int ret;
> +
> +libxl_defbool_setdefault(_info->device_model_stubdomain, false);
> +
> +if (b_info->device_model_version)
> +return 0;
> +
> +switch (b_info->type) {
> +case LIBXL_DOMAIN_TYPE_HVM:
> +if (libxl_defbool_val(b_info->device_model_stubdomain)) {
> +b_info->device_model_version =
> +LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> +} else {
> +b_info->device_model_version = libxl__default_device_model(gc);
> +}
> +break;
> +default:
> +ret = libxl__need_xenpv_qemu(gc, d_config);
> +switch (ret) {
> +case 1:
> +d_config->b_info.device_model_version =
> +LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> +break;
> +case 0:
> +d_config->b_info.device_model_version =
> +LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED;
> +break;
> +default:
> +LOGE(ERROR, "Unable to determine QEMU requisite");
> +return ERROR_FAIL;
You could just return ret and propagate the error from the previous
function.
> +}
> +}
> +
> +if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN)
> {
> +const char *dm;
> +
> +dm = libxl__domain_device_model(gc, b_info);
> +ret = access(dm, X_OK);
> +if (ret < 0) {
> +/* qemu-xen unavailable, use qemu-xen-traditional */
> +if (errno == ENOENT) {
> +LOGE(INFO, "qemu-xen is
Re: [Xen-devel] [PATCH 3/4] README: document that `python` is required
On Mon, May 13, 2019 at 08:27:16AM -0600, Jan Beulich wrote: > >>> On 13.05.19 at 15:47, wrote: > > --- a/README > > +++ b/README > > @@ -181,6 +181,10 @@ Various tools, such as pygrub, have the following > > runtime dependencies: > >URL:http://www.python.org/ > >Debian: python > > > > +Note that the build system expects `python` to be available. If your system > > +only has `python2` or `python3` but not `python` (as in Linux From > > Scratch), > > +you will need to create a symlink for it. > > Is creating a symlink indeed the only option? What about specifying > PYTHON= on the make cmdline? I don't mean to say the set of > workarounds needs to be exhaustive here, but perhaps add at > least "e.g." if there are other options as well? Xen's build system is strange in that make A=B is not the same as A=B make . In practice this has bitten a few people in the past. That's why I opted to not mention that variant. But since you ask, I can add it to next version. Wei. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] memory: restrict XENMEM_remove_from_physmap to translated guests
On 3/5/19 1:28 PM, Jan Beulich wrote:
> The commit re-introducing it (14eb3b41d0 ["xen: reinstate previously
> unused XENMEM_remove_from_physmap hypercall"]) as well as the one having
> originally introduced it (d818f3cb7c ["hvm: Use main memory for video
> memory"]) and the one then purging it again (78c3097e4f ["Remove unused
> XENMEM_remove_from_physmap"]) make clear that this operation is intended
> for use on HVM (i.e. translated) guests only. Restrict it at least as
> much, because for PV guests documentation (in the public header) does
> not even match the implementation: It talks about GPFN as input, but
> get_page_from_gfn() assumes a GMFN in the non-translated case (and hands
> back the value passed in).
>
> Also lift the check in XENMEM_add_to_physmap{,_batch} handling up
> directly into top level hypercall handling, and clarify things in the
> public header accordingly.
>
> Take the liberty and also replace a pointless use of "current" with a
> more efficient use of an existing local variable (or function parameter
> to be precise).
>
> Signed-off-by: Jan Beulich
Looks good, sorry for the delay:
Reviewed-by: George Dunlap
A couple of comments:
> ---
> TBD: Is using P2M_ALLOC here really appropriate? It means e.g.
> pointlessly populating a PoD slot just to unpopulate it again right
> away, with the page then free floating, i.e. no longer available
> for use to replace another PoD slot, and (afaict) no longer
> accessible by the guest in any way.
Looks like the P2M_ALLOC was introduced in c/s 06e7bfed206. I can't
immediately see any reason to do the allocation -- I think it just must
have been C without much thought given as to what was going to happen
next.
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4470,9 +4470,6 @@ int xenmem_add_to_physmap_one(
> mfn_t mfn = INVALID_MFN;
> p2m_type_t p2mt;
>
> -if ( !paging_mode_translate(d) )
> -return -EACCES;
> -
> switch ( space )
> {
> case XENMAPSPACE_shared_info:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -815,6 +815,8 @@ int xenmem_add_to_physmap(struct domain
> long rc = 0;
> union xen_add_to_physmap_batch_extra extra;
>
> +ASSERT(paging_mode_translate(d));
So, just trying to think through the principles behind these two. We
know that this is never going to be called w/o first calling
xatp_permission_check(); if we assume that such a change will be tested
(i.e., that something with paging_mode_translate() will call this
hypercall before a release), then a single ASSERT() should be enough to
make sure that both functions are updated properly?
I guess that's good enough. (It's hard not to start to get paranoid
when you ask yourself these sorts of questions.)
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 5/13/19 3:14 PM, Andrii Anisov wrote: Hello Julien, Hello, On 13.05.19 14:16, Julien Grall wrote: I am afraid I can't possible back this assumption. As I pointed out in the previous version, I would be OK with the always map solution on Arm32 (pending performance) because it would be possible to increase the virtual address area by reworking the address space. I'm sorry, I'm not sure what should be my actions about that. There no code modification involved so far. Just updating your cover letter with what I just said above. OK, I'll take it for the next version. The patch looks wrong to me. You are using virt_to_phys() on a percpu area. What does actually promise you the physical address will always be the same? Sorry for my ignorance here, could you please elaborate more about what is wrong here? While the virtual address will never change over over the life cycle of a variable, I am not entirely sure we can make the same assumption for the physical address. I know that kmalloc() is promising you that the physical address will not change. But percpu does not seem to use kmalloc() so have you confirmed this assumption can hold? I need a bit more time to investigate. Are you saying that the command dd is the CPUBurn? I am not sure how this could be considered as a CPUBurn. IHMO, this is more IO related. Both /dev/null and /dev/zero are virtual devices no actual IO is performed during their operations, all the load is CPU (user and sys). Thank you for the explanation. Shall I guess this is an existing benchmark [1]? Well, "dd if=/dev/zero of=/dev/null" is just a trivial way to get one rn CPU core busy. It works for (almost) any Linux system without any additional movements. Using another trivial GO application for that purpose, seems to me like an overkill. Yet if you insist, I can use it. My point of my message is to understand the exact workload/setup you are using. "dd" was not an entirely obvious choice for CPUBurn and Google didn't provide a lot of website backing this information. Anyway, now I understand a bit more the workload, a couple of more questions: - How many vCPUs are you using in each domain? - What scheduler are you using? - What is the affinity? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/4] INSTALL: remove duplicate sentence
On Mon, May 13, 2019 at 08:29:14AM -0600, Jan Beulich wrote: > >>> On 13.05.19 at 15:47, wrote: > > --- a/INSTALL > > +++ b/INSTALL > > @@ -225,7 +225,6 @@ XEN_BUILD_TIME=hh:mm:ss > > SMBIOS_REL_DATE=mm/dd/ > > VGABIOS_REL_DATE="dd Mon " > > > > -During tools build external repos will be cloned into the source tree. > > This variable can be used to point to a different git binary to be used. > > GIT= > > To me it would seem more logical to delete the other, later > instance of the sentence. You wouldn't need $(GIT) if there > was no cloning. But deleting that one would be wrong -- it would make that paragraph seems to be only about stubdom, while in fact most of those envars are about tools build. Wei. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hello Julien, On 13.05.19 13:50, Julien Grall wrote: Also, your DomD .config has CONFIG_UNMAP_KERNEL_AT_EL0. So how do you disable kpti? Sorry for the mess, I was looking for and did not find "CONFIG_PAGE_TABLE_ISOLATION". What was googled by me as a KPTI enable config. But it is for x86, what I've overlooked. So I have KPTI enabled here. Thank you. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/4] INSTALL: remove duplicate sentence
>>> On 13.05.19 at 15:47, wrote: > --- a/INSTALL > +++ b/INSTALL > @@ -225,7 +225,6 @@ XEN_BUILD_TIME=hh:mm:ss > SMBIOS_REL_DATE=mm/dd/ > VGABIOS_REL_DATE="dd Mon " > > -During tools build external repos will be cloned into the source tree. > This variable can be used to point to a different git binary to be used. > GIT= To me it would seem more logical to delete the other, later instance of the sentence. You wouldn't need $(GIT) if there was no cloning. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] docs/xl: Clarify documentation for mem-max and mem-set
On Mon, May 13, 2019 at 02:59:30PM +0100, Wei Liu wrote: > On Wed, May 01, 2019 at 03:59:41PM +0100, George Dunlap wrote: > > On 4/8/19 12:09 PM, George Dunlap wrote: > > > mem-set is the primary command that users will need to use and > > > understand. Move it first, and clarify the wording; also specify that > > > you can't set the target higher than maxmem from the domain config. > > > > > > mem-max is actually a pretty useless command at the moment. Clarify > > > that users are not expected to use it; and document all of its quirky > > > behavior. > > > > > > Signed-off-by: George Dunlap > > > > Wei / Ian: Ping? > > > > Juergen replied to my "review note" comment, not to anything actionable > > in the patch (or commit message) itself. > > Acked-by: Wei Liu > > (I already said this looked okay to me in v1) I believe Lars' Rb from v1 still stands. I will add while I commit this patch. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] README: document that `python` is required
>>> On 13.05.19 at 15:47, wrote: > --- a/README > +++ b/README > @@ -181,6 +181,10 @@ Various tools, such as pygrub, have the following > runtime dependencies: >URL:http://www.python.org/ >Debian: python > > +Note that the build system expects `python` to be available. If your system > +only has `python2` or `python3` but not `python` (as in Linux From Scratch), > +you will need to create a symlink for it. Is creating a symlink indeed the only option? What about specifying PYTHON= on the make cmdline? I don't mean to say the set of workarounds needs to be exhaustive here, but perhaps add at least "e.g." if there are other options as well? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/4] public/tmem.h: fix version number in comment
>>> On 13.05.19 at 15:47, wrote: > The version number has been changed above due to rebasing onto 4.13 > branch, but the one in the matching comment was left unchanged. > > Signed-off-by: Wei Liu Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/12] x86/IRQ: fix locking around vector management
>>> On 13.05.19 at 15:48, wrote: > On Wed, May 08, 2019 at 07:10:59AM -0600, Jan Beulich wrote: >> --- a/xen/drivers/passthrough/vtd/iommu.c >> +++ b/xen/drivers/passthrough/vtd/iommu.c >> @@ -2134,11 +2134,16 @@ static void adjust_irq_affinity(struct a >> unsigned int node = rhsa ? pxm_to_node(rhsa->proximity_domain) >> : NUMA_NO_NODE; >> const cpumask_t *cpumask = _online_map; >> +struct irq_desc *desc; >> >> if ( node < MAX_NUMNODES && node_online(node) && >> cpumask_intersects(_to_cpumask(node), cpumask) ) >> cpumask = _to_cpumask(node); >> -dma_msi_set_affinity(irq_to_desc(drhd->iommu->msi.irq), cpumask); >> + >> +desc = irq_to_desc(drhd->iommu->msi.irq); >> +spin_lock_irq(>lock); > > I would use the irqsave/irqrestore variants here for extra safety. Hmm, maybe. But I think we're in bigger trouble if IRQs indeed ended up enabled at any of the two points where this function gets called. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] gitlab-ci: allow specifying base and tip in build test
Doug, ping?
On Tue, Apr 16, 2019 at 08:21:39AM +0100, Wei Liu wrote:
> We will soon provide this new capability to humans and automated
> systems.
>
> The default behaviour is retained: tip and base are passed by Gitlab
> CI.
>
> Signed-off-by: Wei Liu
> ---
> automation/gitlab-ci/build-each-commit.sh | 10 +-
> automation/gitlab-ci/test.yaml| 2 +-
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/automation/gitlab-ci/build-each-commit.sh
> b/automation/gitlab-ci/build-each-commit.sh
> index 879028b5a7..19e337b468 100755
> --- a/automation/gitlab-ci/build-each-commit.sh
> +++ b/automation/gitlab-ci/build-each-commit.sh
> @@ -1,18 +1,18 @@
> #!/bin/bash
>
> # For a newly pushed branch the BEFORE_SHA will be all 0s
> -if [[ ${CI_COMMIT_BEFORE_SHA} ==
> ]]; then
> +if [[ ${BASE} == ]]; then
> echo "Newly pushed branch, skipped"
> exit 0
> fi
>
> -git merge-base --is-ancestor ${CI_COMMIT_BEFORE_SHA} ${CI_COMMIT_SHA}
> +git merge-base --is-ancestor ${BASE} ${TIP}
> if [[ $? -ne 0 ]]; then
> -echo "${CI_COMMIT_SHA} is not a descendent of ${CI_COMMIT_BEFORE_SHA},
> skipped"
> +echo "${TIP} is not a descendent of ${BASE}, skipped"
> exit 0
> fi
>
> -echo "Building ${CI_COMMIT_BEFORE_SHA}..${CI_COMMIT_SHA}"
> +echo "Building ${BASE}..${TIP}"
>
> -NON_SYMBOLIC_REF=1 ./automation/scripts/build-test.sh
> ${CI_COMMIT_BEFORE_SHA} ${CI_COMMIT_SHA} \
> +NON_SYMBOLIC_REF=1 ./automation/scripts/build-test.sh ${BASE} ${TIP} \
> bash -c "git clean -ffdx && ./automation/scripts/build"
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index d4556afe11..a795866673 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -7,7 +7,7 @@ build-each-commit-gcc:
> XEN_TARGET_ARCH: x86_64
> CC: gcc
>script:
> -- ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee
> build-each-commit-gcc.log
> +- BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}}
> TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh
> 2>&1 | tee build-each-commit-gcc.log
>artifacts:
> paths:
>- '*.log'
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] gitignore: ignore .vscode directory
On Mon, May 13, 2019 at 03:13:06PM +0100, Anthony PERARD wrote: > On Mon, May 13, 2019 at 02:47:11PM +0100, Wei Liu wrote: > > The directory is created by Visual Studio Code editor to store its > > local state. > > > > Signed-off-by: Wei Liu > > --- > > .gitignore | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/.gitignore b/.gitignore > > index 26bc583f74..3822bb75ba 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -30,6 +30,7 @@ cscope.out > > cscope.po.out > > .config > > .vimrc > > +.vscode > > Shouldn't this go in "~/.config/git/ignore" instead? Or whatever `git > config core.excludesFile` point to. > > `git help ignore` for more detail. We already put a bunch of files for various tools in xen.git's gitignore. I don't see why this can't be done for vscode. Wei. > > -- > Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hello Julien, On 13.05.19 14:16, Julien Grall wrote: I am afraid I can't possible back this assumption. As I pointed out in the previous version, I would be OK with the always map solution on Arm32 (pending performance) because it would be possible to increase the virtual address area by reworking the address space. I'm sorry, I'm not sure what should be my actions about that. There no code modification involved so far. Just updating your cover letter with what I just said above. OK, I'll take it for the next version. The patch looks wrong to me. You are using virt_to_phys() on a percpu area. What does actually promise you the physical address will always be the same? Sorry for my ignorance here, could you please elaborate more about what is wrong here? While the virtual address will never change over over the life cycle of a variable, I am not entirely sure we can make the same assumption for the physical address. I know that kmalloc() is promising you that the physical address will not change. But percpu does not seem to use kmalloc() so have you confirmed this assumption can hold? I need a bit more time to investigate. Are you saying that the command dd is the CPUBurn? I am not sure how this could be considered as a CPUBurn. IHMO, this is more IO related. Both /dev/null and /dev/zero are virtual devices no actual IO is performed during their operations, all the load is CPU (user and sys). Thank you for the explanation. Shall I guess this is an existing benchmark [1]? Well, "dd if=/dev/zero of=/dev/null" is just a trivial way to get one CPU core busy. It works for (almost) any Linux system without any additional movements. Using another trivial GO application for that purpose, seems to me like an overkill. Yet if you insist, I can use it. I did run it until avg more ore less stabilizes (2-3 minutes), then took the minimal avg (note, we have a moving average there). Did you re-run multiple time?Yes, sure. These are not the one try results. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] gitignore: ignore .vscode directory
On Mon, May 13, 2019 at 02:47:11PM +0100, Wei Liu wrote: > The directory is created by Visual Studio Code editor to store its > local state. > > Signed-off-by: Wei Liu > --- > .gitignore | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.gitignore b/.gitignore > index 26bc583f74..3822bb75ba 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -30,6 +30,7 @@ cscope.out > cscope.po.out > .config > .vimrc > +.vscode Shouldn't this go in "~/.config/git/ignore" instead? Or whatever `git config core.excludesFile` point to. `git help ignore` for more detail. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/hvm: Generic instruction re-execution mechanism for execute faults
>>> On 13.05.19 at 15:58, wrote: > On 11/27/18 12:49 PM, Razvan Cojocaru wrote: >>> With a sufficiently complete insn emulator, single-stepping should >>> not be needed at all imo. Granted we're not quite there yet with >>> the emulator, but we've made quite a bit of progress. As before, >>> if there are particular instructions you know of that the emulator >>> doesn't handle yet, please keep pointing these out. Last I know >>> were some AVX move instructions, which have long been >>> implemented. >> True, I haven't seen emulator issues in that respect with staging - the >> emulator appears lately to be sufficiently complete. Thank you very much >> for your help and support - we'll definitely point out unsupported >> instructions if we spot some again. > > We've come accross a new instruction that the emulator can't handle in > Xen 4.13-unstable today: > > vpmaddwd xmm4,xmm4,XMMWORD PTR ds:0x513fbb20 > > Perhaps there are plans for this to go into the emulator as well? You're kidding? This is already in 4.12.0, and if it weren't I'm sure you're aware there are about 40 more AVX512 patches pending review. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/watchdog: Fold exit paths to have a single unlock
>>> On 13.05.19 at 15:51, wrote:
> On 13/05/2019 14:47, Jan Beulich wrote:
> On 10.05.19 at 20:28, wrote:
>>> --- a/xen/common/schedule.c
>>> +++ b/xen/common/schedule.c
>>> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>>>
>>> static long domain_watchdog(struct domain *d, uint32_t id, uint32_t
>>> timeout)
>>> {
>>> +long rc = 0;
>> I'm wondering why this isn't plain int. Not a big deal, but I'm
>> curious anyway.
>
> To match the return value.
But the compiler will happily sign-extend the value at the return statement.
> This function is compiled twice AFAICT.
I have no idea how this matters.
>> As per your own response to patch 2 I understand that the
>> other patches in this series don#t need looking at until you
>> send a v2.
>
> patch 3 is independent of the ABI changes, so fine in principle to
> review now.
Okay.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] IOMMU/x86: make page type checks consistent when mapping pages
>>> On 13.05.19 at 15:44, wrote:
> On 3/5/19 1:26 PM, Jan Beulich wrote:
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -192,21 +192,27 @@ void __hwdom_init iommu_hwdom_init(struc
>>
>> page_list_for_each ( page, >page_list )
>> {
>> -unsigned long mfn = mfn_x(page_to_mfn(page));
>> -unsigned long dfn = mfn_to_gmfn(d, mfn);
>> -unsigned int mapping = IOMMUF_readable;
>> -int ret;
>> +if ( (page->u.inuse.type_info & PGT_type_mask) == PGT_none )
>> +{
>> +ASSERT(!(page->u.inuse.type_info & PGT_count_mask));
>> +if ( get_page_and_type(page, d, PGT_writable_page) )
>> +put_page_and_type(page);
>> +else if ( !rc )
>> +rc = -EBUSY;
>> +}
>>
>> -if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) ||
>> - ((page->u.inuse.type_info & PGT_type_mask)
>> - == PGT_writable_page) )
>> -mapping |= IOMMUF_writable;
>> +if ( ((page->u.inuse.type_info & PGT_type_mask) ==
>> + PGT_writable_page) )
>> +{
>> +unsigned long mfn = mfn_x(page_to_mfn(page));
>> +unsigned long dfn = mfn_to_gmfn(d, mfn);
>> +int ret = iommu_map(d, _dfn(dfn), _mfn(mfn), 0,
>> +IOMMUF_readable | IOMMUF_writable,
>> +_flags);
>
> What's the idea behind calling iommu_map() here, rather than relying on
> the one in _get_page_type()? Does need_iommu_pt_sync() not work yet at
> this point, or do you expect there to be pages that have been marked
> PGT_writable without having gone through _get_page_type()?
No, I think I simply didn't realize that this could be deleted altogether
with the added get_page_and_type() invocation.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] x86/hvm: Generic instruction re-execution mechanism for execute faults
On 11/27/18 12:49 PM, Razvan Cojocaru wrote: With a sufficiently complete insn emulator, single-stepping should not be needed at all imo. Granted we're not quite there yet with the emulator, but we've made quite a bit of progress. As before, if there are particular instructions you know of that the emulator doesn't handle yet, please keep pointing these out. Last I know were some AVX move instructions, which have long been implemented. True, I haven't seen emulator issues in that respect with staging - the emulator appears lately to be sufficiently complete. Thank you very much for your help and support - we'll definitely point out unsupported instructions if we spot some again. We've come accross a new instruction that the emulator can't handle in Xen 4.13-unstable today: vpmaddwd xmm4,xmm4,XMMWORD PTR ds:0x513fbb20 Perhaps there are plans for this to go into the emulator as well? Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xenbus: Avoid deadlock during suspend due to open transactions
During a suspend/resume, the xenwatch thread waits for all outstanding
xenstore requests and transactions to complete. This does not work
correctly for transactions started by userspace because it waits for
them to complete after freezing userspace threads which means the
transactions have no way of completing, resulting in a deadlock. This is
trivial to reproduce by running this script and then suspending the VM:
import pyxs, time
c = pyxs.client.Client(xen_bus_path="/dev/xen/xenbus")
c.connect()
c.transaction()
time.sleep(3600)
Even if this deadlock were resolved, misbehaving userspace should not
prevent a VM from being migrated. So, instead of waiting for these
transactions to complete before suspending, store the current generation
id for each transaction when it is started. The global generation id is
incremented during resume. If the caller commits the transaction and the
generation id does not match the current generation id, return EAGAIN so
that they try again. If the transaction was instead discarded, return OK
since no changes were made anyway.
This only affects users of the xenbus file interface. In-kernel users of
xenbus are assumed to be well-behaved and complete all transactions
before freezing.
Signed-off-by: Ross Lagerwall
---
Changed in v2: rewrote according to Juergen's suggestion.
drivers/xen/xenbus/xenbus.h | 3 +++
drivers/xen/xenbus/xenbus_dev_frontend.c | 18 ++
drivers/xen/xenbus/xenbus_xs.c | 7 +--
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h
index 092981171df1..d75a2385b37c 100644
--- a/drivers/xen/xenbus/xenbus.h
+++ b/drivers/xen/xenbus/xenbus.h
@@ -83,6 +83,7 @@ struct xb_req_data {
int num_vecs;
int err;
enum xb_req_state state;
+ bool user_req;
void (*cb)(struct xb_req_data *);
void *par;
};
@@ -133,4 +134,6 @@ void xenbus_ring_ops_init(void);
int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par);
void xenbus_dev_queue_reply(struct xb_req_data *req);
+extern unsigned int xb_dev_generation_id;
+
#endif
diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c
b/drivers/xen/xenbus/xenbus_dev_frontend.c
index 0782ff3c2273..39c63152a358 100644
--- a/drivers/xen/xenbus/xenbus_dev_frontend.c
+++ b/drivers/xen/xenbus/xenbus_dev_frontend.c
@@ -62,6 +62,8 @@
#include "xenbus.h"
+unsigned int xb_dev_generation_id;
+
/*
* An element of a list of outstanding transactions, for which we're
* still waiting a reply.
@@ -69,6 +71,7 @@
struct xenbus_transaction_holder {
struct list_head list;
struct xenbus_transaction handle;
+ unsigned int generation_id;
};
/*
@@ -441,6 +444,7 @@ static int xenbus_write_transaction(unsigned msg_type,
rc = -ENOMEM;
goto out;
}
+ trans->generation_id = xb_dev_generation_id;
list_add(>list, >transactions);
} else if (msg->hdr.tx_id != 0 &&
!xenbus_get_transaction(u, msg->hdr.tx_id))
@@ -449,6 +453,20 @@ static int xenbus_write_transaction(unsigned msg_type,
!(msg->hdr.len == 2 &&
(!strcmp(msg->body, "T") || !strcmp(msg->body, "F"
return xenbus_command_reply(u, XS_ERROR, "EINVAL");
+ else if (msg_type == XS_TRANSACTION_END) {
+ trans = xenbus_get_transaction(u, msg->hdr.tx_id);
+ if (trans && trans->generation_id != xb_dev_generation_id) {
+ list_del(>list);
+ kfree(trans);
+ if (!strcmp(msg->body, "T"))
+ return xenbus_command_reply(u, XS_ERROR,
+ "EAGAIN");
+ else
+ return xenbus_command_reply(u,
+ XS_TRANSACTION_END,
+ "OK");
+ }
+ }
rc = xenbus_dev_request_and_reply(>hdr, u);
if (rc && trans) {
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 49a3874ae6bb..ddc18da61834 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -105,6 +105,7 @@ static void xs_suspend_enter(void)
static void xs_suspend_exit(void)
{
+ xb_dev_generation_id++;
spin_lock(_state_lock);
xs_suspend_active--;
spin_unlock(_state_lock);
@@ -125,7 +126,7 @@ static uint32_t xs_request_enter(struct xb_req_data *req)
spin_lock(_state_lock);
}
- if (req->type == XS_TRANSACTION_START)
+ if (req->type == XS_TRANSACTION_START && !req->user_req)
xs_state_users++;
xs_state_users++;
rq_id = xs_request_id++;
@@ -140,7 +141,7 @@
Re: [Xen-devel] [PATCH] x86/IO-APIC: dump full destination ID in x2APIC mode
On Tue, Apr 02, 2019 at 07:04:41AM -0600, Jan Beulich wrote: > In x2APIC mode it is 32 bits wide. > > In __print_IO_APIC() drop logging of both physical and logical IDs: > The latter covers a superset of the bits of the former in the RTE, and > we write full 8-bit values anyway even in physical mode for all ordinary > interrupts, regardless of INT_DEST_MODE (see the users of SET_DEST()). > > Adjust other column arrangement (and heading) a little as well. > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] install pkgconfig files into libdir
On Mon, Mar 25, 2019 at 05:00:10PM +0100, Olaf Hering wrote: > Most pkgconfig files contain a Libs: variable, which is either /usr/lib > or /usr/lib64. If a 32bit and a 64bit variant of xen libraries is > installed, the last one wins. As a result compiling for the other > bitsize will fail. > > Instead of sharedir use libdir as install target. This matches both the > documentation and the expected result. > > Signed-off-by: Olaf Hering Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/watchdog: Fold exit paths to have a single unlock
On 13/05/2019 14:47, Jan Beulich wrote:
On 10.05.19 at 20:28, wrote:
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>>
>> static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
>> {
>> +long rc = 0;
> I'm wondering why this isn't plain int. Not a big deal, but I'm
> curious anyway.
To match the return value. This function is compiled twice AFAICT.
>
> As per your own response to patch 2 I understand that the
> other patches in this series don#t need looking at until you
> send a v2.
patch 3 is independent of the ABI changes, so fine in principle to
review now.
Patches 2 and 4 will definitely need changing.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 07/12] x86/IRQ: fix locking around vector management
On Wed, May 08, 2019 at 07:10:59AM -0600, Jan Beulich wrote:
> All of __{assign,bind,clear}_irq_vector() manipulate struct irq_desc
> fields, and hence ought to be called with the descriptor lock held in
> addition to vector_lock. This is currently the case for only
> set_desc_affinity() (in the common case) and destroy_irq(), which also
> clarifies what the nesting behavior between the locks has to be.
> Reflect the new expectation by having these functions all take a
> descriptor as parameter instead of an interrupt number.
>
> Also take care of the two special cases of calls to set_desc_affinity():
> set_ioapic_affinity_irq() and VT-d's dma_msi_set_affinity() get called
> directly as well, and in these cases the descriptor locks hadn't got
> acquired till now. For set_ioapic_affinity_irq() this means acquiring /
> releasing of the IO-APIC lock can be plain spin_{,un}lock() then.
>
> Drop one of the two leading underscores from all three functions at
> the same time.
>
> There's one case left where descriptors get manipulated with just
> vector_lock held: setup_vector_irq() assumes its caller to acquire
> vector_lock, and hence can't itself acquire the descriptor locks (wrong
> lock order). I don't currently see how to address this.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Roger Pau Monné
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2134,11 +2134,16 @@ static void adjust_irq_affinity(struct a
> unsigned int node = rhsa ? pxm_to_node(rhsa->proximity_domain)
> : NUMA_NO_NODE;
> const cpumask_t *cpumask = _online_map;
> +struct irq_desc *desc;
>
> if ( node < MAX_NUMNODES && node_online(node) &&
> cpumask_intersects(_to_cpumask(node), cpumask) )
> cpumask = _to_cpumask(node);
> -dma_msi_set_affinity(irq_to_desc(drhd->iommu->msi.irq), cpumask);
> +
> +desc = irq_to_desc(drhd->iommu->msi.irq);
> +spin_lock_irq(>lock);
I would use the irqsave/irqrestore variants here for extra safety.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/watchdog: Fold exit paths to have a single unlock
>>> On 10.05.19 at 20:28, wrote:
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1050,6 +1050,8 @@ static void domain_watchdog_timeout(void *data)
>
> static long domain_watchdog(struct domain *d, uint32_t id, uint32_t timeout)
> {
> +long rc = 0;
I'm wondering why this isn't plain int. Not a big deal, but I'm
curious anyway.
As per your own response to patch 2 I understand that the
other patches in this series don#t need looking at until you
send a v2.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4/4] INSTALL: remove duplicate sentence
The same sentence is repeated in the next paragraph. Signed-off-by: Wei Liu --- INSTALL | 1 - 1 file changed, 1 deletion(-) diff --git a/INSTALL b/INSTALL index 9aa9ebdddc..1665ddd6a4 100644 --- a/INSTALL +++ b/INSTALL @@ -225,7 +225,6 @@ XEN_BUILD_TIME=hh:mm:ss SMBIOS_REL_DATE=mm/dd/ VGABIOS_REL_DATE="dd Mon " -During tools build external repos will be cloned into the source tree. This variable can be used to point to a different git binary to be used. GIT= -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/4] public/tmem.h: fix version number in comment
The version number has been changed above due to rebasing onto 4.13 branch, but the one in the matching comment was left unchanged. Signed-off-by: Wei Liu --- xen/include/public/tmem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h index d9b1c266f6..362ba45d5a 100644 --- a/xen/include/public/tmem.h +++ b/xen/include/public/tmem.h @@ -107,7 +107,7 @@ typedef struct tmem_op tmem_op_t; DEFINE_XEN_GUEST_HANDLE(tmem_op_t); #endif -#endif /* __XEN_INTERFACE_VERSION__ < 0x00041200 */ +#endif /* __XEN_INTERFACE_VERSION__ < 0x00041300 */ #endif /* __XEN_PUBLIC_TMEM_H__ */ -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/4] gitignore: ignore .vscode directory
The directory is created by Visual Studio Code editor to store its local state. Signed-off-by: Wei Liu --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 26bc583f74..3822bb75ba 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,7 @@ cscope.out cscope.po.out .config .vimrc +.vscode dist stubdom/*.tar.gz -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/4] README: document that `python` is required
The hypervisor build system requires `python`. To avoid putting too much (confusing) information in README, mandate availability of `python`. Signed-off-by: Wei Liu --- README | 4 1 file changed, 4 insertions(+) diff --git a/README b/README index 23e4f7c3dc..a60ccf6e9c 100644 --- a/README +++ b/README @@ -181,6 +181,10 @@ Various tools, such as pygrub, have the following runtime dependencies: URL:http://www.python.org/ Debian: python +Note that the build system expects `python` to be available. If your system +only has `python2` or `python3` but not `python` (as in Linux From Scratch), +you will need to create a symlink for it. + Intel(R) Trusted Execution Technology Support = -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/4] Misc patches
Wei Liu (4): gitignore: ignore .vscode directory public/tmem.h: fix version number in comment README: document that `python` is required INSTALL: remove duplicate sentence .gitignore| 1 + INSTALL | 1 - README| 4 xen/include/public/tmem.h | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] IOMMU/x86: make page type checks consistent when mapping pages
On 3/5/19 1:26 PM, Jan Beulich wrote:
> There are currently three more or less different checks:
> - _get_page_type() adjusts the IOMMU mappings according to the new type
> alone,
> - arch_iommu_populate_page_table() wants just the type to be
> PGT_writable_page,
> - iommu_hwdom_init() additionally permits all other types with a type
> refcount of zero.
> The canonical one is in _get_page_type(), and as of XSA-288
> arch_iommu_populate_page_table() also has no need anymore to deal with
> PGT_none pages. In the PV Dom0 case, however, PGT_none pages are still
> necessary to consider, since in that case pages don't get handed to
> guest_physmap_add_entry(). Furthermore, the function so far also
> established r/o mappings, which is not in line with the rules set forth
> by the XSA-288 change.
>
> For arch_iommu_populate_page_table() to not encounter PGT_none pages
> anymore even in cases where the IOMMU gets enabled for a domain only
> when it is already running, replace the IOMMU dependency in
> guest_physmap_add_entry()'s handling of PV guests to check just the
> system wide state instead of the domain property.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -837,11 +837,11 @@ guest_physmap_add_entry(struct domain *d
> *
> * Retain this property by grabbing a writable type ref and then
> * dropping it immediately. The result will be pages that have a
> - * writable type (and an IOMMU entry), but a count of 0 (such that
> - * any guest-requested type changes succeed and remove the IOMMU
> - * entry).
> + * writable type (and an IOMMU entry if necessary), but a count of 0
> + * (such that any guest-requested type changes succeed and remove the
> + * IOMMU entry).
> */
> -if ( !need_iommu_pt_sync(d) || t != p2m_ram_rw )
> +if ( !iommu_enabled || t != p2m_ram_rw )
> return 0;
This looks good. One question about the next one...
>
> for ( i = 0; i < (1UL << page_order); ++i, ++page )
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -192,21 +192,27 @@ void __hwdom_init iommu_hwdom_init(struc
>
> page_list_for_each ( page, >page_list )
> {
> -unsigned long mfn = mfn_x(page_to_mfn(page));
> -unsigned long dfn = mfn_to_gmfn(d, mfn);
> -unsigned int mapping = IOMMUF_readable;
> -int ret;
> +if ( (page->u.inuse.type_info & PGT_type_mask) == PGT_none )
> +{
> +ASSERT(!(page->u.inuse.type_info & PGT_count_mask));
> +if ( get_page_and_type(page, d, PGT_writable_page) )
> +put_page_and_type(page);
> +else if ( !rc )
> +rc = -EBUSY;
> +}
>
> -if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) ||
> - ((page->u.inuse.type_info & PGT_type_mask)
> - == PGT_writable_page) )
> -mapping |= IOMMUF_writable;
> +if ( ((page->u.inuse.type_info & PGT_type_mask) ==
> + PGT_writable_page) )
> +{
> +unsigned long mfn = mfn_x(page_to_mfn(page));
> +unsigned long dfn = mfn_to_gmfn(d, mfn);
> +int ret = iommu_map(d, _dfn(dfn), _mfn(mfn), 0,
> +IOMMUF_readable | IOMMUF_writable,
> +_flags);
What's the idea behind calling iommu_map() here, rather than relying on
the one in _get_page_type()? Does need_iommu_pt_sync() not work yet at
this point, or do you expect there to be pages that have been marked
PGT_writable without having gone through _get_page_type()?
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen commit 9b0bc91b3 possibly removed too much info from README
On Tue, Apr 16, 2019 at 09:31:41PM +0800, Kevin Buckley wrote: > > Oh wait, I don't think there is anything to fix there. Those sentences > > look repetitive but they do say different things: in tools case, it says > > "repos will be cloned"; in stubdom case, it says "external packages > > will be downloaded. So they do reflect correctly what will happen. > > > > Let me narrow things down a bit further to highlight the duplication > > 8<8<8<8<8<8<8<8< > During tools build external repos will be cloned into the source tree. > <=== 1 > This variable can be used to point to a different git binary to be used. > GIT= > > During tools build external repos will be cloned into the source tree. > <=== 2 > During stubdom build external packages will be downloaded into the > source tree. These variables can be used to point to a different > locations. > XEN_EXTFILES_URL= > OVMF_UPSTREAM_URL= > 8<8<8<8<8<8<8<8< > > Hope that's clearer ? Yes it is clearer. Thanks. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/4] xen/watchdog: Rearrange the logic to fold the timer-arming paths
On 10/05/2019 19:28, Andrew Cooper wrote: > By rearranging the logic, the timer allocation loop can reuse the common timer > arming/clearing logic. This results in easier to follow code, and a modest > reduction in compiled code size (-64, 290 => 226). > > For domains which use watchdogs, the overwhemling majoriy of hypercalls will > be re-arming an existing timer. Arrange the fastpath to match. > > This does cause one change in behaviour for a corner case. Previously, > specifying id = 0, timeout = 0 would instantly kill the domain, as the timer > would fire before returning to the guest. This corner case is going to be > reused for a different purpose in a later change. Actually, it turns out that this corner case is used for deliberately fencing in some cases. I'll have to invent some different way of clearing all timers. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/watchdog: Fold exit paths to have a single unlock
On Fri, May 10, 2019 at 07:28:01PM +0100, Andrew Cooper wrote: > This is mostly to simplify future logical changes, but it does come with a > modest redunction in compiled code size (-55, 345 => 290). > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-upstream-4.12-testing test] 136047: regressions - FAIL
flight 136047 qemu-upstream-4.12-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/136047/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qcow2 17 guest-localmigrate/x10 fail in 135450 REGR. vs. 133734 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 21 leak-check/check fail in 135450 pass in 136047 test-amd64-amd64-xl-qemuu-ovmf-amd64 21 leak-check/check fail in 135450 pass in 136047 test-amd64-amd64-xl-qcow216 guest-saverestore.2fail pass in 135450 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 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-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-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-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuu8023a62081ffbe3f734019076ec1a2b4213142bb baseline version: qemuude5b678ca4dcdfa83e322491d478d66df56c1986 Last test of basis 133734 2019-03-12 07:09:17 Z 62 days Testing same since 134859 2019-04-16 10:09:05 Z 27 days9 attempts People who touched revisions under test: Alberto Garcia Alex Bennée Anthony PERARD BALATON Zoltan Bharata B Rao Christian Borntraeger Corey Minyard Cornelia Huck Daniel Henrique Barboza Daniel P. Berrangé David Gibson Denis V. Lunev Dr. David Alan Gilbert Eduardo Habkost
Re: [Xen-devel] [PATCH v2 2/2] xen: implement VCPUOP_register_runstate_phys_memory_area
On 08.05.19 18:40, Julien Grall wrote:
This patch is quite hard to read because you are reworking the code and at the
same time implementing the new VCPUOP. How about moving the rework in a
separate patch? The implementation can then be fold in the previous patch as
suggested by George.
OK.
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 6dc633e..8e24e63 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -275,32 +275,55 @@ static void ctxt_switch_to(struct vcpu *n)
}
/* Update per-VCPU guest runstate shared memory area (if registered). */
-static void update_runstate_area(struct vcpu *v)
+void update_runstate_area(struct vcpu *v)
Why do you export update_runstate_area? The function does not seem to be called
outside.
Ouch, this left from one of the previous versions.
{
- void __user *guest_handle = NULL;
+ if ( !guest_handle_is_null(runstate_guest(v)) )
+ {
+ void __user *guest_handle = NULL;
+ if ( VM_ASSIST(v->domain, runstate_update_flag) )
+ {
+ guest_handle = >runstate_guest.p->state_entry_time + 1;
+ guest_handle--;
+ v->runstate.state_entry_time |= XEN_RUNSTATE_UPDATE;
+ __raw_copy_to_guest(guest_handle,
+ (void *)(>runstate.state_entry_time + 1) -
1,
+ 1);
+ smp_wmb();
+ }
- if ( guest_handle_is_null(runstate_guest(v)) )
- return;
+ __copy_to_guest(runstate_guest(v), >runstate, 1);
- if ( VM_ASSIST(v->domain, runstate_update_flag) )
- {
- guest_handle = >runstate_guest.p->state_entry_time + 1;
- guest_handle--;
- v->runstate.state_entry_time |= XEN_RUNSTATE_UPDATE;
- __raw_copy_to_guest(guest_handle,
- (void *)(>runstate.state_entry_time + 1) - 1,
1);
- smp_wmb();
+ if ( guest_handle )
+ {
+ v->runstate.state_entry_time &= ~XEN_RUNSTATE_UPDATE;
+ smp_wmb();
+ __raw_copy_to_guest(guest_handle,
+ (void *)(>runstate.state_entry_time + 1) -
1,
+ 1);
+ }
}
- __copy_to_guest(runstate_guest(v), >runstate, 1);
-
- if ( guest_handle )
+ spin_lock(>mapped_runstate_lock);
+ if ( v->mapped_runstate )
The code looks a bit odd to me, you seem to allow a guest to provide 2 runstate
areas: one using guest virtual address the other using guest physical address.
It would be best if we prevent a guest to mix match them.
Firstly I turned to implementing in that way, but the locking and decissions
code become really ugly and complex while trying to cover 'guest's misbehavior'
scenarios.
IOW, if the guest provide a physical address first, then *all* the call should
be physical address. Alternatively this could be a per vCPU decision.
I guess we should agree what to implement first.
{
- v->runstate.state_entry_time &= ~XEN_RUNSTATE_UPDATE;
- smp_wmb();
- __raw_copy_to_guest(guest_handle,
- (void *)(>runstate.state_entry_time + 1) - 1,
1);
+ if ( VM_ASSIST(v->domain, runstate_update_flag) )
+ {
+ v->mapped_runstate->state_entry_time |= XEN_RUNSTATE_UPDATE;
+ smp_wmb();
+ v->runstate.state_entry_time |= XEN_RUNSTATE_UPDATE;
+ }
+
+ memcpy(v->mapped_runstate, >runstate, sizeof(v->runstate));
+
+ if ( VM_ASSIST(v->domain, runstate_update_flag) )
+ {
+ v->mapped_runstate->state_entry_time &= ~XEN_RUNSTATE_UPDATE;
+ smp_wmb();
+ v->runstate.state_entry_time &= ~XEN_RUNSTATE_UPDATE;
+ }
}
+ spin_unlock(>mapped_runstate_lock);
+
NIT: The newline is not necessary here.
OK.
}
static void schedule_tail(struct vcpu *prev)
@@ -998,6 +1021,7 @@ long do_arm_vcpu_op(int cmd, unsigned int vcpuid,
XEN_GUEST_HANDLE_PARAM(void) a
{
case VCPUOP_register_vcpu_info:
case VCPUOP_register_runstate_memory_area:
+ case VCPUOP_register_runstate_phys_memory_area:
return do_vcpu_op(cmd, vcpuid, arg);
default:
return -EINVAL;
[...]
diff --git a/xen/common/domain.c b/xen/common/domain.c
index ae22049..6df76c6 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -149,6 +149,7 @@ struct vcpu *vcpu_create(
v->dirty_cpu = VCPU_CPU_CLEAN;
spin_lock_init(>virq_lock);
+ spin_lock_init(>mapped_runstate_lock);
tasklet_init(>continue_hypercall_tasklet, NULL, 0);
@@ -699,6 +700,69 @@ int rcu_lock_live_remote_domain_by_id(domid_t dom, struct
domain **d)
return 0;
}
+static void _unmap_runstate_area(struct vcpu *v)
A better name would be unamep_runstate_area_locked() so you avoid the reserved
name and make clear of the use.
OK.
+{
+ mfn_t mfn;
+
+ if ( !v->mapped_runstate )
+ return;
+
+ mfn =
Re: [Xen-devel] [PATCH v2 03/12] x86/IRQ: avoid UB (or worse) in trace_irq_mask()
>>> On 13.05.19 at 12:42, wrote: > On 5/8/19 2:07 PM, Jan Beulich wrote: >> TBD: I wonder whether the function shouldn't gain an early tb_init_done >> check, like many other trace_*() have. > > Yeah, avoiding these memcopies when tracing is not enabled seems like a > good thing. I've taken note to submit a respective follow-on patch. > Either way: > > Acked-by: George Dunlap Thanks, Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 136170: tolerable all pass - PUSHED
flight 136170 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/136170/ 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 1e31c150f6b0efac59df1824e9881b3eb00b01b5 baseline version: xen e83077a3d11072708a5c38fa09fa9d011914e2a1 Last test of basis 135857 2019-05-07 14:00:34 Z5 days Testing same since 136170 2019-05-13 09:00:34 Z0 days1 attempts People who touched revisions under test: Alexandru Isaila Andrew Cooper Brian Woods Eslam Elnikety George Dunlap Igor Druzhinin Jan Beulich Kevin Tian Marek Marczykowski-Górecki Razvan Cojocaru Tamas K Lengyel 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 e83077a3d1..1e31c150f6 1e31c150f6b0efac59df1824e9881b3eb00b01b5 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 06/12] x86/IRQ: consolidate use of ->arch.cpu_mask
On Wed, May 08, 2019 at 07:10:29AM -0600, Jan Beulich wrote:
> Mixed meaning was implied so far by different pieces of code -
> disagreement was in particular about whether to expect offline CPUs'
> bits to possibly be set. Switch to a mostly consistent meaning
> (exception being high priority interrupts, which would perhaps better
> be switched to the same model as well in due course). Use the field to
> record the vector allocation mask, i.e. potentially including bits of
> offline (parked) CPUs. This implies that before passing the mask to
> certain functions (most notably cpu_mask_to_apicid()) it needs to be
> further reduced to the online subset.
>
> The exception of high priority interrupts is also why for the moment
> _bind_irq_vector() is left as is, despite looking wrong: It's used
> exclusively for IRQ0, which isn't supposed to move off CPU0 at any time.
>
> The prior lack of restricting to online CPUs in set_desc_affinity()
> before calling cpu_mask_to_apicid() in particular allowed (in x2APIC
> clustered mode) offlined CPUs to end up enabled in an IRQ's destination
> field. (I wonder whether vector_allocation_cpumask_flat() shouldn't
> follow a similar model, using cpu_present_map in favor of
> cpu_online_map.)
>
> For IO-APIC code it was definitely wrong to potentially store, as a
> fallback, TARGET_CPUS (i.e. all online ones) into the field, as that
> would have caused problems when determining on which CPUs to release
> vectors when they've gone out of use. Disable interrupts instead when
> no valid target CPU can be established (which code elsewhere should
> guarantee to never happen), and log a message in such an unlikely event.
>
> Signed-off-by: Jan Beulich
Thanks.
Reviewed-by: Roger Pau Monné
Some comments below.
> ---
> v2: New.
>
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -680,7 +680,7 @@ void /*__init*/ setup_ioapic_dest(void)
> continue;
> irq = pin_2_irq(irq_entry, ioapic, pin);
> desc = irq_to_desc(irq);
> -BUG_ON(cpumask_empty(desc->arch.cpu_mask));
> +BUG_ON(!cpumask_intersects(desc->arch.cpu_mask,
> _online_map));
I wonder if maybe you could instead do:
if ( cpumask_intersects(desc->arch.cpu_mask, _online_map) )
set_ioapic_affinity_irq(desc, desc->arch.cpu_mask);
else
ASSERT_UNREACHABLE();
I guess if the IRQ is in use by Xen itself the failure ought to be
fatal.
> set_ioapic_affinity_irq(desc, desc->arch.cpu_mask);
> }
>
> @@ -2197,7 +2197,6 @@ int io_apic_set_pci_routing (int ioapic,
> {
> struct irq_desc *desc = irq_to_desc(irq);
> struct IO_APIC_route_entry entry;
> -cpumask_t mask;
> unsigned long flags;
> int vector;
>
> @@ -2232,11 +2231,17 @@ int io_apic_set_pci_routing (int ioapic,
> return vector;
> entry.vector = vector;
>
> -cpumask_copy(, TARGET_CPUS);
> -/* Don't chance ending up with an empty mask. */
> -if (cpumask_intersects(, desc->arch.cpu_mask))
> -cpumask_and(, , desc->arch.cpu_mask);
> -SET_DEST(entry, logical, cpu_mask_to_apicid());
> +if (cpumask_intersects(desc->arch.cpu_mask, TARGET_CPUS)) {
> +cpumask_t *mask = this_cpu(scratch_cpumask);
> +
> +cpumask_and(mask, desc->arch.cpu_mask, TARGET_CPUS);
> +SET_DEST(entry, logical, cpu_mask_to_apicid(mask));
> +} else {
> +printk(XENLOG_ERR "IRQ%d: no target CPU (%*pb vs %*pb)\n",
> + irq, nr_cpu_ids, cpumask_bits(desc->arch.cpu_mask),
> + nr_cpu_ids, cpumask_bits(TARGET_CPUS));
> +desc->status |= IRQ_DISABLED;
> +}
Hm, part of this file doesn't seem to use Xen coding style, but the
chunk you add below does use it. And there are functions (like
mask_and_ack_level_ioapic_irq that seem to use a mix of coding
styles).
I'm not sure what's the policy here, should new chunks follow Xen's
coding style?
>
> apic_printk(APIC_DEBUG, KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry "
> "(%d-%d -> %#x -> IRQ %d Mode:%i Active:%i)\n", ioapic,
> @@ -2422,7 +2427,21 @@ int ioapic_guest_write(unsigned long phy
> /* Set the vector field to the real vector! */
> rte.vector = desc->arch.vector;
>
> -SET_DEST(rte, logical, cpu_mask_to_apicid(desc->arch.cpu_mask));
> +if ( cpumask_intersects(desc->arch.cpu_mask, TARGET_CPUS) )
> +{
> +cpumask_t *mask = this_cpu(scratch_cpumask);
> +
> +cpumask_and(mask, desc->arch.cpu_mask, TARGET_CPUS);
> +SET_DEST(rte, logical, cpu_mask_to_apicid(mask));
> +}
> +else
> +{
> +gprintk(XENLOG_ERR, "IRQ%d: no target CPU (%*pb vs %*pb)\n",
> + irq, nr_cpu_ids, cpumask_bits(desc->arch.cpu_mask),
> + nr_cpu_ids, cpumask_bits(TARGET_CPUS));
> +desc->status |= IRQ_DISABLED;
> +rte.mask = 1;
> +}
>
> __ioapic_write_entry(apic, pin, 0, rte);
>
> --- a/xen/arch/x86/irq.c
> +++
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
On 5/13/19 11:15 AM, Andrii Anisov wrote: Hello Julien, On 08.05.19 16:59, Julien Grall wrote: Hi, On 23/04/2019 09:10, Andrii Anisov wrote: From: Andrii Anisov Following discussion [1] it is introduced and implemented a runstate registration interface which uses guest's phys address instead of a virtual one. The new hypercall employes the same data structures as a predecessor, but expects the vcpu_runstate_info structure to not cross a page boundary. The interface is implemented in a way vcpu_runstate_info structure is mapped to the hypervisor on the hypercall processing and is directly accessed during its updates. This runstate area mapping follows vcpu_info structure registration. Permanent mapping of runstate area would consume vmap area on arm32 what is limited to 1G. Though it is assumed that ARM32 does not target the server market and the rest of possible applications will not host a huge number of VCPUs to render the limitation into the issue. I am afraid I can't possible back this assumption. As I pointed out in the previous version, I would be OK with the always map solution on Arm32 (pending performance) because it would be possible to increase the virtual address area by reworking the address space. I'm sorry, I'm not sure what should be my actions about that. There no code modification involved so far. Just updating your cover letter with what I just said above. The series is tested for ARM64. Build tested for x86. I'd appreciate if someone could check it with x86. The Linux kernel patch is here [2]. Though it is for 4.14. The patch looks wrong to me. You are using virt_to_phys() on a percpu area. What does actually promise you the physical address will always be the same? Sorry for my ignorance here, could you please elaborate more about what is wrong here? While the virtual address will never change over over the life cycle of a variable, I am not entirely sure we can make the same assumption for the physical address. I know that kmalloc() is promising you that the physical address will not change. But percpu does not seem to use kmalloc() so have you confirmed this assumption can hold? Are you saying that the command dd is the CPUBurn? I am not sure how this could be considered as a CPUBurn. IHMO, this is more IO related. Both /dev/null and /dev/zero are virtual devices no actual IO is performed during their operations, all the load is CPU (user and sys). Thank you for the explanation. Shall I guess this is an existing benchmark [1]? VCPU(dX)->idle->VCPU(dX). with following results: mapped mapped on access on init GLMark2 320x240 2852 2877 +0.8% +Dom0 CPUBurn 2088 2094 +0.2% GLMark2 800x600 2368 2375 +0.3% +Dom0 CPUBurn 1868 1921 +2.8% GLMark2 1920x1080 931 931 0% +Dom0 CPUBurn 892 894 +0.2% Please note that "mapped on access" means using the old runstate registering interface. And runstate update in this case still often fails to map runstate area like [5], despite the fact that our Linux kernel does not have KPTI enabled. So runstate area update, in this case, is really shortened. We know that the old interface is broken, so telling us the new interface is faster is not entirely useful. What I am more interested is how it if you use a guest physical address on the version "Mapped on access". Hm, I see your point. Well, I can make it for ARM to compare performance. Also it was checked IRQ latency difference using TBM in a setup similar to [5]. Please note that the IRQ rate is one in 30 seconds, and only VCPU->idle->VCPU use-case is considered. With following results (in ns, the timer granularity 120ns): How long did you run the benchmark? I did run it until avg more ore less stabilizes (2-3 minutes), then took the minimal avg (note, we have a moving average there). Did you re-run multiple time? mapped on access: max=9960 warm_max=8640 min=7200 avg=7626 mapped on init: max=9480 warm_max=8400 min=7080 avg=7341 Unfortunately there are no consitent results yet from profiling using Lauterbach PowerTrace. Still in communication with the tracer vendor in order to setup the proper configuration. [1] https://patrickmn.com/projects/cpuburn/? If so, a link to the benchmark -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.7-testing test] 136050: regressions - FAIL
flight 136050 xen-4.7-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/136050/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm6 xen-buildfail REGR. vs. 133596 build-i386-prev 6 xen-buildfail REGR. vs. 133596 build-amd64-xsm 6 xen-buildfail REGR. vs. 133596 build-amd64 6 xen-buildfail REGR. vs. 133596 build-amd64-prev 6 xen-buildfail REGR. vs. 133596 build-i3866 xen-buildfail REGR. vs. 133596 build-arm64 6 xen-buildfail REGR. vs. 133596 build-arm64-xsm 6 xen-buildfail REGR. vs. 133596 build-armhf 6 xen-buildfail REGR. vs. 133596 Tests which did not succeed, but are not blocking: test-amd64-amd64-migrupgrade 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-xtf-amd64-amd64-41 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1)blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-i386-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-xtf-amd64-amd64-51 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-xtf-amd64-amd64-11 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1)
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hi, On 5/8/19 5:01 PM, Andrii Anisov wrote: On 08.05.19 17:31, Julien Grall wrote: I haven't seen them with nokpti platform so far. I am curious to know what is your configuration here. XEN 4.12 with our patches. Thin Dom0 is a generic armv8 Linux, LK 4.14.75 with patches from Renesas and us. DomD is LK 4.14.75 with HW assigned and drivers. LK configs you can find on my google drive [1]. Those faults fire only for DomD (on its start). vcpu_show_execution_state(current) should do the job here. Here it is: (XEN) d1v2 par 0x809 (XEN) d1v2: Failed to walk page-table va 0x80002ff66357 (XEN) *** Dumping Dom1 vcpu#2 state: *** (XEN) [ Xen-4.12.0 arm64 debug=n Not tainted ] (XEN) CPU: 2 (XEN) PC: bd28dc88 (XEN) LR: bd28e674 (XEN) SP_EL0: e9890410 (XEN) SP_EL1: 0803c000 (XEN) CPSR: 4000 MODE:64-bit EL0t (Guest User) This one is happening when the guest was running in user mode. Is it always the case? Also, your DomD .config has CONFIG_UNMAP_KERNEL_AT_EL0. So how do you disable kpti? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] libxl: make vkbd tunable for HVM guests
On Tue, May 07, 2019 at 01:53:20PM +, Eslam Elnikety wrote:
> Each HVM guest currently gets a vkbd frontend/backend pair (c/s ebbd2561b4c).
> This consumes host resources unnecessarily for guests that have no use for
> vkbd. Make this behaviour tunable to allow an administrator to choose. The
> commit retains the current behaviour -- HVM guests still get vkdb unless
> specified otherwise.
>
> Signed-off-by: Eslam Elnikety
>
> ---
> Changes in v2:
> - Added a missing hunk / setting vkb_device per config
> ---
> tools/libxl/libxl_create.c | 9 ++---
> tools/libxl/libxl_types.idl | 1 +
> tools/xl/xl_parse.c | 1 +
> tools/xl/xl_sxp.c | 2 ++
> 4 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 89fe80fc9c..03ce166f4f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -310,6 +310,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(_info->u.hvm.vpt_align, true);
> libxl_defbool_setdefault(_info->u.hvm.altp2m, false);
> libxl_defbool_setdefault(_info->u.hvm.usb,false);
> +libxl_defbool_setdefault(_info->u.hvm.vkb_device, true);
> libxl_defbool_setdefault(_info->u.hvm.xen_platform_pci, true);
>
> libxl_defbool_setdefault(_info->u.hvm.spice.enable, false);
> @@ -1416,9 +1417,11 @@ static void domcreate_launch_dm(libxl__egc *egc,
> libxl__multidev *multidev,
> libxl__device_console_add(gc, domid, , state, );
> libxl__device_console_dispose();
>
> -libxl_device_vkb_init();
> -libxl__device_add(gc, domid, __vkb_devtype, );
> -libxl_device_vkb_dispose();
> +if (libxl_defbool_val(d_config->b_info.u.hvm.vkb_device)) {
> +libxl_device_vkb_init();
> +libxl__device_add(gc, domid, __vkb_devtype, );
> +libxl_device_vkb_dispose();
> +}
>
> dcs->sdss.dm.guest_domid = domid;
> if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index b685ac47ac..9a0b92f1d4 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -583,6 +583,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> # - "tablet" for absolute mouse,
> # - "mouse" for PS/2 protocol
> relative mouse
> ("usbdevice",string),
> + ("vkb_device", libxl_defbool),
> ("soundhw", string),
> ("xen_platform_pci", libxl_defbool),
> ("usbdevice_list",
> libxl_string_list),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 352cd214dd..e105bda2bb 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2652,6 +2652,7 @@ skip_usbdev:
> fprintf(stderr,"xl: Unable to parse usbdevice.\n");
> exit(-ERROR_FAIL);
> }
> +xlu_cfg_get_defbool(config, "vkb_device", _info->u.hvm.vkb_device,
> 0);
> xlu_cfg_replace_string (config, "soundhw", _info->u.hvm.soundhw,
> 0);
> xlu_cfg_get_defbool(config, "xen_platform_pci",
> _info->u.hvm.xen_platform_pci, 0);
Oh here it is the code which uses the new field -- in that case, you
also need to document this in xl manpage.
The manpage is docs/man/xl.conf.pod.5.
Sorry for not having mentioned this earlier: we also ask for an
accompanying macro for the new field in the public interface. See
various LIBXL_HAVE macros in libxl.h.
Let me know if you have further questions.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
On Mon, May 13, 2019 at 01:29:12PM +0300, Viktor Mitin wrote: > > > aarch64-poky-linux-gcc -DBUILD_ID -fno-strict-aliasing -std=gnu99 > > > -Wall -Wstrict-prototypes -Wdeclaration-after-statement > > > -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2 > > > -fomit-frame-pointer > > > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF > > > .handlereg.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror > > > -Wmissing-prototypes -I./include > > > -I/home/c/w/rcar_h3_ubuntu1604_yocto/build/tmp/work/aarch64-poky-linux/xen/4.12.0.0+gitAUTOINC+fd2a34c965-r0/git/tools/libs/toolcore/../../../tools/include > > > -c -o handlereg.o handlereg.c > > > > ... this looks like a tool building error when I only touch the > > hypervisor part. Are you certain this is my patch and not another error > > in Xen 4.12 (or any patch you have on top)? > > Julien, you are right, it was local environment build issue (sorry for that). > Xen GCC coverage feature works well with Aarch64 with this patch. > Checked both commands, xencov read and xencov reset - both work well > (no crashes anymore). > > Please also note that use case mentioned in Xen documentation > (xencov_split) is also ok with generated coverage.dat input: > xen.git/xen$ ssh root@host xencov read > coverage.dat > xen.git/xen$ ../tools/xencov_split coverage.dat --output-dir=/ > xen.git/xen$ geninfo . -o cov.info > xen.git/xen$ genhtml cov.info -o cov/ > xen.git/xen$ $BROWSER cov/index.html > > > Minor observation about coverage build procedure. Documentation states: > "To build with coverage support, enable CONFIG_COVERAGE in Kconfig." > However, to build it properly, it needs to enable coverage feature in > two places - main xen make command line and hypervisor .config file. > Is it expected way to build xen with coverage feature? If yes, > probably we should improve (or at least document) it some day... What does your make command line look like? Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/12] x86/IRQ: avoid UB (or worse) in trace_irq_mask()
On 5/8/19 2:07 PM, Jan Beulich wrote: > Dynamically allocated CPU mask objects may be smaller than cpumask_t, so > copying has to be restricted to the actual allocation size. This is > particulary important since the function doesn't bail early when tracing > is not active, so even production builds would be affected by potential > misbehavior here. > > Take the opportunity and also > - use initializers instead of assignment + memset(), > - constify the cpumask_t input pointer, > - u32 -> uint32_t. > > Signed-off-by: Jan Beulich > --- > v2: New. > --- > TBD: I wonder whether the function shouldn't gain an early tb_init_done > check, like many other trace_*() have. Yeah, avoiding these memcopies when tracing is not enabled seems like a good thing. Either way: Acked-by: George Dunlap > > George, despite your general request to be copied on entire series > rather than individual patches, I thought it would be better to copy > you on just this one (for its tracing aspect), as the patch here is > independent of the rest of the series, but at least one later patch > depends on the parameter constification done here. Yes, I think in this case this was the easiest thing for me. Thanks. :-) -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mm: free_page_type() is PV-only
>>> On 13.05.19 at 12:33, wrote: > On 5/10/19 3:12 PM, Jan Beulich wrote: >> @@ -2640,11 +2639,11 @@ int free_page_type(struct page_info *pag >> /* A page table is dirtied when its type count becomes zero. */ >> paging_mark_dirty(owner, page_to_mfn(page)); >> >> -ASSERT(!shadow_mode_refcounts(owner)); >> +ASSERT(shadow_mode_enabled(owner)); >> +ASSERT(!paging_mode_refcounts(owner)); >> +ASSERT(!paging_mode_translate(owner)); > > In the context of my patch to CODING_STYLE about the use of ASSERTs, > thinking about ASSERT vs BUG_ON vs something else here. I guess in this > case: > > 1. PV guests can't be in translate mode > 2. If that ever changed, we'd probably trip over the ASSERT() while > debugging > > So I guess ASSERT() is probably fine. Right, in other (more likely to be [wrongly] exposed to actual execution) cases I'd probably not have used plain ASSERT() here. > Reviewed-by: George Dunlap Thanks. > So does that mean, though, that SHARED_M2P_ENTRY & friends are entirely > vestigal now, and can be removed? No, it's pointless to use here only because there's no M2P translation done here in the first place, due to the code being PV only. In code paths reachable for HVM these ought to still be necessary. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
On 5/13/19 11:29 AM, Viktor Mitin wrote: aarch64-poky-linux-gcc -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .handlereg.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror -Wmissing-prototypes -I./include -I/home/c/w/rcar_h3_ubuntu1604_yocto/build/tmp/work/aarch64-poky-linux/xen/4.12.0.0+gitAUTOINC+fd2a34c965-r0/git/tools/libs/toolcore/../../../tools/include -c -o handlereg.o handlereg.c ... this looks like a tool building error when I only touch the hypervisor part. Are you certain this is my patch and not another error in Xen 4.12 (or any patch you have on top)? Julien, you are right, it was local environment build issue (sorry for that). Xen GCC coverage feature works well with Aarch64 with this patch. Checked both commands, xencov read and xencov reset - both work well (no crashes anymore). Please also note that use case mentioned in Xen documentation (xencov_split) is also ok with generated coverage.dat input: xen.git/xen$ ssh root@host xencov read > coverage.dat xen.git/xen$ ../tools/xencov_split coverage.dat --output-dir=/ xen.git/xen$ geninfo . -o cov.info xen.git/xen$ genhtml cov.info -o cov/ xen.git/xen$ $BROWSER cov/index.html Minor observation about coverage build procedure. Documentation states: "To build with coverage support, enable CONFIG_COVERAGE in Kconfig." However, to build it properly, it needs to enable coverage feature in two places - main xen make command line and hypervisor .config file. Is it expected way to build xen with coverage feature? If yes, probably we should improve (or at least document) it some day... What is require on the make command line? As usual, patches are welcomed ;). Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mm: free_page_type() is PV-only
On 5/10/19 3:12 PM, Jan Beulich wrote:
> While it already has a CONFIG_PV wrapped around its entire body, it is
> still uselessly invoking mfn_to_gmfn(), which is about to be replaced.
> Avoid morphing this code into even more suspicious shape and remove the
> effectively dead code - translated mode has been made impossible for PV
> quite some time ago.
>
> Adjust and extend the assertions at the same time: The original
> ASSERT(!shadow_mode_refcounts(owner)) really means
> ASSERT(!shadow_mode_enabled(owner) || !paging_mode_refcounts(owner)),
> which isn't what we want here.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2632,7 +2632,6 @@ int free_page_type(struct page_info *pag
> {
> #ifdef CONFIG_PV
> struct domain *owner = page_get_owner(page);
> -unsigned long gmfn;
> int rc;
>
> if ( likely(owner != NULL) && unlikely(paging_mode_enabled(owner)) )
> @@ -2640,11 +2639,11 @@ int free_page_type(struct page_info *pag
> /* A page table is dirtied when its type count becomes zero. */
> paging_mark_dirty(owner, page_to_mfn(page));
>
> -ASSERT(!shadow_mode_refcounts(owner));
> +ASSERT(shadow_mode_enabled(owner));
> +ASSERT(!paging_mode_refcounts(owner));
> +ASSERT(!paging_mode_translate(owner));
In the context of my patch to CODING_STYLE about the use of ASSERTs,
thinking about ASSERT vs BUG_ON vs something else here. I guess in this
case:
1. PV guests can't be in translate mode
2. If that ever changed, we'd probably trip over the ASSERT() while
debugging
So I guess ASSERT() is probably fine.
Reviewed-by: George Dunlap
So does that mean, though, that SHARED_M2P_ENTRY & friends are entirely
vestigal now, and can be removed?
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen GCC coverage ARM64 testing - Unexpected Trap: Data Abort
> > aarch64-poky-linux-gcc -DBUILD_ID -fno-strict-aliasing -std=gnu99 > > -Wall -Wstrict-prototypes -Wdeclaration-after-statement > > -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2 > > -fomit-frame-pointer > > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF > > .handlereg.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror > > -Wmissing-prototypes -I./include > > -I/home/c/w/rcar_h3_ubuntu1604_yocto/build/tmp/work/aarch64-poky-linux/xen/4.12.0.0+gitAUTOINC+fd2a34c965-r0/git/tools/libs/toolcore/../../../tools/include > > -c -o handlereg.o handlereg.c > > ... this looks like a tool building error when I only touch the > hypervisor part. Are you certain this is my patch and not another error > in Xen 4.12 (or any patch you have on top)? Julien, you are right, it was local environment build issue (sorry for that). Xen GCC coverage feature works well with Aarch64 with this patch. Checked both commands, xencov read and xencov reset - both work well (no crashes anymore). Please also note that use case mentioned in Xen documentation (xencov_split) is also ok with generated coverage.dat input: xen.git/xen$ ssh root@host xencov read > coverage.dat xen.git/xen$ ../tools/xencov_split coverage.dat --output-dir=/ xen.git/xen$ geninfo . -o cov.info xen.git/xen$ genhtml cov.info -o cov/ xen.git/xen$ $BROWSER cov/index.html Minor observation about coverage build procedure. Documentation states: "To build with coverage support, enable CONFIG_COVERAGE in Kconfig." However, to build it properly, it needs to enable coverage feature in two places - main xen make command line and hypervisor .config file. Is it expected way to build xen with coverage feature? If yes, probably we should improve (or at least document) it some day... Thanks ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] pygrub not starting first menuentry in Fedora 30
There seems to be some changes in Fedora 30 that cause the second boot entry in grub.cfg to be booted instead of the first. This means that Fedora 30 systems either always boot into an older kernel, or in the case of systems with only one kernel installed, the rescue image. There also seems to be some new issues with the move to BLSCFG - however it seems a new requirement is to have GRUB_ENABLE_BLSCFG="false" in /etc/default/grub. This causes grub2-mkconfig to work correctly and spit out a grub.cfg file that pygrub can then use. Is this a bug in pygrub, or a problem with how Fedora 30 generates a grub.cfg? I tried to pick through pygrub - but couldn't quite follow the python logic to see where the default boot option is selected. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [linux-4.9 test] 136013: regressions - FAIL
osstest service owner writes ("[linux-4.9 test] 136013: regressions - FAIL"):
> flight 136013 linux-4.9 real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/136013/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs.
> 134015
This is the known ThunderX bootloader missing output issue.
I have force opushed this.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] Introduce runstate area registration with phys address
Hello Julien, On 08.05.19 16:59, Julien Grall wrote: Hi, On 23/04/2019 09:10, Andrii Anisov wrote: From: Andrii Anisov Following discussion [1] it is introduced and implemented a runstate registration interface which uses guest's phys address instead of a virtual one. The new hypercall employes the same data structures as a predecessor, but expects the vcpu_runstate_info structure to not cross a page boundary. The interface is implemented in a way vcpu_runstate_info structure is mapped to the hypervisor on the hypercall processing and is directly accessed during its updates. This runstate area mapping follows vcpu_info structure registration. Permanent mapping of runstate area would consume vmap area on arm32 what is limited to 1G. Though it is assumed that ARM32 does not target the server market and the rest of possible applications will not host a huge number of VCPUs to render the limitation into the issue. I am afraid I can't possible back this assumption. As I pointed out in the previous version, I would be OK with the always map solution on Arm32 (pending performance) because it would be possible to increase the virtual address area by reworking the address space. I'm sorry, I'm not sure what should be my actions about that. The series is tested for ARM64. Build tested for x86. I'd appreciate if someone could check it with x86. The Linux kernel patch is here [2]. Though it is for 4.14. The patch looks wrong to me. You are using virt_to_phys() on a percpu area. What does actually promise you the physical address will always be the same? Sorry for my ignorance here, could you please elaborate more about what is wrong here? Are you saying that the command dd is the CPUBurn? I am not sure how this could be considered as a CPUBurn. IHMO, this is more IO related. Both /dev/null and /dev/zero are virtual devices no actual IO is performed during their operations, all the load is CPU (user and sys). VCPU(dX)->idle->VCPU(dX). with following results: mapped mapped on access on init GLMark2 320x240 2852 2877 +0.8% +Dom0 CPUBurn 2088 2094 +0.2% GLMark2 800x600 2368 2375 +0.3% +Dom0 CPUBurn 1868 1921 +2.8% GLMark2 1920x1080 931 931 0% +Dom0 CPUBurn 892 894 +0.2% Please note that "mapped on access" means using the old runstate registering interface. And runstate update in this case still often fails to map runstate area like [5], despite the fact that our Linux kernel does not have KPTI enabled. So runstate area update, in this case, is really shortened. We know that the old interface is broken, so telling us the new interface is faster is not entirely useful. What I am more interested is how it if you use a guest physical address on the version "Mapped on access". Hm, I see your point. Well, I can make it for ARM to compare performance. Also it was checked IRQ latency difference using TBM in a setup similar to [5]. Please note that the IRQ rate is one in 30 seconds, and only VCPU->idle->VCPU use-case is considered. With following results (in ns, the timer granularity 120ns): How long did you run the benchmark? I did run it until avg more ore less stabilizes (2-3 minutes), then took the minimal avg (note, we have a moving average there). mapped on access: max=9960 warm_max=8640 min=7200 avg=7626 mapped on init: max=9480 warm_max=8400 min=7080 avg=7341 Unfortunately there are no consitent results yet from profiling using Lauterbach PowerTrace. Still in communication with the tracer vendor in order to setup the proper configuration. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/4] libxl: add libxl_get_parameters() function
On Thu, May 09, 2019 at 05:41:27PM +0200, Vasilis Liaskovitis wrote:
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ec71574e99..124033e5a3 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -669,6 +669,21 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
> return 0;
> }
>
> +int libxl_get_parameters(libxl_ctx *ctx, char *params, char *values)
> +{
> +int ret;
> +GC_INIT(ctx);
> +
> +ret = xc_get_parameters(ctx->xch, params, values);
Please name the variable `r' instead of 'ret'. See CODING_STYLE as for
why.
> +if (ret < 0) {
> +LOGEV(ERROR, ret, "getting parameters");
LOGEV takes `errno' as second parameter, the value of `ret' seems to be
-1 or 0, and xc_get_parameters should set `errno'. So, using the macro
LOGE() should be enough.
> +GC_FREE;
> +return ret;//ERROR_FAIL;
Almost! I think ERROR_FAIL should be returned here, not a return value
from a libxc call.
> +}
> +GC_FREE;
> +return 0;
Instead of having to write GC_FREE twice, you could:
if (r < 0) {
LOG...
rc = ERROR_FAIL;
goto out;
}
rc=0;
out:
GC_FREE;
return rc;
> +}
> +
Thanks,
--
Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] xen/arm: fix mask calculation in pdx_init_mask
Hi Stefano,
On 5/8/19 11:47 PM, Stefano Stabellini wrote:
The mask calculation in pdx_init_mask is wrong when the first bank
starts at address 0x0. The reason is that pdx_init_mask will do '0 - 1'
causing an underflow. As a result, the mask becomes 0x
which is the biggest possible mask and ends up causing a significant
memory waste in the frametable size computation.
For instance, on platforms that have a low memory bank starting at 0x0
and a high memory bank, the frametable will end up covering all the
holes in between.
The purpose of the mask is to be passed as a parameter to
pfn_pdx_hole_setup, which based on the mask parameter calculates
pfn_pdx_hole_shift, pfn_pdx_bottom_mask, etc. which are actually the
important masks for frametable initialization later on.
pfn_pdx_hole_setup never compresses addresses below MAX_ORDER bits (1GB
on ARM). Thus, it is safe to initialize mask passing 1ULL << (MAX_ORDER
+ PAGE_SHIFT) as start address to pdx_init_mask.
Signed-off-by: Stefano Stabellini
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
CC: george.dun...@eu.citrix.com
CC: ian.jack...@eu.citrix.com
CC: konrad.w...@oracle.com
CC: t...@xen.org
CC: wei.l...@citrix.com
---
Changes in v2:
- update commit message
- add in-code comments regarding update sites
- improve in-code comments
- move the mask initialization changes to pdx_init_mask
---
xen/arch/arm/setup.c | 9 -
xen/common/pdx.c | 8 +++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ccb0f181ea..afaafe7b84 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -482,7 +482,14 @@ static void __init init_pdx(void)
{
paddr_t bank_start, bank_size, bank_end;
-u64 mask = pdx_init_mask(bootinfo.mem.bank[0].start);
+/*
+ * Pass 0x0 to pdx_init_mask to get a mask initialized with the
+ * first to 1<
What matter is Arm doesn't have any specific restriction on the
compression, but the common code may have. So how about something:
"Arm does not have any restriction on the bits to compress. Pass 0 to
let the common code to further restrict".
+ *
+ * If the logic changes in pfn_pdx_hole_setup we might have to
+ * update this function too.
+ */ > +u64 mask = pdx_init_mask(0x0);
int bank;
for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
diff --git a/xen/common/pdx.c b/xen/common/pdx.c
index bb7e437049..268d6f7ec3 100644
--- a/xen/common/pdx.c
+++ b/xen/common/pdx.c
@@ -50,9 +50,13 @@ static u64 __init fill_mask(u64 mask)
return mask;
}
+/*
+ * We always map the first 1<
I thought I already pointed out in the previous version. At least on
Arm, we never map the first 1 << MAX_ORDER of RAM. Instead what you want
to say is that we don't compress the first N bits of the address.
+ * are left uncompressed.
+ */
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
