[Xen-devel] [linux-3.14 test] 95164: tolerable FAIL - PUSHED
flight 95164 linux-3.14 real [real] http://logs.test-lab.xenproject.org/osstest/logs/95164/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail REGR. vs. 94568 build-i386-rumpuserxen6 xen-buildfail like 94568 build-amd64-rumpuserxen 6 xen-buildfail like 94568 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 94568 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 94568 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94568 Tests which did not succeed, but are not blocking: test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass version targeted for testing: linuxf06cb456a442c7df95a4ba6e2f3a341cf925d7cf baseline version: linuxc977650a67e6ca6c3cff9548b031d072d00db80a Last test of basis94568 2016-05-19 01:45:01 Z 14 days Testing same since95164 2016-06-01 19:46:34 Z0 days1 attempts People who touched revisions under test: Adrian HunterCatalin Vasile Chanwoo Choi Chen Yu Christoffer Dall David Sterba Greg Kroah-Hartman H. Nikolaus Schaller Hans-Christoph Schemmel Herbert Xu Jiri Slaby Johan Hovold Josef Bacik Krzysztof Kozlowski Lee Jones Lukas Wunner Lv Zheng Marc Zyngier Marcel Holtmann Matt Gumbel Rafael J. Wysocki Roger Quadros Sachin Prabhu Schemmel Hans-Christoph Stefan Metzmacher Steve French Steve French Steven Rostedt (Red Hat) Steven Rostedt Ulf Hansson 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 build-amd64-rumpuserxen fail build-i386-rumpuserxen fail test-amd64-amd64-xl pass test-amd64-i386-xl pass test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm pass test-amd64-amd64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-xl-xsm pass
[Xen-devel] [qemu-mainline test] 95144: regressions - FAIL
flight 95144 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/95144/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail REGR. vs. 94856 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail REGR. vs. 94856 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail in 95121 REGR. vs. 94856 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 6 xen-boot fail in 95089 pass in 95144 test-amd64-amd64-xl-rtds 6 xen-boot fail in 95089 pass in 95144 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 13 guest-localmigrate fail in 95121 pass in 95144 test-amd64-i386-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail pass in 95089 test-amd64-amd64-xl-credit2 11 guest-start fail pass in 95121 test-amd64-amd64-i386-pvgrub 9 debian-di-install fail pass in 95121 test-amd64-amd64-xl-qemuu-winxpsp3 13 guest-localmigratefail pass in 95121 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail in 95121 blocked in 94856 test-amd64-amd64-xl-rtds 9 debian-install fail like 94856 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: qemuu500acc9c410bcea17148a1072e323c08d12e6a6b baseline version: qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe Last test of basis94856 2016-05-27 20:14:49 Z5 days Failing since 94983 2016-05-31 09:40:12 Z1 days5 attempts Testing same since94994 2016-05-31 15:42:55 Z1 days4 attempts People who touched revisions under test: Anthony PERARDBenjamin Herrenschmidt Bharata B Rao Chen Fan Cédric Le Goater
[Xen-devel] [qemu-upstream-4.3-testing test] 95166: trouble: blocked/broken
flight 95166 qemu-upstream-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/95166/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 3 host-install(3) broken REGR. vs. 80927 build-i386-pvops 3 host-install(3) broken REGR. vs. 80927 build-i3863 host-install(3) broken REGR. vs. 80927 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927 Tests which did not succeed, but are not blocking: build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-pv1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pv 1 build-check(1) blocked n/a version targeted for testing: qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1 baseline version: qemuu10c1b763c26feb645627a1639e722515f3e1e876 Last test of basis80927 2016-02-06 13:30:02 Z 116 days Testing same since93977 2016-05-10 11:09:16 Z 22 days 97 attempts People who touched revisions under test: Gerd HoffmannStefano Stabellini jobs: build-amd64 broken build-i386 broken build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopsbroken build-i386-pvops broken test-amd64-amd64-xl blocked test-amd64-i386-xl blocked test-amd64-i386-qemuu-rhel6hvm-amd blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked test-amd64-i386-freebsd10-amd64 blocked test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked test-amd64-amd64-xl-qemuu-win7-amd64 blocked test-amd64-i386-xl-qemuu-win7-amd64 blocked test-amd64-amd64-xl-credit2 blocked test-amd64-i386-freebsd10-i386 blocked test-amd64-i386-qemuu-rhel6hvm-intel blocked test-amd64-amd64-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-xl-multivcpublocked
Re: [Xen-devel] [Patch v6 07/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending (top level ones)
On June 01, 2016 6:39 PM, Jan Beulichwrote: > >>> On 31.05.16 at 15:57, wrote: > > static int device_power_down(void) > > { > > -console_suspend(); > > +if ( console_suspend() ) > > +return SAVED_NONE; > > > > -time_suspend(); > > +if ( time_suspend() ) > > +return SAVED_CONSOLE; > > > > -i8259A_suspend(); > > +if ( i8259A_suspend() ) > > +return SAVED_TIME; > > > > +/* ioapic_suspend cannot fail */ > > ioapic_suspend(); > > > > -iommu_suspend(); > > +if ( iommu_suspend() ) > > +return SAVED_IOAPIC; > > > > -lapic_suspend(); > > +if ( lapic_suspend() ) > > +return SAVED_IOMMU; > > > > -return 0; > > +return SAVED_NONE; > > SAVED_ALL Agreed. I was disturbed by the below 'if ( error > 0 )'. > > > @@ -169,6 +203,10 @@ static int enter_state(u32 state) > > { > > printk(XENLOG_ERR "Some devices failed to power down."); > > system_state = SYS_STATE_resume; > > + > > +if ( error > 0 ) > > +device_power_up(error); > > if ( error != SAVED_NONE ) > > (Or really you could just call this without any if().) I prefer to drop this if(). > > > @@ -2389,16 +2393,25 @@ static int intel_iommu_group_id(u16 seg, u8 > > bus, u8 devfn) } > > > > static u32 iommu_state[MAX_IOMMUS][MAX_IOMMU_REGS]; > > -static void vtd_suspend(void) > > + > > +static int __must_check vtd_suspend(void) > > { > > struct acpi_drhd_unit *drhd; > > struct iommu *iommu; > > u32i; > > +int rc = 0; > > Pointless initializer. > Indeed, if "return 0" to make obvious that no error path comes at the end of this function. > > if ( !iommu_enabled ) > > -return; > > +return 0; > > > > -iommu_flush_all(); > > +rc = iommu_flush_all(); > > +if ( unlikely(rc) ) > > +{ > > +printk(XENLOG_WARNING VTDPREFIX > > + " suspend: IOMMU flush all failed: %d\n", rc); > > + > > +return rc; > > +} > > > > for_each_drhd_unit ( drhd ) > > { > > @@ -2427,6 +2440,8 @@ static void vtd_suspend(void) > > if ( !iommu_intremap && iommu_qinval ) > > disable_qinval(iommu); > > } > > + > > +return rc; > > } > > Perhaps better "return 0" to make obvious that no error path comes here. > Agreed. Quan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch v6 09/11] vt-d: fix the IOMMU flush issue
On June 01, 2016 11:37 PM, Jan Beulichwrote: > >>> On 31.05.16 at 15:57, wrote: > > @@ -1542,14 +1600,36 @@ int domain_context_unmap_one( > > return -EINVAL; > > } > > > > -if ( iommu_flush_context_device(iommu, iommu_domid, > > -(((u16)bus) << 8) | devfn, > > -DMA_CCMD_MASK_NOBIT, 0) ) > > -iommu_flush_write_buffer(iommu); > > -else > > +rc = iommu_flush_context_device(iommu, iommu_domid, > > +PCI_BDF2(bus, devfn), > > +DMA_CCMD_MASK_NOBIT, 0); > > + > > +/* > > + * The current logic for rc returns: > > + * - positive invoke iommu_flush_write_buffer to flush cache. > > + * - zero on success. > > + * - negative on failure. Continue to flush IOMMU IOTLB on a > > + * best effort basis. > > + * > > + * Moreover, IOMMU flush handlers flush_context_qi or flush_iotlb_qi > > + * (or flush_context_reg and flush_iotlb_reg, deep functions in the > > + * call trees of iommu_flush_context_device and iommu_flush_iotlb_dsi) > > + * are with the same logic to bubble up positive return value. > > + */ > > This is the 3rd instance of that comment. I'd prefer the latter ones to simply > refer to the first one, but I'll obviously leave it to the maintainers to > decide. > Kevin / Feng .. What's your opinion? > With those cosmetic issues taken care of > Reviewed-by: Jen Beulich > -Quan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 6/1/2016 6:35 PM, Julien Grall wrote: Hello Aaron, On 01/06/2016 20:54, Aaron Cornelius wrote: I'm not 100% sure, from the "VMID pool exhausted" message it would appear that the p2m_init() function failed to allocate a VM ID, which caused domain creation to fail, and the NULL pointer dereference when trying to clean up the not-fully-created domain. However, since I only have 1 domain active at a time, I'm not sure why I should run out of VM IDs. arch_domain_destroy (and p2m_teardown) is only called when all the reference on the given domain are released. It may take a while to release all the resources. So if you launch the domain as the same time as you destroy the previous guest. You will have more than 1 domain active. Can you detail how you create/destroy guest? This is with a custom application, we use the libxl APIs to interact with Xen. Domains are created using the libxl_domain_create_new() function, and domains are destroyed using the libxl_domain_destroy() function. The test in this case creates a domain, waits a minute, then deletes/creates the next domain, waits a minute, and so on. So I wouldn't be surprised to see the VMID occasionally indicate there are 2 active domains since there could be one being created and one being destroyed in a very short time. However, I wouldn't expect to ever have 256 domains. The CubieTruck only has 2GB of RAM, I allocate 512MB for dom0 which means that only 48 of the the Mirage domains (with 32MB of RAM) would work at the same time anyway. Which doesn't account for the various inter-domain resources or the RAM used by Xen itself. If the p2m_teardown() function checked for NULL it would prevent the crash, but I suspect Xen would be just as broken since all of my resources have leaked away. More broken in fact, since if the board reboots at least the applications will restart and domains can be recreated. It certainly appears that some resources are leaking when domains are deleted (possibly only on the ARM or ARM32 platforms). We will try to add some debug prints and see if we can discover exactly what is going on. - Aaron Cornelius ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Unable to boot Xen 4.7-rc4 on HiKey
On Wed, Jun 1, 2016 at 3:49 PM, Julien Grallwrote: > On 01/06/2016 18:10, Tamas K Lengyel wrote: >> >> Hi all, > > > Hi Tamas, > >> following the steps from http://wiki.xen.org/wiki/HiKey I'm unable to >> get the board to boot Xen. I'm using the Debian reference image from >> linaro as base >> (https://builds.96boards.org/releases/hikey/linaro/debian/latest/) >> and that boots fine both from the SD card directly and through >> startup.nsh. However, when I try to boot Xen I see only the following: >> >> Xen 4.7.0-rc (c/s Mon May 23 12:07:20 2016 +0100 git:8478c94-dirty) EFI >> loader >> Using configuration file 'xen.cfg' >> Image: 0x7a121000-0x7acb8a70 >> >> After that no output and dom0 never comes online on the network >> either. I tried compiling Xen on the device itself in case it was a >> problem with my cross-compile setup but no difference. Also with and >> without debug=y. Any tips on what might be going wrong here? > > > I gave a quick look at the upstream device-tree of the hikey, the path > /smb/uart@f7113000. > > If you use the upstream device-tree, it should be able to find the correct > UART via "stdout-path" and therefore dtuart=... should not be necessary. > > Let me know if it works for you, and I will update the wiki page. No, I removed the dtuart=... part from xen.cfg but there is no difference in the output and dom0 never comes up on the network either. Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for 4.7] xen: Rename of xSplice to livepatch.
On Thu, Jun 02, 2016 at 01:32:33AM +0100, Andrew Cooper wrote: > On 02/06/2016 01:14, Konrad Rzeszutek Wilk wrote: > > @@ -182,7 +182,7 @@ tools/misc/xen_cpuperf > > tools/misc/xen-cpuid > > tools/misc/xen-detect > > tools/misc/xen-tmem-list-parse > > -tools/misc/xen-xsplice > > +tools/misc/xen-livepatch > > tools/misc/xenperf > > tools/misc/xenpm > > tools/misc/xen-hvmctx > > Hate to be a pedant, but this could do with being `sort`ed. > But please don't do that in this series. We can "sort" this out later. > > @@ -247,9 +247,9 @@ xen/arch/x86/efi/check.efi > > xen/arch/x86/efi/disabled > > xen/arch/x86/efi/mkreloc > > xen/arch/x86/test/config.h > > -xen/arch/x86/test/xen_hello_world.xsplice > > -xen/arch/x86/test/xen_bye_world.xsplice > > -xen/arch/x86/test/xen_replace_world.xsplice > > +xen/arch/x86/test/xen_hello_world.xlivepatch > > +xen/arch/x86/test/xen_bye_world.xlivepatch > > +xen/arch/x86/test/xen_replace_world.xlivepatch > > xen/arch/*/efi/boot.c > > xen/arch/*/efi/compat.c > > xen/arch/*/efi/efi.h > > And to throw a spanner in the works, a file extension of .xpatch would > be a more natural fit here. (Then again, the contents of the file is > all .livepatch.$FOO, so perhaps consistency is the more important > attribute.) > +1 for consistency. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Intermittent xenvif_disconnect() hang on domU destroy
I'm seeing the xenwatch kernel thread hang intermittently when destroying a domU on recent stable xen 4.5, with Linux 4.4.11 + grsec dom0. The domU is created with a virtual network interface connected to a physical interface (ixgbevf) via an openvswitch virtual switch. Everything works fine until the domain is destroyed. Once in a while, a few seconds after the domain goes away, xenwatch hangs in xenvif_disconnect(), calling kthread_stop() on a dealloc task. I added a warning to xenvif_dealloc_kthread_should_stop() when kthread_should_stop() is true and queue->inflight_packets > 0, printing inflight_packets as well as stats.tx_zerocopy_*. Each time the hang occurs, inflight_packets == 1 and tx_zerocopy_sent == tx_zerocopy_success + tx_zerocopy_fail + 1. I also added a warning to xenvif_skb_zerocopy_complete() when queue->task is null. If I manually bring down the physical interface to which the vif was connected (ifconfig down), this somehow causes the last in-flight packet to be transmitted, and everything is unblocked. The following shows xenwatch hung trying to stop vif44.0-q0-dealloc, waking up again after I bring down the physical interface net0_52. [xl destroy] ... [ 2914.510070] net vif44.0: stopping vif44.0-q0-dealloc task (pid 28045) [ 2914.510224] xen_netback:xenvif_dealloc_kthread_should_stop: vif44.0-q0-dealloc task (pid 28045) should_stop=1 inflight_packets=1 tx_zerocopy_sent=209494 tx_zerocopy_success=209492 tx_zerocopy_fail=1 ... [ 2933.561404] device net0_52 left promiscuous mode [ 2933.564813] device vif44.0 left promiscuous mode [ 3136.324009] INFO: task xenwatch:29 blocked for more than 120 seconds. [ 3136.324119] Not tainted 4.4.11-grsec-skyport #66 [ 3136.324181] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 3136.324284] xenwatchD c90005d0baf8 1264029 2 0x [ 3136.324411] c90005d0baf8 81b70d60 8804ec08c500 [ 3136.324536] 8804ec08dbb8 c900115abec0 c900115abeb8 8804ec08c500 [ 3136.324662] 0005 c90005d0bb10 816d6ff7 7fff [ 3136.324806] Call Trace: [ 3136.324866] [] schedule+0x37/0x80 [ 3136.324932] [] schedule_timeout+0x1bc/0x240 [ 3136.325004] [] ? xen_clocksource_read+0x1c/0x30 [ 3136.325078] [] ? sched_clock+0x13/0x20 [ 3136.325153] [] ? local_clock+0x1c/0x20 [ 3136.325228] [] ? mark_held_locks+0x79/0xa0 [ 3136.325298] [] ? _raw_spin_unlock_irq+0x27/0x50 [ 3136.325367] [] ? trace_hardirqs_on_caller+0x13d/0x1d0 [ 3136.325441] [] wait_for_completion+0xd6/0x110 [ 3136.325514] [] ? wake_up_q+0x70/0x70 [ 3136.325585] [] kthread_stop+0x47/0x80 [ 3136.325660] [] xenvif_disconnect+0xb1/0x130 [ 3136.325729] [] set_backend_state+0x116/0xde0 [ 3136.325805] [] ? xenbus_gather+0x10e/0x140 [ 3136.325881] [] ? kfree+0x1c2/0x1e0 [ 3136.325960] [] ? local_clock+0x1c/0x20 [ 3136.326026] [] frontend_changed+0xb7/0xc0 [ 3136.326095] [] xenbus_otherend_changed+0x80/0x90 [ 3136.330341] [] ? unregister_xenbus_watch+0x260/0x260 [ 3136.330414] [] frontend_changed+0xb/0x10 [ 3136.330483] [] xenwatch_thread+0x3a/0x130 [ 3136.330553] [] ? wake_up_atomic_t+0x30/0x30 [ 3136.330621] [] kthread+0xfc/0x120 [ 3136.330686] [] ? kthread_create_on_node+0x240/0x240 [ 3136.330775] [] ret_from_fork+0x3e/0x70 [ 3136.330840] [] ? kthread_create_on_node+0x240/0x240 [ 3136.330911] 1 lock held by xenwatch/29: [ 3136.330972] #0: (xenwatch_mutex){+.+.+.}, at: [] 814374a7 ... [ifconfig net0_52 down] ... [ 3162.217907] ixgbe :81:00.0 net0: VF Reset msg received from vf 52 [ 3162.228840] [ cut here ] [ 3162.228945] WARNING: CPU: 3 PID: 31978 at drivers/net/xen-netback/interface.c:71 xenvif_skb_zerocopy_complete+0x79/0x90() [ 3162.229104] vif44.0: dead queue vif44.0-q0 decremented inflight_packets to 0 [ 3162.229184] Modules linked in: xt_physdev br_netfilter bridge stp llc tun xen_blkback ip_gre ip_tunnel gre ixgbevf drbg ansi_cprng dm_crypt algif_skcipher af_alg xen_evtchn xenfs xen_privcmd xen_pciback openvswitch nf_defrag_ipv6 libcrc32c nfsd auth_rpcgss nfs_acl nfs lockd grace fscache sunrpc nf_log_ipv4 nf_log_common xt_LOG xt_limit xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack iptable_filter ip_tables x_tables iTCO_wdt iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul raid1 aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd sb_edac lpc_ich mfd_core mei_me mei i2c_i801 ioatdma ipmi_ssif ipmi_msghandler squashfs lz4_decompress ixgbe mdio vxlan xhci_pci xhci_hcd ip6_udp_tunnel udp_tunnel ptp pps_core dca ahci libahci ehci_pci ehci_hcd usbcore usb_common tpm_tis [ 3162.230580] CPU: 3 PID: 31978 Comm: ifconfig Not tainted 4.4.11-grsec-skyport #66 [ 3162.230681] Hardware name: ABCD, BIOS SE5C610.86B.01.01.0009.C1.060120151350 06/01/2015 [ 3162.230814] c9000f7c3a68 8137d3f8
Re: [Xen-devel] [PATCH for 4.7] xen: Rename of xSplice to livepatch.
On 02/06/2016 01:14, Konrad Rzeszutek Wilk wrote: > @@ -182,7 +182,7 @@ tools/misc/xen_cpuperf > tools/misc/xen-cpuid > tools/misc/xen-detect > tools/misc/xen-tmem-list-parse > -tools/misc/xen-xsplice > +tools/misc/xen-livepatch > tools/misc/xenperf > tools/misc/xenpm > tools/misc/xen-hvmctx Hate to be a pedant, but this could do with being `sort`ed. > @@ -247,9 +247,9 @@ xen/arch/x86/efi/check.efi > xen/arch/x86/efi/disabled > xen/arch/x86/efi/mkreloc > xen/arch/x86/test/config.h > -xen/arch/x86/test/xen_hello_world.xsplice > -xen/arch/x86/test/xen_bye_world.xsplice > -xen/arch/x86/test/xen_replace_world.xsplice > +xen/arch/x86/test/xen_hello_world.xlivepatch > +xen/arch/x86/test/xen_bye_world.xlivepatch > +xen/arch/x86/test/xen_replace_world.xlivepatch > xen/arch/*/efi/boot.c > xen/arch/*/efi/compat.c > xen/arch/*/efi/efi.h And to throw a spanner in the works, a file extension of .xpatch would be a more natural fit here. (Then again, the contents of the file is all .livepatch.$FOO, so perhaps consistency is the more important attribute.) > diff --git a/MAINTAINERS b/MAINTAINERS > index 62f4ffd..d5792ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -274,6 +274,16 @@ L: minios-de...@lists.xenproject.org > T: git git://xenbits.xen.org/mini-os.git > F: config/MiniOS.mk > > +LIVE PATCH PATCHING ? > @@ -424,8 +424,8 @@ one uint32_t to determine the sub-operations and one > padding field which > *MUST* always be zero. > > > -struct xen_sysctl_xsplice_op { > -uint32_t cmd; /* IN: XEN_SYSCTL_XSPLICE_*. */ > +struct xen_sysctl_livepatch_op { > +uint32_t cmd; /* IN: XEN_SYSCTL_LIVE_PATCH_*. */ There is an inconsistency here between having a space or not. I would expect it to be XEN_SYSCTL_LIVEPATCH_*, to be consistent with other similar names. I think livepatch is preferable to live_patch in this case. Same for similar constructs. > -sysctl.cmd = XEN_SYSCTL_xsplice_op; > -sysctl.u.xsplice.cmd = XEN_SYSCTL_XSPLICE_ACTION; > +sysctl.cmd = XEN_SYSCTL_livepatch_op; > +sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVE_PATCH_ACTION; Particularly emphasised here. ~Andrew (P.S. It seems that even renaming things is hard.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 8/8] x86/vm_event: Add HVM debug exception vm_events
On Wed, Jun 1, 2016 at 4:17 PM, Andrew Cooperwrote: > On 01/06/2016 22:46, Tamas K Lengyel wrote: >> On Tue, May 31, 2016 at 1:59 AM, Jan Beulich wrote: >> On 30.05.16 at 22:13, wrote: On Mon, May 30, 2016 at 8:16 AM, Jan Beulich wrote: On 30.05.16 at 00:37, wrote: >> @@ -3393,8 +3409,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) >> } >> else { >> int handled = >> -hvm_monitor_breakpoint(regs->eip, >> - >> HVM_MONITOR_SOFTWARE_BREAKPOINT); >> +hvm_monitor_debug(regs->eip, >> + >> HVM_MONITOR_SOFTWARE_BREAKPOINT, >> + X86_EVENTTYPE_SW_EXCEPTION, >> 1); > Please let's not add further mistakes like this, assuming INT3 can't > have any prefixes. It can, even if they're useless. You mean the instruction length is not necessarily 1? Ultimately it doesn't seem to matter because reinjecting it with xc_hvm_inject_trap ignores this field. Instruction length is only required to be properly set AFAICT for a subset of debug exceptions during reinjection. >>> As you suggest later in your reply, if the insn length really doesn't >>> matter, this should be made recognizable here. Either by a suitably >>> named manifest constant (which could then even evaluate to zero), >>> or by a comment (personally I'd prefer the former, but I'm not >>> maintainer of this code). >>> >>> Jan >> >> Running Andrew's framework with xen-access monitoring breakpoints results in >> >> xen-access: >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> >> xl dmesg: >> (d28) --- Xen Test Framework --- >> (d28) Environment: HVM 64bit (Long mode 4 levels) >> (d28) Trap emulation >> (d28) Warning: FEP support not detected - some tests will be skipped >> (d28) Test cpl0: all perms ok >> (d28) Testing int3 >> (XEN) d28v0 VMRESUME error: 0x7 >> (XEN) domain_crash_sync called from vmcs.c:1599 >> (XEN) Domain 28 (vcpu#0) crashed on cpu#7: >> (XEN) [ Xen-4.6.1 x86_64 debug=n Not tainted ] >> (XEN) CPU:7 >> (XEN) RIP:0008:[<001032d1>] >> (XEN) RFLAGS: 0046 CONTEXT: hvm guest (d28v0) >> (XEN) rax: 001032d2 rbx: 001102b0 rcx: >> (XEN) rdx: 00104af0 rsi: rdi: >> (XEN) rbp: 0001 rsp: 00114f98 r8: 000f >> (XEN) r9: 00ad r10: 000f r11: 0004 >> (XEN) r12: 0003 r13: r14: >> (XEN) r15: cr0: 8011 cr4: 0020 >> (XEN) cr3: 0010b000 cr2: >> (XEN) ds: 0033 es: 0033 fs: 0033 gs: 0033 ss: cs: 0008 >> >> This is likely because xen-access sets the instruction length to 0 >> during reinjection. If I change that to 1 the tests still fail but >> without crashing the domain, output: >> >> xen-access: >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e2, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e2, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032d1, gfn=103 (vcpu 0) >> Got event from Xen >> Breakpoint: rip=001032e1, gfn=103 (vcpu 0) >> >> xl dmesg: >> (d30) Environment: HVM 64bit (Long mode 4 levels) >> (d30) Trap emulation >> (d30) Warning: FEP support not detected - some tests will be skipped >> (d30) Test cpl0: all perms ok >> (d30) Testing int3 >> (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 >> (d30) exlog[00] 0008:001032e2 vec 3[] >> (d30) exlog[01] 0008:001032e3 vec 3[] >> (d30) Testing int $3 >> (d30) Testing icebp >> (d30) Testing int $1 >> (d30) Testing into >> (d30) Test cpl0: p=0 >> (d30) Testing int3 >> (d30) Testing int $3 >> (d30) Testing icebp >> (d30) Testing int $1 >> (d30) Testing into >> (d30) Test cpl3: all perms ok >> (d30) Testing int3 >> (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 >> (d30)
[Xen-devel] [ovmf test] 95133: regressions - FAIL
flight 95133 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/95133/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 version targeted for testing: ovmf 9bedfb2f5b23c68c600770a9f853092d01eab6d4 baseline version: ovmf dc99315b8732b6e3032d01319d3f534d440b43d0 Last test of basis94748 2016-05-24 22:43:25 Z8 days Failing since 94750 2016-05-25 03:43:08 Z7 days 23 attempts Testing same since95133 2016-06-01 12:13:26 Z0 days1 attempts People who touched revisions under test: Cohen, EugeneDandan Bi Darbin Reyes Eugene Cohen Fu Siyuan Fu, Siyuan Gary Lin Hao Wu Jeff Fan Jiaxin Wu Katie Dellaquila Laszlo Ersek Liming Gao Marvin H?user Marvin Haeuser Maurice Ma Michael Zimmermann Star Zeng Yonghong Zhu 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 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 900 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] crash on boot with 4.6.1 on fedora 24
On 06/01/2016 05:01 PM, Martin Cerveny wrote: > Hello. > > On Wed, 1 Jun 2016, Boris Ostrovsky wrote: >> On 06/01/2016 12:23 PM, Martin Cerveny wrote: >>> :-( >>> >>> On Wed, 1 Jun 2016, Martin Cerveny wrote: I hit probably the same error with released "XenServer 7.0". - I have Xen4.6.1 (commit d77bac5c064ffb9dbb5b89b55b89853f1b784ebf - update Xen version to 4.6.1) - XS7 (Dundee) beta3 (kernel-3.10.96-479.383024.x86_64.rpm) work OK - XS7 release (kernel-3.10.96-484.383030.x86_64.rpm) crash - patch does not work, arch/x86/xen/mmu.c is very old in 3.10 - Can someone verify error ? Thanks, Martin Cerveny Crash (kernel-3.10.96-479.383024.x86_64.rpm): >>> ^^^ >>> correction: kernel-3.10.96-484.383030.x86_64.rpm >> If you can provide vmlinux (better) or System.map we can probably see >> whether it's the same signature. > > http://xenserver.org/open-source-virtualization-download.html > -> > XenServer-7.0.0-main.iso or XenServer-7.0.0-binpkg.iso > -> > kernel-3.10.96-484.383030.x86_64.rpm > -> > System.map-3.10.0+10 vmlinuz-3.10.0+10 > -> > http://s000.tinyupload.com/index.php?file_id=30528714656973136220 > > Thanks for analyzing, Martin This looks like a different problem, the stack is ... start_kernel cleanup_highmap xen_set_pmd_hyper arbitrary_virt_to_machine Can you reproduce this with a newer kernel? -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
Hello Aaron, On 01/06/2016 20:54, Aaron Cornelius wrote: I am doing some work with Xen 4.7 on the cubietruck (ARM32). I've noticed some strange behavior after I create/destroy enough domains and put together a script to do the add/remove for me. For this particular test I am creating a small mini-os (Mirage) domain with 32MB of RAM, deleting it, creating the new one, and so on. After running this for a while, I get the following error (with version 8478c9409a2c6726208e8dbc9f3e455b76725a33): (d846) Virtual -> physical offset = 3fc0 (d846) Checking DTB at 023ff000... (d846) [32;1mMirageOS booting...[0m (d846) Initialising console ... done. (d846) gnttab_stubs.c: initialised mini-os gntmap (d846) allocate_ondemand(1, 1) returning 230 (d846) allocate_ondemand(1, 1) returning 2301000 (XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0) (XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2) dom:(0) (XEN) p2m.c: dom1101: VMID pool exhausted (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.7.0-rc arm32 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 0021fdd4 free_domheap_pages+0x1c/0x324 (XEN) CPSR: 6001011a MODE:Hypervisor (XEN) R0: R1: 0001 R2: 0003 R3: 00304320 (XEN) R4: 41c57000 R5: 41c57188 R6: 00200200 R7: 00100100 (XEN) R8: 41c57180 R9: 43fdfe60 R10: R11:43fdfd5c R12: (XEN) HYP: SP: 43fdfd2c LR: 0025b0cc (XEN) (XEN) VTCR_EL2: 80003558 (XEN) VTTBR_EL2: 0001bfb0e000 (XEN) (XEN) SCTLR_EL2: 30cd187f (XEN)HCR_EL2: 0038663f (XEN) TTBR0_EL2: bfafc000 (XEN) (XEN)ESR_EL2: 9406 (XEN) HPFAR_EL2: 0001c810 (XEN) HDFAR: 0014 (XEN) HIFAR: 84e37182 (XEN) (XEN) Xen stack trace from sp=43fdfd2c: (XEN)002cf1b7 43fdfd64 41c57000 0100 41c57000 41c57188 00200200 00100100 (XEN)41c57180 43fdfe60 43fdfd7c 0025b0cc 41c57000 fff0 43fdfe60 (XEN)001f 044d 43fdfe60 43fdfd8c 0024f668 41c57000 fff0 43fdfda4 (XEN)0024f8f0 41c57000 001f 43fdfddc 0020854c 43fdfddc (XEN) cccd 00304600 002822bc b6f20004 044d 00304600 (XEN)00304320 d767a000 43fdfeec 00206d6c 43fdfe6c 00218f8c (XEN)0007 43fdfe30 43fdfe34 43fdfe20 0002 43fdfe48 43fdfe78 (XEN) 7622 2b0e 40023000 43fdfec8 (XEN)0002 43fdfebc 00218f8c 0001 000b b6eba880 000b (XEN)5abab87d f34aab2c 6adc50b8 e1713cd0 (XEN)b6eba8d8 50043f00 b6eb5038 b6effba8 003e 000c3034 (XEN)000b9cb8 000bda30 000bda30 b6eba56c 003e b6effba8 b6effdb0 (XEN)be9558d4 00d0 be9558d4 0071 b6effba8 b6effd6c b6ed6fb4 4a000ea1 (XEN)c01077f8 43fdff58 002067b8 00305000 be9557c8 d767a000 43fdff54 (XEN)00260130 43fdfefc 43fdff1c 200f019a 400238f4 0004 0004 (XEN)002c9f00 00304600 c094c240 00305000 be9557a0 d767a000 (XEN) 43fdff44 c094c240 00305000 be9557c8 d767a000 (XEN) 43fdff58 00263b10 b6f20004 (XEN)c094c240 00305000 be9557c8 d767a000 0001 0024 (XEN) b691ab34 c01077f8 60010013 be9557c4 c0a38600 c010c400 (XEN) Xen call trace: (XEN)[<0021fdd4>] free_domheap_pages+0x1c/0x324 (PC) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (LR) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (XEN)[<0024f668>] arch_domain_destroy+0x20/0x50 (XEN)[<0024f8f0>] arch_domain_create+0x258/0x284 (XEN)[<0020854c>] domain_create+0x2dc/0x510 (XEN)[<00206d6c>] do_domctl+0x5b4/0x1928 (XEN)[<00260130>] do_trap_hypervisor+0x1170/0x15b0 (XEN)[<00263b10>] entry.o#return_from_trap+0/0x4 (XEN) (XEN) (XEN) (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort (XEN) (XEN) (XEN) (XEN) Reboot in five seconds... I'm not 100% sure, from the "VMID pool exhausted" message it would appear that the p2m_init() function failed to allocate a VM ID, which caused domain creation to fail, and the NULL pointer dereference when trying to clean up the not-fully-created domain. However, since I only have 1 domain active at a time, I'm not sure why I should run out of VM IDs. arch_domain_destroy (and p2m_teardown) is only called when all the reference on the given domain are released. It may take a while to release all the resources. So if you launch the domain as the same time as you destroy the previous guest. You will have more than 1 domain active. Can you detail how you create/destroy guest? Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 01/06/2016 23:24, Julien Grall wrote: > Hi, > > On 01/06/2016 22:35, Andrew Cooper wrote: >> On 01/06/2016 20:54, Aaron Cornelius wrote: >>> >>> (XEN) Xen call trace: >>> (XEN)[<0021fdd4>] free_domheap_pages+0x1c/0x324 (PC) >>> (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (LR) >>> (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 >>> (XEN)[<0024f668>] arch_domain_destroy+0x20/0x50 >>> (XEN)[<0024f8f0>] arch_domain_create+0x258/0x284 >>> (XEN)[<0020854c>] domain_create+0x2dc/0x510 >>> (XEN)[<00206d6c>] do_domctl+0x5b4/0x1928 >>> (XEN)[<00260130>] do_trap_hypervisor+0x1170/0x15b0 >>> (XEN)[<00263b10>] entry.o#return_from_trap+0/0x4 >>> (XEN) >>> (XEN) >>> (XEN) >>> (XEN) Panic on CPU 0: >>> (XEN) CPU0: Unexpected Trap: Data Abort >>> (XEN) >>> (XEN) >>> (XEN) >>> (XEN) Reboot in five seconds... >> >> As for this specific crash itself, In the case of an early error path, >> p2m->root can be NULL in p2m_teardown(), in which case >> free_domheap_pages() will fall over in a heap. This patch should >> resolve it. > > Good catch! > >> >> @@ -1408,7 +1411,8 @@ void p2m_teardown(struct domain *d) >> while ( (pg = page_list_remove_head(>pages)) ) >> free_domheap_page(pg); >> >> -free_domheap_pages(p2m->root, P2M_ROOT_ORDER); >> +if ( p2m->root ) >> +free_domheap_pages(p2m->root, P2M_ROOT_ORDER); >> >> p2m->root = NULL; >> >> I would be tempted to suggest making free_domheap_pages() tolerate NULL >> pointers, except that would only be a safe thing to do if we assert that >> the order parameter is 0, which won't help this specific case. > > free_xenheap_pages already tolerates NULL (even if an order != 0). Is > there any reason to not do the same for free_domheap_pages? The xenheap allocation functions deal in terms of plain virtual addresses, while the domheap functions deal in terms of struct page_info *. Overall, this means that the domheap functions have a more restricted input/output set than their xenheap variants. As there is already precedent with xenheap, making domheap tolerate NULL is probably fine, and indeed the preferred course of action. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 01/06/2016 23:18, Julien Grall wrote: > Hi Andrew, > > On 01/06/2016 22:24, Andrew Cooper wrote: >> On 01/06/2016 21:45, Aaron Cornelius wrote: > However, since I only have 1 domain active at a time, I'm not sure > why I should run out of VM IDs. Sounds like a VMID resource leak. Check to see whether it is freed properly in domain_destroy(). ~Andrew >>> That would be my assumption. But as far as I can tell, >>> arch_domain_destroy() calls pwm_teardown() which calls >>> p2m_free_vmid(), and none of the functionality related to freeing a >>> VM ID appears to have changed in years. >> >> The VMID handling looks suspect. It can be called repeatedly during >> domain destruction, and it will repeatedly clear the same bit out of the >> vmid_mask. > > Can you explain how the p2m_free_vmid can be called multiple time? > > We have the following path: >arch_domain_destroy -> p2m_teardown -> p2m_free_vmid. > > And I can find only 3 call of arch_domain_destroy we should only be > done once per domain. > > If arch_domain_destroy is called multiple time, p2m_free_vmid will not > be the only place where Xen will be in trouble. You are correct. I was getting my phases of domain destruction mixed up. arch_domain_destroy() is strictly once, after the RCU reference of the domain has dropped to 0. > >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 838d004..7adb39a 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -1393,7 +1393,10 @@ static void p2m_free_vmid(struct domain *d) >> struct p2m_domain *p2m = >arch.p2m; >> spin_lock(_alloc_lock); >> if ( p2m->vmid != INVALID_VMID ) >> -clear_bit(p2m->vmid, vmid_mask); >> +{ >> +ASSERT(test_and_clear_bit(p2m->vmid, vmid_mask)); >> +p2m->vmid = INVALID_VMID; >> +} >> >> spin_unlock(_alloc_lock); >> } >> >> Having said that, I can't explain why that bug would result in the >> symptoms you are seeing. It is also possibly that your issue is memory >> corruption from a separate source. >> >> Can you see about instrumenting p2m_alloc_vmid()/p2m_free_vmid() (with >> vmid_alloc_lock held) to see which vmid is being allocated/freed ? >> After the initial boot of the system, you should see the same vmid being >> allocated and freed for each of your domains. > > Looking quickly at the log, the domain is dom1101. However, the number > maximum number of VMID supported is 256, so the exhaustion might be a > race somewhere. > > I would be interested to get a reproducer. I wrote a script to cycle a > domain (create/domain) in loop, and I have not seen any issue after > 1200 cycles (and counting). Given that my previous thought was wrong, I am going to suggest that some other form of memory corruption is a more likely cause. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
Hi, On 01/06/2016 22:35, Andrew Cooper wrote: On 01/06/2016 20:54, Aaron Cornelius wrote: (XEN) Xen call trace: (XEN)[<0021fdd4>] free_domheap_pages+0x1c/0x324 (PC) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (LR) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (XEN)[<0024f668>] arch_domain_destroy+0x20/0x50 (XEN)[<0024f8f0>] arch_domain_create+0x258/0x284 (XEN)[<0020854c>] domain_create+0x2dc/0x510 (XEN)[<00206d6c>] do_domctl+0x5b4/0x1928 (XEN)[<00260130>] do_trap_hypervisor+0x1170/0x15b0 (XEN)[<00263b10>] entry.o#return_from_trap+0/0x4 (XEN) (XEN) (XEN) (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort (XEN) (XEN) (XEN) (XEN) Reboot in five seconds... As for this specific crash itself, In the case of an early error path, p2m->root can be NULL in p2m_teardown(), in which case free_domheap_pages() will fall over in a heap. This patch should resolve it. Good catch! @@ -1408,7 +1411,8 @@ void p2m_teardown(struct domain *d) while ( (pg = page_list_remove_head(>pages)) ) free_domheap_page(pg); -free_domheap_pages(p2m->root, P2M_ROOT_ORDER); +if ( p2m->root ) +free_domheap_pages(p2m->root, P2M_ROOT_ORDER); p2m->root = NULL; I would be tempted to suggest making free_domheap_pages() tolerate NULL pointers, except that would only be a safe thing to do if we assert that the order parameter is 0, which won't help this specific case. free_xenheap_pages already tolerates NULL (even if an order != 0). Is there any reason to not do the same for free_domheap_pages? Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 8/8] x86/vm_event: Add HVM debug exception vm_events
On 01/06/2016 22:46, Tamas K Lengyel wrote: > On Tue, May 31, 2016 at 1:59 AM, Jan Beulichwrote: > On 30.05.16 at 22:13, wrote: >>> On Mon, May 30, 2016 at 8:16 AM, Jan Beulich wrote: >>> On 30.05.16 at 00:37, wrote: > @@ -3393,8 +3409,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > } > else { > int handled = > -hvm_monitor_breakpoint(regs->eip, > - > HVM_MONITOR_SOFTWARE_BREAKPOINT); > +hvm_monitor_debug(regs->eip, > + > HVM_MONITOR_SOFTWARE_BREAKPOINT, > + X86_EVENTTYPE_SW_EXCEPTION, 1); Please let's not add further mistakes like this, assuming INT3 can't have any prefixes. It can, even if they're useless. >>> You mean the instruction length is not necessarily 1? Ultimately it >>> doesn't seem to matter because reinjecting it with xc_hvm_inject_trap >>> ignores this field. Instruction length is only required to be properly >>> set AFAICT for a subset of debug exceptions during reinjection. >> As you suggest later in your reply, if the insn length really doesn't >> matter, this should be made recognizable here. Either by a suitably >> named manifest constant (which could then even evaluate to zero), >> or by a comment (personally I'd prefer the former, but I'm not >> maintainer of this code). >> >> Jan > > Running Andrew's framework with xen-access monitoring breakpoints results in > > xen-access: > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > > xl dmesg: > (d28) --- Xen Test Framework --- > (d28) Environment: HVM 64bit (Long mode 4 levels) > (d28) Trap emulation > (d28) Warning: FEP support not detected - some tests will be skipped > (d28) Test cpl0: all perms ok > (d28) Testing int3 > (XEN) d28v0 VMRESUME error: 0x7 > (XEN) domain_crash_sync called from vmcs.c:1599 > (XEN) Domain 28 (vcpu#0) crashed on cpu#7: > (XEN) [ Xen-4.6.1 x86_64 debug=n Not tainted ] > (XEN) CPU:7 > (XEN) RIP:0008:[<001032d1>] > (XEN) RFLAGS: 0046 CONTEXT: hvm guest (d28v0) > (XEN) rax: 001032d2 rbx: 001102b0 rcx: > (XEN) rdx: 00104af0 rsi: rdi: > (XEN) rbp: 0001 rsp: 00114f98 r8: 000f > (XEN) r9: 00ad r10: 000f r11: 0004 > (XEN) r12: 0003 r13: r14: > (XEN) r15: cr0: 8011 cr4: 0020 > (XEN) cr3: 0010b000 cr2: > (XEN) ds: 0033 es: 0033 fs: 0033 gs: 0033 ss: cs: 0008 > > This is likely because xen-access sets the instruction length to 0 > during reinjection. If I change that to 1 the tests still fail but > without crashing the domain, output: > > xen-access: > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e2, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e2, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032d1, gfn=103 (vcpu 0) > Got event from Xen > Breakpoint: rip=001032e1, gfn=103 (vcpu 0) > > xl dmesg: > (d30) Environment: HVM 64bit (Long mode 4 levels) > (d30) Trap emulation > (d30) Warning: FEP support not detected - some tests will be skipped > (d30) Test cpl0: all perms ok > (d30) Testing int3 > (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 > (d30) exlog[00] 0008:001032e2 vec 3[] > (d30) exlog[01] 0008:001032e3 vec 3[] > (d30) Testing int $3 > (d30) Testing icebp > (d30) Testing int $1 > (d30) Testing into > (d30) Test cpl0: p=0 > (d30) Testing int3 > (d30) Testing int $3 > (d30) Testing icebp > (d30) Testing int $1 > (d30) Testing into > (d30) Test cpl3: all perms ok > (d30) Testing int3 > (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 > (d30) exlog[00] 0023:001032e2 vec 3[] > (d30) exlog[01] 0023:001032e3 vec 3[] > (d30) Testing int $3 > (d30) Testing icebp > (d30) Testing int $1 > (d30) Testing into > (d30) Test
Re: [Xen-devel] Xen 4.7 crash
Hi Andrew, On 01/06/2016 22:24, Andrew Cooper wrote: On 01/06/2016 21:45, Aaron Cornelius wrote: However, since I only have 1 domain active at a time, I'm not sure why I should run out of VM IDs. Sounds like a VMID resource leak. Check to see whether it is freed properly in domain_destroy(). ~Andrew That would be my assumption. But as far as I can tell, arch_domain_destroy() calls pwm_teardown() which calls p2m_free_vmid(), and none of the functionality related to freeing a VM ID appears to have changed in years. The VMID handling looks suspect. It can be called repeatedly during domain destruction, and it will repeatedly clear the same bit out of the vmid_mask. Can you explain how the p2m_free_vmid can be called multiple time? We have the following path: arch_domain_destroy -> p2m_teardown -> p2m_free_vmid. And I can find only 3 call of arch_domain_destroy we should only be done once per domain. If arch_domain_destroy is called multiple time, p2m_free_vmid will not be the only place where Xen will be in trouble. diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 838d004..7adb39a 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1393,7 +1393,10 @@ static void p2m_free_vmid(struct domain *d) struct p2m_domain *p2m = >arch.p2m; spin_lock(_alloc_lock); if ( p2m->vmid != INVALID_VMID ) -clear_bit(p2m->vmid, vmid_mask); +{ +ASSERT(test_and_clear_bit(p2m->vmid, vmid_mask)); +p2m->vmid = INVALID_VMID; +} spin_unlock(_alloc_lock); } Having said that, I can't explain why that bug would result in the symptoms you are seeing. It is also possibly that your issue is memory corruption from a separate source. Can you see about instrumenting p2m_alloc_vmid()/p2m_free_vmid() (with vmid_alloc_lock held) to see which vmid is being allocated/freed ? After the initial boot of the system, you should see the same vmid being allocated and freed for each of your domains. Looking quickly at the log, the domain is dom1101. However, the number maximum number of VMID supported is 256, so the exhaustion might be a race somewhere. I would be interested to get a reproducer. I wrote a script to cycle a domain (create/domain) in loop, and I have not seen any issue after 1200 cycles (and counting). Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Unable to boot Xen 4.7-rc4 on HiKey
On 01/06/2016 18:10, Tamas K Lengyel wrote: Hi all, Hi Tamas, following the steps from http://wiki.xen.org/wiki/HiKey I'm unable to get the board to boot Xen. I'm using the Debian reference image from linaro as base (https://builds.96boards.org/releases/hikey/linaro/debian/latest/) and that boots fine both from the SD card directly and through startup.nsh. However, when I try to boot Xen I see only the following: Xen 4.7.0-rc (c/s Mon May 23 12:07:20 2016 +0100 git:8478c94-dirty) EFI loader Using configuration file 'xen.cfg' Image: 0x7a121000-0x7acb8a70 After that no output and dom0 never comes online on the network either. I tried compiling Xen on the device itself in case it was a problem with my cross-compile setup but no difference. Also with and without debug=y. Any tips on what might be going wrong here? I gave a quick look at the upstream device-tree of the hikey, the path /smb/uart@f7113000. If you use the upstream device-tree, it should be able to find the correct UART via "stdout-path" and therefore dtuart=... should not be necessary. Let me know if it works for you, and I will update the wiki page. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 8/8] x86/vm_event: Add HVM debug exception vm_events
On Tue, May 31, 2016 at 1:59 AM, Jan Beulichwrote: On 30.05.16 at 22:13, wrote: >> On Mon, May 30, 2016 at 8:16 AM, Jan Beulich wrote: >> On 30.05.16 at 00:37, wrote: @@ -3393,8 +3409,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) } else { int handled = -hvm_monitor_breakpoint(regs->eip, - HVM_MONITOR_SOFTWARE_BREAKPOINT); +hvm_monitor_debug(regs->eip, + HVM_MONITOR_SOFTWARE_BREAKPOINT, + X86_EVENTTYPE_SW_EXCEPTION, 1); >>> >>> Please let's not add further mistakes like this, assuming INT3 can't >>> have any prefixes. It can, even if they're useless. >> >> You mean the instruction length is not necessarily 1? Ultimately it >> doesn't seem to matter because reinjecting it with xc_hvm_inject_trap >> ignores this field. Instruction length is only required to be properly >> set AFAICT for a subset of debug exceptions during reinjection. > > As you suggest later in your reply, if the insn length really doesn't > matter, this should be made recognizable here. Either by a suitably > named manifest constant (which could then even evaluate to zero), > or by a comment (personally I'd prefer the former, but I'm not > maintainer of this code). > > Jan Running Andrew's framework with xen-access monitoring breakpoints results in xen-access: Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) xl dmesg: (d28) --- Xen Test Framework --- (d28) Environment: HVM 64bit (Long mode 4 levels) (d28) Trap emulation (d28) Warning: FEP support not detected - some tests will be skipped (d28) Test cpl0: all perms ok (d28) Testing int3 (XEN) d28v0 VMRESUME error: 0x7 (XEN) domain_crash_sync called from vmcs.c:1599 (XEN) Domain 28 (vcpu#0) crashed on cpu#7: (XEN) [ Xen-4.6.1 x86_64 debug=n Not tainted ] (XEN) CPU:7 (XEN) RIP:0008:[<001032d1>] (XEN) RFLAGS: 0046 CONTEXT: hvm guest (d28v0) (XEN) rax: 001032d2 rbx: 001102b0 rcx: (XEN) rdx: 00104af0 rsi: rdi: (XEN) rbp: 0001 rsp: 00114f98 r8: 000f (XEN) r9: 00ad r10: 000f r11: 0004 (XEN) r12: 0003 r13: r14: (XEN) r15: cr0: 8011 cr4: 0020 (XEN) cr3: 0010b000 cr2: (XEN) ds: 0033 es: 0033 fs: 0033 gs: 0033 ss: cs: 0008 This is likely because xen-access sets the instruction length to 0 during reinjection. If I change that to 1 the tests still fail but without crashing the domain, output: xen-access: Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e2, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e2, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032d1, gfn=103 (vcpu 0) Got event from Xen Breakpoint: rip=001032e1, gfn=103 (vcpu 0) xl dmesg: (d30) Environment: HVM 64bit (Long mode 4 levels) (d30) Trap emulation (d30) Warning: FEP support not detected - some tests will be skipped (d30) Test cpl0: all perms ok (d30) Testing int3 (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 (d30) exlog[00] 0008:001032e2 vec 3[] (d30) exlog[01] 0008:001032e3 vec 3[] (d30) Testing int $3 (d30) Testing icebp (d30) Testing int $1 (d30) Testing into (d30) Test cpl0: p=0 (d30) Testing int3 (d30) Testing int $3 (d30) Testing icebp (d30) Testing int $1 (d30) Testing into (d30) Test cpl3: all perms ok (d30) Testing int3 (d30) Fail redundant: Expected 1 exception (vec 3 at 001032e3), got 2 (d30) exlog[00] 0023:001032e2 vec 3[] (d30) exlog[01] 0023:001032e3 vec 3[] (d30) Testing int $3 (d30) Testing icebp (d30) Testing int $1 (d30) Testing into (d30) Test cpl3: p=0 (d30) Testing int3 (d30) Testing int $3 (d30) Testing icebp (d30) Testing int $1 (d30) Testing into (d30) Test cpl3: dpl=0 (d30) Testing int3 (d30) Testing int $3 (d30) Testing icebp (d30) Testing int $1 (d30)
Re: [Xen-devel] 4.7 qemu regression: HVM guests fail to boot from xvda
On Wed, Jun 01, Olaf Hering wrote: > Here is a list. Essentially it means that upgrading dom0 to xen-4.7 > might break existing domU if they happen to use xvd* in domU.cfg: Actually the list goes like shown below after some real testing. Now sure why xvd/qemu-xen/raw|qcow2 fails with our 4.5.2 based dom0. In the end the regression remains in xvd+qemu-xen. tool_ver vdev_str qemu format usable bootable xen-4.5 xvd qtradrawyes no xen-4.5 xvd qtrad qcow2 no - xen-4.5 xvd qmainrawyes SUSE:no, staging:yes xen-4.5 xvd qmain qcow2 yes SUSE:no, staging:yes xen-4.5 hd qtradrawyes yes xen-4.5 hd qtrad qcow2 no - xen-4.5 hd qmainrawyes yes xen-4.5 hd qmain qcow2 yes yes xen-4.6 xvd qtradrawyes no xen-4.6 xvd qtrad qcow2 no - xen-4.6 xvd qmainrawyes yes xen-4.6 xvd qmain qcow2 yes yes xen-4.6 hd qtradrawyes yes xen-4.6 hd qtrad qcow2 no - xen-4.6 hd qmainrawyes yes xen-4.6 hd qmain qcow2 yes yes xen-4.7 xvd qtradrawyes no xen-4.7 xvd qtrad qcow2 no - xen-4.7 xvd qmainrawyes no xen-4.7 xvd qmain qcow2 yes no xen-4.7 hd qtradrawyes yes xen-4.7 hd qtrad qcow2 no - xen-4.7 hd qmainrawyes yes xen-4.7 hd qmain qcow2 yes yes Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 01/06/2016 20:54, Aaron Cornelius wrote: > > (XEN) Xen call trace: > (XEN)[<0021fdd4>] free_domheap_pages+0x1c/0x324 (PC) > (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (LR) > (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 > (XEN)[<0024f668>] arch_domain_destroy+0x20/0x50 > (XEN)[<0024f8f0>] arch_domain_create+0x258/0x284 > (XEN)[<0020854c>] domain_create+0x2dc/0x510 > (XEN)[<00206d6c>] do_domctl+0x5b4/0x1928 > (XEN)[<00260130>] do_trap_hypervisor+0x1170/0x15b0 > (XEN)[<00263b10>] entry.o#return_from_trap+0/0x4 > (XEN) > (XEN) > (XEN) > (XEN) Panic on CPU 0: > (XEN) CPU0: Unexpected Trap: Data Abort > (XEN) > (XEN) > (XEN) > (XEN) Reboot in five seconds... As for this specific crash itself, In the case of an early error path, p2m->root can be NULL in p2m_teardown(), in which case free_domheap_pages() will fall over in a heap. This patch should resolve it. @@ -1408,7 +1411,8 @@ void p2m_teardown(struct domain *d) while ( (pg = page_list_remove_head(>pages)) ) free_domheap_page(pg); -free_domheap_pages(p2m->root, P2M_ROOT_ORDER); +if ( p2m->root ) +free_domheap_pages(p2m->root, P2M_ROOT_ORDER); p2m->root = NULL; I would be tempted to suggest making free_domheap_pages() tolerate NULL pointers, except that would only be a safe thing to do if we assert that the order parameter is 0, which won't help this specific case. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 01/06/2016 21:45, Aaron Cornelius wrote: >> >>> However, since I only have 1 domain active at a time, I'm not sure why I >> should run out of VM IDs. >> >> Sounds like a VMID resource leak. Check to see whether it is freed properly >> in domain_destroy(). >> >> ~Andrew > That would be my assumption. But as far as I can tell, arch_domain_destroy() > calls pwm_teardown() which calls p2m_free_vmid(), and none of the > functionality related to freeing a VM ID appears to have changed in years. The VMID handling looks suspect. It can be called repeatedly during domain destruction, and it will repeatedly clear the same bit out of the vmid_mask. diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 838d004..7adb39a 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1393,7 +1393,10 @@ static void p2m_free_vmid(struct domain *d) struct p2m_domain *p2m = >arch.p2m; spin_lock(_alloc_lock); if ( p2m->vmid != INVALID_VMID ) -clear_bit(p2m->vmid, vmid_mask); +{ +ASSERT(test_and_clear_bit(p2m->vmid, vmid_mask)); +p2m->vmid = INVALID_VMID; +} spin_unlock(_alloc_lock); } Having said that, I can't explain why that bug would result in the symptoms you are seeing. It is also possibly that your issue is memory corruption from a separate source. Can you see about instrumenting p2m_alloc_vmid()/p2m_free_vmid() (with vmid_alloc_lock held) to see which vmid is being allocated/freed ? After the initial boot of the system, you should see the same vmid being allocated and freed for each of your domains. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 95129: tolerable FAIL - PUSHED
flight 95129 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/95129/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 9 debian-install fail blocked in 95086 build-amd64-rumpuserxen 6 xen-buildfail like 95086 build-i386-rumpuserxen6 xen-buildfail like 95086 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95086 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 95086 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95086 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 95086 Tests which did not succeed, but are not blocking: test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: xen bbfd2d6ccb31a3aeea49c8f9c7884792ddc26e3b baseline version: xen 1dda826420fff634983e94f97fb8411486acda0d Last test of basis95086 2016-05-31 20:13:13 Z1 days Testing same since95098 2016-06-01 06:46:24 Z0 days2 attempts People who touched revisions under test: Chris PattersonJan Beulich jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-oldkern pass build-i386-oldkern pass
Re: [Xen-devel] crash on boot with 4.6.1 on fedora 24
Hello. On Wed, 1 Jun 2016, Boris Ostrovsky wrote: On 06/01/2016 12:23 PM, Martin Cerveny wrote: :-( On Wed, 1 Jun 2016, Martin Cerveny wrote: I hit probably the same error with released "XenServer 7.0". - I have Xen4.6.1 (commit d77bac5c064ffb9dbb5b89b55b89853f1b784ebf - update Xen version to 4.6.1) - XS7 (Dundee) beta3 (kernel-3.10.96-479.383024.x86_64.rpm) work OK - XS7 release (kernel-3.10.96-484.383030.x86_64.rpm) crash - patch does not work, arch/x86/xen/mmu.c is very old in 3.10 - Can someone verify error ? Thanks, Martin Cerveny Crash (kernel-3.10.96-479.383024.x86_64.rpm): ^^^ correction: kernel-3.10.96-484.383030.x86_64.rpm If you can provide vmlinux (better) or System.map we can probably see whether it's the same signature. http://xenserver.org/open-source-virtualization-download.html -> XenServer-7.0.0-main.iso or XenServer-7.0.0-binpkg.iso -> kernel-3.10.96-484.383030.x86_64.rpm -> System.map-3.10.0+10 vmlinuz-3.10.0+10 -> http://s000.tinyupload.com/index.php?file_id=30528714656973136220 Thanks for analyzing, Martin -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
> -Original Message- > From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of > Andrew Cooper > Sent: Wednesday, June 1, 2016 4:01 PM > To: Aaron Cornelius; Xen-devel de...@lists.xenproject.org> > Subject: Re: [Xen-devel] Xen 4.7 crash > > On 01/06/2016 20:54, Aaron Cornelius wrote: > > I am doing some work with Xen 4.7 on the cubietruck (ARM32). I've > noticed some strange behavior after I create/destroy enough domains and > put together a script to do the add/remove for me. For this particular test I > am creating a small mini-os (Mirage) domain with 32MB of RAM, deleting it, > creating the new one, and so on. > > > > After running this for a while, I get the following error (with version > 8478c9409a2c6726208e8dbc9f3e455b76725a33): > > > > (d846) Virtual -> physical offset = 3fc0 > > (d846) Checking DTB at 023ff000... > > (d846) [32;1mMirageOS booting...[0m > > (d846) Initialising console ... done. > > (d846) gnttab_stubs.c: initialised mini-os gntmap > > (d846) allocate_ondemand(1, 1) returning 230 > > (d846) allocate_ondemand(1, 1) returning 2301000 > > (XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) > > dom:(0) > > (XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2) > > dom:(0) > > (XEN) p2m.c: dom1101: VMID pool exhausted > > (XEN) CPU0: Unexpected Trap: Data Abort > > > > I'm not 100% sure, from the "VMID pool exhausted" message it would > appear that the p2m_init() function failed to allocate a VM ID, which caused > domain creation to fail, and the NULL pointer dereference when trying to > clean up the not-fully-created domain. > > > > However, since I only have 1 domain active at a time, I'm not sure why I > should run out of VM IDs. > > Sounds like a VMID resource leak. Check to see whether it is freed properly > in domain_destroy(). > > ~Andrew That would be my assumption. But as far as I can tell, arch_domain_destroy() calls pwm_teardown() which calls p2m_free_vmid(), and none of the functionality related to freeing a VM ID appears to have changed in years. - Aaron ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Basic bare metal ARM domain interface
On Tue, May 31, 2016 at 10:53:06AM +0100, Julien Grall wrote: > > > On 30/05/16 21:21, Ivan Pavić2 wrote: > >Hello, > > Hello Ivan, > > Sorry for the late answer. > > >>>I used FreeRTOS code for console output. It is based on Mini OS code. > >>>There are two problems as I've >determined > >>>with debugging. First is that vsnprintf blocks for some reason in print > >>>function so i commented it out. After the > > > >>snprintf blocks... > > > >>>hypercall function blocked as well. I modified hypercall function so it > >>>looks like this: > >>>(void)HYPERVISOR_console_io(CONSOLEIO_write, 3, "yes"); > > > >>As the call failed I decided to make hypervisor call directly in boot > >>procedure, so I put this assembler code just >before > >>branch to main: > > > >>mov r12, #18 ; console io code > >>mov r0, #0 ; write operation(first parameter) > >>mov r1, #5 ; length of message (second parameter) > >> ldr r2, =msg ; message address (third parameter) > >>.long 0xe140ea71 ; hvc instruction > >>b main ; branch to main > > For your information, hypervisor calls have to be done with data > cache enabled. Otherwise you not may get garbage (or nothing at all) > on the console. > > > > >>msg is defined as: > > > >>msg: > >>.asciz "hello" > > > >>I get deadbeef in registers, apperently something happened (xenctx output): > >>PC: 4000c5bc > >>CPSR: 61f3 > >>USR: SP: LR: > >>SVC: SPSR: SP:4011c200 LR:400080a8 > >>FIQ: SPSR: SP:40124200 LR: > >>IRQ: SPSR: SP:40120200 LR: > >>ABT: SPSR: SP:40128200 LR: > >>UND: SPSR: SP:4012c200 LR: > > > > >r0_usr: r1_usr: deadbeefr2_usr: deadbeef > > >r3_usr: r4_usr: r5_usr: > > >r6_usr: r7_usr: r8_usr: > > >r9_usr: 0064 r10_usr: 0064 r11_usr: > >>r12_usr: deadbeef > > > > > >>According to arch-arm.h r0 is return value of call. It is 0, operation > >>successful Still I don't get output on > >>console... > > > >>Thank you in advance, > > > >>Regards, > > > >>Ivan Pavic > > > >I still didn't solve why I don't see no output on emergency console, I think > >I should because if deadbeef in registers it > >do_console_io should have been called. > > The emergency console and any Xen debug facilities (hvc 0xffxx) only > works when the hypervisor has been built with debug enabled. > > By default, release version are built with debug disabled. You will > have to pass debug=y on the build command line to enable debug. > > >However new problem emerged i tried to add iomem > >parameter in configuration file to get access over gpio but domain won't > >start because operation is not permitted. > >Should I somehow release disable that memory space for dom0, perhaps in dts > >for dom0? > > > >Snippet from dom.cfg file: > > > >iomem = ["0x1340,1@0x4140"] > > > >0x1340 is base address of GPIO that I want to use. > > iomem expects a physical page number (see the documentation in > docs/man/xl.cfg.pod.5). So it should be: > > iomem = ["0x13400,1@0x41400"] > > > > >I get this error: (snippet from xl -vvv create -c dom.cfg) > > > >libxl: debug: libxl_create.c:1213:domcreate_launch_dm: dom4 iomem > >1340-1340 > >libxl: error: libxl_create.c:1220:domcreate_launch_dm: failed give dom4 > >access to iomem range 1340-1340: Operation not permitted > >libxl: debug: libxl.c:1719:devices_destroy_cb: forked pid 3835 for destroy > >of domain 4 > > > >Thank you in advance, > > Regards, > > -- > Julien Grall Hello Julien, thank you very much, I succeded with debug console and pin toggle (iomem). It's actually good that you answered late, because it made me look through xen traps.c and console.c code, so I recompiled xen few times. The main problem with was compiler, I think. I've always got data abort with hypervisor calls and similiar. Now I'm using gcc-linaro-arm-none-eabi-4.9 for both xen and application. I'll try to document this as much as possible and I'll put code on github. (github.com/dumpram). Next thing I will do, is to measure gpio latency in dom0 and guest domain with different schedulers. Final step would be to create starting point for porting some RTOS. Thank you once again for the patience, with my beginner questions, but I'm afraid that there will be more... Regards, Ivan Pavic ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.7 crash
On 01/06/2016 20:54, Aaron Cornelius wrote: > I am doing some work with Xen 4.7 on the cubietruck (ARM32). I've noticed > some strange behavior after I create/destroy enough domains and put together > a script to do the add/remove for me. For this particular test I am creating > a small mini-os (Mirage) domain with 32MB of RAM, deleting it, creating the > new one, and so on. > > After running this for a while, I get the following error (with version > 8478c9409a2c6726208e8dbc9f3e455b76725a33): > > (d846) Virtual -> physical offset = 3fc0 > (d846) Checking DTB at 023ff000... > (d846) [32;1mMirageOS booting...[0m > (d846) Initialising console ... done. > (d846) gnttab_stubs.c: initialised mini-os gntmap > (d846) allocate_ondemand(1, 1) returning 230 > (d846) allocate_ondemand(1, 1) returning 2301000 > (XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0) > (XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2) dom:(0) > (XEN) p2m.c: dom1101: VMID pool exhausted > (XEN) CPU0: Unexpected Trap: Data Abort > > > I'm not 100% sure, from the "VMID pool exhausted" message it would appear > that the p2m_init() function failed to allocate a VM ID, which caused domain > creation to fail, and the NULL pointer dereference when trying to clean up > the not-fully-created domain. > > However, since I only have 1 domain active at a time, I'm not sure why I > should run out of VM IDs. Sounds like a VMID resource leak. Check to see whether it is freed properly in domain_destroy(). ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Xen 4.7 crash
I am doing some work with Xen 4.7 on the cubietruck (ARM32). I've noticed some strange behavior after I create/destroy enough domains and put together a script to do the add/remove for me. For this particular test I am creating a small mini-os (Mirage) domain with 32MB of RAM, deleting it, creating the new one, and so on. After running this for a while, I get the following error (with version 8478c9409a2c6726208e8dbc9f3e455b76725a33): (d846) Virtual -> physical offset = 3fc0 (d846) Checking DTB at 023ff000... (d846) [32;1mMirageOS booting...[0m (d846) Initialising console ... done. (d846) gnttab_stubs.c: initialised mini-os gntmap (d846) allocate_ondemand(1, 1) returning 230 (d846) allocate_ondemand(1, 1) returning 2301000 (XEN) grant_table.c:3288:d0v1 Grant release (0) ref:(9) flags:(2) dom:(0) (XEN) grant_table.c:3288:d0v1 Grant release (1) ref:(11) flags:(2) dom:(0) (XEN) p2m.c: dom1101: VMID pool exhausted (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.7.0-rc arm32 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 0021fdd4 free_domheap_pages+0x1c/0x324 (XEN) CPSR: 6001011a MODE:Hypervisor (XEN) R0: R1: 0001 R2: 0003 R3: 00304320 (XEN) R4: 41c57000 R5: 41c57188 R6: 00200200 R7: 00100100 (XEN) R8: 41c57180 R9: 43fdfe60 R10: R11:43fdfd5c R12: (XEN) HYP: SP: 43fdfd2c LR: 0025b0cc (XEN) (XEN) VTCR_EL2: 80003558 (XEN) VTTBR_EL2: 0001bfb0e000 (XEN) (XEN) SCTLR_EL2: 30cd187f (XEN)HCR_EL2: 0038663f (XEN) TTBR0_EL2: bfafc000 (XEN) (XEN)ESR_EL2: 9406 (XEN) HPFAR_EL2: 0001c810 (XEN) HDFAR: 0014 (XEN) HIFAR: 84e37182 (XEN) (XEN) Xen stack trace from sp=43fdfd2c: (XEN)002cf1b7 43fdfd64 41c57000 0100 41c57000 41c57188 00200200 00100100 (XEN)41c57180 43fdfe60 43fdfd7c 0025b0cc 41c57000 fff0 43fdfe60 (XEN)001f 044d 43fdfe60 43fdfd8c 0024f668 41c57000 fff0 43fdfda4 (XEN)0024f8f0 41c57000 001f 43fdfddc 0020854c 43fdfddc (XEN) cccd 00304600 002822bc b6f20004 044d 00304600 (XEN)00304320 d767a000 43fdfeec 00206d6c 43fdfe6c 00218f8c (XEN)0007 43fdfe30 43fdfe34 43fdfe20 0002 43fdfe48 43fdfe78 (XEN) 7622 2b0e 40023000 43fdfec8 (XEN)0002 43fdfebc 00218f8c 0001 000b b6eba880 000b (XEN)5abab87d f34aab2c 6adc50b8 e1713cd0 (XEN)b6eba8d8 50043f00 b6eb5038 b6effba8 003e 000c3034 (XEN)000b9cb8 000bda30 000bda30 b6eba56c 003e b6effba8 b6effdb0 (XEN)be9558d4 00d0 be9558d4 0071 b6effba8 b6effd6c b6ed6fb4 4a000ea1 (XEN)c01077f8 43fdff58 002067b8 00305000 be9557c8 d767a000 43fdff54 (XEN)00260130 43fdfefc 43fdff1c 200f019a 400238f4 0004 0004 (XEN)002c9f00 00304600 c094c240 00305000 be9557a0 d767a000 (XEN) 43fdff44 c094c240 00305000 be9557c8 d767a000 (XEN) 43fdff58 00263b10 b6f20004 (XEN)c094c240 00305000 be9557c8 d767a000 0001 0024 (XEN) b691ab34 c01077f8 60010013 be9557c4 c0a38600 c010c400 (XEN) Xen call trace: (XEN)[<0021fdd4>] free_domheap_pages+0x1c/0x324 (PC) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (LR) (XEN)[<0025b0cc>] p2m_teardown+0xa0/0x108 (XEN)[<0024f668>] arch_domain_destroy+0x20/0x50 (XEN)[<0024f8f0>] arch_domain_create+0x258/0x284 (XEN)[<0020854c>] domain_create+0x2dc/0x510 (XEN)[<00206d6c>] do_domctl+0x5b4/0x1928 (XEN)[<00260130>] do_trap_hypervisor+0x1170/0x15b0 (XEN)[<00263b10>] entry.o#return_from_trap+0/0x4 (XEN) (XEN) (XEN) (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort (XEN) (XEN) (XEN) (XEN) Reboot in five seconds... I'm not 100% sure, from the "VMID pool exhausted" message it would appear that the p2m_init() function failed to allocate a VM ID, which caused domain creation to fail, and the NULL pointer dereference when trying to clean up the not-fully-created domain. However, since I only have 1 domain active at a time, I'm not sure why I should run out of VM IDs. - Aaron Cornelius ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.3-testing test] 95145: trouble: blocked/broken
flight 95145 qemu-upstream-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/95145/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 3 host-install(3) broken REGR. vs. 80927 build-i386-pvops 3 host-install(3) broken REGR. vs. 80927 build-i3863 host-install(3) broken REGR. vs. 80927 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927 Tests which did not succeed, but are not blocking: build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-pv1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pv 1 build-check(1) blocked n/a version targeted for testing: qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1 baseline version: qemuu10c1b763c26feb645627a1639e722515f3e1e876 Last test of basis80927 2016-02-06 13:30:02 Z 116 days Testing same since93977 2016-05-10 11:09:16 Z 22 days 96 attempts People who touched revisions under test: Gerd HoffmannStefano Stabellini jobs: build-amd64 broken build-i386 broken build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopsbroken build-i386-pvops broken test-amd64-amd64-xl blocked test-amd64-i386-xl blocked test-amd64-i386-qemuu-rhel6hvm-amd blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked test-amd64-i386-freebsd10-amd64 blocked test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked test-amd64-amd64-xl-qemuu-win7-amd64 blocked test-amd64-i386-xl-qemuu-win7-amd64 blocked test-amd64-amd64-xl-credit2 blocked test-amd64-i386-freebsd10-i386 blocked test-amd64-i386-qemuu-rhel6hvm-intel blocked test-amd64-amd64-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-xl-multivcpublocked
[Xen-devel] [PATCH v4 1/2] AMD IOMMU: Removing currently non-functioning guest iommu feature
From: Suravee SuthikulpanitThe guest IOMMU feature is currently not functioning. However, the current guest_iommu_init() is causing issue when it tries to register mmio handler because the it is currently called by the following code path: arch/x86/domain.c: arch_domain_create() ]- drivers/passthrough/iommu.c: iommu_domain_init() |- drivers/passthrough/amd/pci_amd_iommu.c: amd_iommu_domain_init(); |- drivers/passthrough/amd/iommu_guest.c: guest_iommu_init() At this point, the hvm_domain_initialised() has not been called. So register_mmio_handler() in guest_iommu_init() silently fails. This patch removes the guest IOMMU feature for now until we can properly support it. Signed-off-by: Suravee Suthikulpanit --- xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 1 file changed, 4 deletions(-) diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index 70b7475..fce9827 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -272,9 +272,6 @@ static int amd_iommu_domain_init(struct domain *d) hd->arch.paging_mode = is_hvm_domain(d) ? IOMMU_PAGING_MODE_LEVEL_2 : get_paging_mode(max_page); - -guest_iommu_init(d); - return 0; } @@ -474,7 +471,6 @@ static void deallocate_iommu_page_tables(struct domain *d) static void amd_iommu_domain_destroy(struct domain *d) { -guest_iommu_destroy(d); deallocate_iommu_page_tables(d); amd_iommu_flush_all_pages(d); } -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/2] x86/hvm: Add check when register io handler
From: Suravee SuthikulpanitAt the time of registering HVM I/O handler, the HVM domain might not have been initialized, which means the hvm_domain.io_handler would be NULL. In the hvm_next_io_handler(), this should be asserted. Reviewed-by: Paul Durrant Signed-off-by: Suravee Suthikulpanit --- xen/arch/x86/hvm/intercept.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c index fc757d0..bf141c9 100644 --- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -258,6 +258,8 @@ struct hvm_io_handler *hvm_next_io_handler(struct domain *d) { unsigned int i = d->arch.hvm_domain.io_handler_count++; +ASSERT(d->arch.hvm_domain.io_handler); + if ( i == NR_IO_HANDLERS ) { domain_crash(d); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator
On Wed, Jun 01, 2016 at 10:02:51AM -0600, Jan Beulich wrote: > >>> On 01.06.16 at 17:58,wrote: > > On Fri, May 27, 2016 at 02:37:06AM -0600, Jan Beulich wrote: > >> >>> On 25.05.16 at 21:48, wrote: > >> > On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote: > >> >> >>> On 15.04.16 at 14:33, wrote: > >> >> > There is a problem with place_string() which is used as early memory > >> >> > allocator. It gets memory chunks starting from start symbol and > >> >> > going down. Sadly this does not work when Xen is loaded using > >> >> > multiboot2 > >> >> > protocol because start lives on 1 MiB address. So, I tried to use > >> >> > mem_lower address calculated by GRUB2. However, it works only on some > >> >> > machines. There are machines in the wild (e.g. Dell PowerEdge R820) > >> >> > which uses first ~640 KiB for boot services code or data... :-((( > >> >> > > >> >> > In case of multiboot2 protocol we need that place_string() only > >> >> > allocate > >> >> > memory chunk for EFI memory map. However, I think that it should be > >> >> > fixed > >> >> > instead of making another function used just in one case. I thought > >> >> > about > >> >> > two solutions. > >> >> > > >> >> > 1) We could use native EFI allocation functions (e.g. AllocatePool() > >> >> >or AllocatePages()) to get memory chunk. However, later (somewhere > >> >> >in __start_xen()) we must copy its contents to safe place or > >> >> > reserve > >> >> >this in e820 memory map and map it in Xen virtual address space. > >> >> >In later case we must also care about conflicts with e.g. crash > >> >> >kernel regions which could be quite difficult. > >> >> > >> >> I don't see why that would be: Simply use an allocation type that > >> >> doesn't lead to the area getting consumed as normal RAM. Nor do > >> >> I see the kexec collision potential. Furthermore (and I think I've > >> >> said so before) ARM is already using AllocatePool() - just with an > >> >> unsuitable memory type -, so doing so on x86 too would allow for > >> > > >> > Nope, they are using standard EfiLoaderData. > >> > >> Note how I said "just with an unsuitable memory type"? > > > > Could you be more precise? > > What else do you need? Just have the arch specify the memory type to > be used (if ARM really _means_ to use that seemingly wrong type), and > make the rest of the code common. This is not the problem. I am not sure how do you understand "seemingly wrong type". Anything outside of the UEFI spec? Or maybe EfiReservedMemoryType or something like that? Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/2] Fix xen crash when starting HVM guest due to missing io handler
From: Suravee SuthikulpanitHi All, Changes from V3: * Remove calls to guest_iommu_init()/destroy() for now since the guest iommu feature is not functing and causing breakage. * Do not change the ordering of the iommu_domain_init() and hvm_domain_init() for now until we agree on proper ordering. OVERVIEW: On systems with iommu v2 enabled, the hypervisor crashes when trying to start up an HVM guest. Investigating shows that the guest_iommu_init() is called before the HVM domain is initialized. It then tries to register_mmio_handler() causing the hvm_next_io_handler() to increment the io_handler_count. However, the registration fails silently and left the I/O handler uninitialized. At later time, hvm_find_io_handler() is called and iterate through the registered handlered, but then resulting in referencing NULL pointers. This patch series proposes workaround for this issue. Thanks, Suravee Suravee Suthikulpanit (2): AMD IOMMU: Removing currently non-functioning guest iommu feature x86/hvm: Add check when register io handler xen/arch/x86/hvm/intercept.c| 2 ++ xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 2 files changed, 2 insertions(+), 4 deletions(-) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
On Wed, Jun 1, 2016 at 1:38 PM, Julien Grallwrote: > Hi Tamas, > > > On 01/06/16 19:21, Tamas K Lengyel wrote: >> >> On Wed, Jun 1, 2016 at 5:24 AM, Julien Grall wrote: >>> >>> Hi, >>> >>> >>> On 01/06/16 09:41, Jan Beulich wrote: >>> >>> >>> On 31.05.16 at 18:28, wrote: > > > On May 31, 2016 01:48, "Jan Beulich" wrote: >> >> >> > On 30.05.16 at 21:47, wrote: >>> >>> >>> On Mon, May 30, 2016 at 5:50 AM, Jan Beulich >>> wrote: >>> >>> >>> On 30.05.16 at 00:37, wrote: > > > +struct vm_event_regs_arm32 { > +uint32_t r0_usr; > +uint32_t r1_usr; > +uint32_t r2_usr; > +uint32_t r3_usr; > +uint32_t r4_usr; > +uint32_t r5_usr; > +uint32_t r6_usr; > +uint32_t r7_usr; > +uint32_t r8_usr; > +uint32_t r9_usr; > +uint32_t r10_usr; > +uint32_t r12_usr; > +uint32_t lr_usr; > +uint32_t fp; > +uint32_t pc; > +uint32_t sp_usr; > +uint32_t sp_svc; > +uint32_t spsr_svc; > +}; It would seem more natural for the "ordinary" registers to be arranged in the numerical sequence, i.e. fp, r12, sp, lr, pc. >>> >>> >>> >>> Not sure I follow. >> >> >> >> For one it is quite natural for someone looking at a sequence of >> register values to assume / expect them to be in natural order. >> And then, having them in natural (numeric) order allows e.g. >> extracting the register identifying bits from an instruction to use >> them as an array index into (part of) this structure. >> >> (For some background: I've been bitten a number of times by >> people sorting x86 registers alphabetically instead or naturally, >> i.e. EAX, EBX, ECX, EDX instead of EAX, ECX, EDX, EBX). > > > > I see, however I believe that would be a very careless use of this > struct > from the user as the register sizes are not even necessarily match the > architecture. For example we only define the 64bit x86 registers, so > trying > to access it as an array of 32bit registers wouldn't work at all. Plus > we > are not doing a full set of registers, and I rather not imply that > every > element in the "natural sequence" is present. It may be, it may be not. Once an ABI is set in stone, and if that ABI allows for optimizations (by consumers) like the one mentioned, I don't think this would be careless use. The resulting code is very clearly much more efficient than e.g. a switch() statement with a case label for each and every register. Well, yes, I already hear the "memory is cheap and hence code size doesn't matter" argument, but as said elsewhere quite recently I don't buy this. >>> >>> >>> >>> I agree with Jan here. >>> >>> ARM64 has 31 general purposes registers (x0-x30). The switch to find a >>> register based on the index will be quite big. >>> >>> If you order the register and provide all the general purposes registers >>> (x0 >>> - x30), you will be able to replace by a single line (for instance see >>> select_user_reg in arch/arm/traps.c). >> >> >> The issue is that the x86 vm_event struct right now has 32*uint64_t >> size. So if we would want to transmit all ARM64 GPRs + cpsr, PC and >> TTBR0/1 we would end up growing this structure beyond what it is >> currently. I want to avoid that as it affects both ARM32 and x86 >> introspection applications as well as now we can hold fewer events on >> the ring. I would say it is better to avoid that then potentially >> saving some on a switch in ARM64 introspection applications. > > > The design choice made for x86 should not impact the ARM design (and > vice-versa). There are key structures in the public interface which differ > between x86 and ARM (see arch_vcpu_info and arch_shared_info). And this is > fine because Xen is not meant to run an x86 guest on an ARM hypervisor. > > As far as I can tell, we currently support VM_EVENT_REASON_MEM_ACCESS and > VM_EVENT_REASON_GUEST_REQUEST. So technically the structure is set in stone. > However, we have an interface version that could be bumped, we can still > decide what is the sensible choice. > > With your suggestions only a part of the general-purpose registers will be > present in the vm_event. I understand that the ring has a limited size. If I > counted correctly, the current size of the vm_event structure is 304 bytes. > So 15 vm_event slots are available. > > If we grow the structure for ARM64, i.e 3 64-bits registers (PC, TTBR0, > TTBR1) and 1 32-bit register (CPSR). Which mean 311
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
On 01/06/16 20:38, Julien Grall wrote: Hi Tamas, On 01/06/16 19:21, Tamas K Lengyel wrote: On Wed, Jun 1, 2016 at 5:24 AM, Julien Grallwrote: Hi, On 01/06/16 09:41, Jan Beulich wrote: On 31.05.16 at 18:28, wrote: On May 31, 2016 01:48, "Jan Beulich" wrote: On 30.05.16 at 21:47, wrote: On Mon, May 30, 2016 at 5:50 AM, Jan Beulich wrote: On 30.05.16 at 00:37, wrote: +struct vm_event_regs_arm32 { +uint32_t r0_usr; +uint32_t r1_usr; +uint32_t r2_usr; +uint32_t r3_usr; +uint32_t r4_usr; +uint32_t r5_usr; +uint32_t r6_usr; +uint32_t r7_usr; +uint32_t r8_usr; +uint32_t r9_usr; +uint32_t r10_usr; +uint32_t r12_usr; +uint32_t lr_usr; +uint32_t fp; +uint32_t pc; +uint32_t sp_usr; +uint32_t sp_svc; +uint32_t spsr_svc; +}; It would seem more natural for the "ordinary" registers to be arranged in the numerical sequence, i.e. fp, r12, sp, lr, pc. Not sure I follow. For one it is quite natural for someone looking at a sequence of register values to assume / expect them to be in natural order. And then, having them in natural (numeric) order allows e.g. extracting the register identifying bits from an instruction to use them as an array index into (part of) this structure. (For some background: I've been bitten a number of times by people sorting x86 registers alphabetically instead or naturally, i.e. EAX, EBX, ECX, EDX instead of EAX, ECX, EDX, EBX). I see, however I believe that would be a very careless use of this struct from the user as the register sizes are not even necessarily match the architecture. For example we only define the 64bit x86 registers, so trying to access it as an array of 32bit registers wouldn't work at all. Plus we are not doing a full set of registers, and I rather not imply that every element in the "natural sequence" is present. It may be, it may be not. Once an ABI is set in stone, and if that ABI allows for optimizations (by consumers) like the one mentioned, I don't think this would be careless use. The resulting code is very clearly much more efficient than e.g. a switch() statement with a case label for each and every register. Well, yes, I already hear the "memory is cheap and hence code size doesn't matter" argument, but as said elsewhere quite recently I don't buy this. I agree with Jan here. ARM64 has 31 general purposes registers (x0-x30). The switch to find a register based on the index will be quite big. If you order the register and provide all the general purposes registers (x0 - x30), you will be able to replace by a single line (for instance see select_user_reg in arch/arm/traps.c). The issue is that the x86 vm_event struct right now has 32*uint64_t size. So if we would want to transmit all ARM64 GPRs + cpsr, PC and TTBR0/1 we would end up growing this structure beyond what it is currently. I want to avoid that as it affects both ARM32 and x86 introspection applications as well as now we can hold fewer events on the ring. I would say it is better to avoid that then potentially saving some on a switch in ARM64 introspection applications. The design choice made for x86 should not impact the ARM design (and vice-versa). There are key structures in the public interface which differ between x86 and ARM (see arch_vcpu_info and arch_shared_info). And this is fine because Xen is not meant to run an x86 guest on an ARM hypervisor. As far as I can tell, we currently support VM_EVENT_REASON_MEM_ACCESS and VM_EVENT_REASON_GUEST_REQUEST. So technically the structure is set in stone. However, we have an interface version that could be bumped, we can still decide what is the sensible choice. With your suggestions only a part of the general-purpose registers will be present in the vm_event. I understand that the ring has a limited size. If I counted correctly, the current size of the vm_event structure is 304 bytes. So 15 vm_event slots are available. If we grow the structure for ARM64, i.e 3 64-bits registers (PC, TTBR0, TTBR1) and 1 32-bit register (CPSR). Which mean 311 bytes, i.e 13 Sorry I miscalculated. So it should be: - 332 bytes - 12 slots vm_event slots. If the vm event subsystem is under pressure, I admit that 2 slots could be a lot. However, as not all the GPRs will be available in the structure you may have to fetch them, through XEN_DOMCTL_getvcpucontext I guess (?). The impact of the introspection to the guest will be significant. I cannot tell how often this will be the case, but I can tell you a compiler can do anything with the register allocation. I.e it could decide to privilege allocation in registers x20-x30 (because big number are nicer). If you are still concern about the pressure on the ring page, Razvan suggested to support multi-ring page. Regards, -- Julien Grall
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
Hi Razvan, On 01/06/16 20:34, Razvan Cojocaru wrote: On 06/01/16 21:21, Tamas K Lengyel wrote: On Wed, Jun 1, 2016 at 5:24 AM, Julien Grallwrote: The only purpose of having that information in the request is to quickly get things that are immediately necessary - otherwise the full context can be obtained with xc_domain_hvm_getcontext_partial(). xc_domain_hvm_getcontext_partial is not yet implemented on ARM. So currently the only solution to get the other registers will be XEN_DOMCTL_getvcpucontext. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 11/16] efi: build xen.gz with EFI code
On Wed, Jun 01, 2016 at 09:58:25AM -0600, Jan Beulich wrote: > >>> On 01.06.16 at 17:48,wrote: > > On Fri, May 27, 2016 at 02:31:52AM -0600, Jan Beulich wrote: > >> >>> On 25.05.16 at 21:07, wrote: > >> > On Wed, May 25, 2016 at 01:53:31AM -0600, Jan Beulich wrote: > >> >> >>> On 15.04.16 at 14:33, wrote: > >> >> > --- a/xen/common/efi/boot.c > >> >> > +++ b/xen/common/efi/boot.c > >> >> > @@ -1244,6 +1244,9 @@ void __init efi_init_memory(void) > >> >> > } *extra, *extra_head = NULL; > >> >> > #endif > >> >> > > >> >> > +if ( !efi_enabled(EFI_PLATFORM) ) > >> >> > +return; > >> >> > >> >> Arguably such checks would then better be put at the call site, > >> >> allowing the respective stubs to just BUG(). > >> > > >> > Ugh... I am confused. Here > >> > http://lists.xen.org/archives/html/xen-devel/2015-08/msg01790.html > >> > you asked for what is done above. So, what is your final decision? > >> > >> Well, in v2 you didn't alter stubs.c at all. It's that connection > >> which makes me think using that earlier approach might be better. > >> The more that, from a purely abstract pov, it could even allow to > >> remove some or all of stubs.c in a truly non-EFI build, provided we > >> never build with -O0. > > > > I am not sure why "provided we never build with -O0". > > Because a minimal amount of optimization is necessary for dead > calls to actually get eliminated. > > >> >> Also - what's your rule for where to put such efi_enabled() checks? > >> >> I would have expected them to get added to everything that has > >> >> a counterpart in stubs.c, but things like efi_get_time() or > >> >> efi_{halt,reset}_system() don't get any added. If those are > >> >> unreachable, I'd at least expect respective ASSERT()s to get added > >> >> there. > >> > > >> > I have added checks to functions which are called from common EFI/BIOS > > code. > >> > >> And how are the ones I named not called from "common" code? > > > > efi_get_time() call is protected by "if ( efi_enabled(EFI_PLATFORM) )" > > in xen/arch/x86/time.c. efi_halt_system() is called from nowhere, so, > > it can be removed. I will do that. > > Please don't. Instead it should get wired up properly (in > machine_halt()). OK, I will try to fix it. Hmmm... Probably efi_halt_system() call was somewhere but it was removed once. It is interesting why? > > efi_reset_system() call is protected > > by different means but EFI related. > > Where is that being protected? Nothing prevents anyone to boot > with "reboot=efi" on a non-EFI system. That's silly, but shouldn't Then it means that on non-EFI platforms we should not accept that, print relevant warning and automatically choose reboot method which make sense. > result in a crash during reboot. Right now its stub is intentionally > doing nothing (instead of BUG()ing). Above should solve that problem. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
Hi Tamas, On 01/06/16 19:21, Tamas K Lengyel wrote: On Wed, Jun 1, 2016 at 5:24 AM, Julien Grallwrote: Hi, On 01/06/16 09:41, Jan Beulich wrote: On 31.05.16 at 18:28, wrote: On May 31, 2016 01:48, "Jan Beulich" wrote: On 30.05.16 at 21:47, wrote: On Mon, May 30, 2016 at 5:50 AM, Jan Beulich wrote: On 30.05.16 at 00:37, wrote: +struct vm_event_regs_arm32 { +uint32_t r0_usr; +uint32_t r1_usr; +uint32_t r2_usr; +uint32_t r3_usr; +uint32_t r4_usr; +uint32_t r5_usr; +uint32_t r6_usr; +uint32_t r7_usr; +uint32_t r8_usr; +uint32_t r9_usr; +uint32_t r10_usr; +uint32_t r12_usr; +uint32_t lr_usr; +uint32_t fp; +uint32_t pc; +uint32_t sp_usr; +uint32_t sp_svc; +uint32_t spsr_svc; +}; It would seem more natural for the "ordinary" registers to be arranged in the numerical sequence, i.e. fp, r12, sp, lr, pc. Not sure I follow. For one it is quite natural for someone looking at a sequence of register values to assume / expect them to be in natural order. And then, having them in natural (numeric) order allows e.g. extracting the register identifying bits from an instruction to use them as an array index into (part of) this structure. (For some background: I've been bitten a number of times by people sorting x86 registers alphabetically instead or naturally, i.e. EAX, EBX, ECX, EDX instead of EAX, ECX, EDX, EBX). I see, however I believe that would be a very careless use of this struct from the user as the register sizes are not even necessarily match the architecture. For example we only define the 64bit x86 registers, so trying to access it as an array of 32bit registers wouldn't work at all. Plus we are not doing a full set of registers, and I rather not imply that every element in the "natural sequence" is present. It may be, it may be not. Once an ABI is set in stone, and if that ABI allows for optimizations (by consumers) like the one mentioned, I don't think this would be careless use. The resulting code is very clearly much more efficient than e.g. a switch() statement with a case label for each and every register. Well, yes, I already hear the "memory is cheap and hence code size doesn't matter" argument, but as said elsewhere quite recently I don't buy this. I agree with Jan here. ARM64 has 31 general purposes registers (x0-x30). The switch to find a register based on the index will be quite big. If you order the register and provide all the general purposes registers (x0 - x30), you will be able to replace by a single line (for instance see select_user_reg in arch/arm/traps.c). The issue is that the x86 vm_event struct right now has 32*uint64_t size. So if we would want to transmit all ARM64 GPRs + cpsr, PC and TTBR0/1 we would end up growing this structure beyond what it is currently. I want to avoid that as it affects both ARM32 and x86 introspection applications as well as now we can hold fewer events on the ring. I would say it is better to avoid that then potentially saving some on a switch in ARM64 introspection applications. The design choice made for x86 should not impact the ARM design (and vice-versa). There are key structures in the public interface which differ between x86 and ARM (see arch_vcpu_info and arch_shared_info). And this is fine because Xen is not meant to run an x86 guest on an ARM hypervisor. As far as I can tell, we currently support VM_EVENT_REASON_MEM_ACCESS and VM_EVENT_REASON_GUEST_REQUEST. So technically the structure is set in stone. However, we have an interface version that could be bumped, we can still decide what is the sensible choice. With your suggestions only a part of the general-purpose registers will be present in the vm_event. I understand that the ring has a limited size. If I counted correctly, the current size of the vm_event structure is 304 bytes. So 15 vm_event slots are available. If we grow the structure for ARM64, i.e 3 64-bits registers (PC, TTBR0, TTBR1) and 1 32-bit register (CPSR). Which mean 311 bytes, i.e 13 vm_event slots. If the vm event subsystem is under pressure, I admit that 2 slots could be a lot. However, as not all the GPRs will be available in the structure you may have to fetch them, through XEN_DOMCTL_getvcpucontext I guess (?). The impact of the introspection to the guest will be significant. I cannot tell how often this will be the case, but I can tell you a compiler can do anything with the register allocation. I.e it could decide to privilege allocation in registers x20-x30 (because big number are nicer). If you are still concern about the pressure on the ring page, Razvan suggested to support multi-ring page. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
On 06/01/16 21:21, Tamas K Lengyel wrote: > On Wed, Jun 1, 2016 at 5:24 AM, Julien Grallwrote: >> Hi, >> >> >> On 01/06/16 09:41, Jan Beulich wrote: >> >> On 31.05.16 at 18:28, wrote: On May 31, 2016 01:48, "Jan Beulich" wrote: > > On 30.05.16 at 21:47, wrote: >> >> On Mon, May 30, 2016 at 5:50 AM, Jan Beulich wrote: >> >> On 30.05.16 at 00:37, wrote: +struct vm_event_regs_arm32 { +uint32_t r0_usr; +uint32_t r1_usr; +uint32_t r2_usr; +uint32_t r3_usr; +uint32_t r4_usr; +uint32_t r5_usr; +uint32_t r6_usr; +uint32_t r7_usr; +uint32_t r8_usr; +uint32_t r9_usr; +uint32_t r10_usr; +uint32_t r12_usr; +uint32_t lr_usr; +uint32_t fp; +uint32_t pc; +uint32_t sp_usr; +uint32_t sp_svc; +uint32_t spsr_svc; +}; >>> >>> >>> It would seem more natural for the "ordinary" registers to be >>> arranged in the numerical sequence, i.e. fp, r12, sp, lr, pc. >> >> >> Not sure I follow. > > > For one it is quite natural for someone looking at a sequence of > register values to assume / expect them to be in natural order. > And then, having them in natural (numeric) order allows e.g. > extracting the register identifying bits from an instruction to use > them as an array index into (part of) this structure. > > (For some background: I've been bitten a number of times by > people sorting x86 registers alphabetically instead or naturally, > i.e. EAX, EBX, ECX, EDX instead of EAX, ECX, EDX, EBX). I see, however I believe that would be a very careless use of this struct from the user as the register sizes are not even necessarily match the architecture. For example we only define the 64bit x86 registers, so trying to access it as an array of 32bit registers wouldn't work at all. Plus we are not doing a full set of registers, and I rather not imply that every element in the "natural sequence" is present. It may be, it may be not. >>> >>> >>> Once an ABI is set in stone, and if that ABI allows for optimizations >>> (by consumers) like the one mentioned, I don't think this would be >>> careless use. The resulting code is very clearly much more efficient >>> than e.g. a switch() statement with a case label for each and every >>> register. Well, yes, I already hear the "memory is cheap and hence >>> code size doesn't matter" argument, but as said elsewhere quite >>> recently I don't buy this. >> >> >> I agree with Jan here. >> >> ARM64 has 31 general purposes registers (x0-x30). The switch to find a >> register based on the index will be quite big. >> >> If you order the register and provide all the general purposes registers (x0 >> - x30), you will be able to replace by a single line (for instance see >> select_user_reg in arch/arm/traps.c). > > The issue is that the x86 vm_event struct right now has 32*uint64_t > size. So if we would want to transmit all ARM64 GPRs + cpsr, PC and > TTBR0/1 we would end up growing this structure beyond what it is > currently. I want to avoid that as it affects both ARM32 and x86 > introspection applications as well as now we can hold fewer events on > the ring. I would say it is better to avoid that then potentially > saving some on a switch in ARM64 introspection applications. > >> >> This will also likely speed up your introspection application. > > Win some, lose some. I would prefer if these changes had no > cross-architectural effect unless absolutely required. Razvan, what do > you think? I think that if we keep a single page sized event ring buffer it would be best to only send what consumers need right now. The only purpose of having that information in the request is to quickly get things that are immediately necessary - otherwise the full context can be obtained with xc_domain_hvm_getcontext_partial(). As long as there's a comment present that says that this is all the information needed at this time, and the struct can grow if needs change, it might be best not to add unnecessary data just in case somebody will need it later. Having said that, growing the struct by about 16 bytes or so shouldn't pose huge problems (I'm not sure I've calculated the correct size based on your previous message). Our application only uses sync-style events, so the ring buffer can only be full when all VCPUs are blocked, so even a one page ring buffer should be enough for most sensible applications right now. But, the way to go for the future, as I've said before, is IMHO to grow the ring buffer and possibly add all the data that
Re: [Xen-devel] crash on boot with 4.6.1 on fedora 24
On 06/01/2016 12:23 PM, Martin Cerveny wrote: > :-( > > On Wed, 1 Jun 2016, Martin Cerveny wrote: >> I hit probably the same error with released "XenServer 7.0". >> - I have Xen4.6.1 (commit d77bac5c064ffb9dbb5b89b55b89853f1b784ebf - >> update Xen version to 4.6.1) >> - XS7 (Dundee) beta3 (kernel-3.10.96-479.383024.x86_64.rpm) work OK >> - XS7 release (kernel-3.10.96-484.383030.x86_64.rpm) crash >> - patch does not work, arch/x86/xen/mmu.c is very old in 3.10 >> - Can someone verify error ? >> >> Thanks, Martin Cerveny >> >> Crash (kernel-3.10.96-479.383024.x86_64.rpm): > ^^^ > correction: kernel-3.10.96-484.383030.x86_64.rpm If you can provide vmlinux (better) or System.map we can probably see whether it's the same signature. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/16] x86: add multiboot2 protocol support for relocatable images
On Wed, Jun 01, 2016 at 08:44:31AM -0600, Jan Beulich wrote: > >>> On 01.06.16 at 15:35,wrote: > > On Wed, May 25, 2016 at 05:03:20AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: > >> > --- a/xen/arch/x86/boot/head.S > >> > +++ b/xen/arch/x86/boot/head.S > >> > @@ -79,6 +79,13 @@ multiboot2_header_start: > >> > /* Align modules at page boundry. */ > >> > mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED) > >> > > >> > +/* Load address preference. */ > >> > +mb2ht_init MB2_HT(RELOCATABLE), MB2_HT(OPTIONAL), \ > >> > + sym_offset(start), /* Min load address. */ \ > >> > + 0x, /* Max load address (4 GiB - 1). */ \ > >> > >> Hardly - that would allow us to be loaded at 4G - 2M, no matter > >> how large the image. Or else the comment is misleading. > > > > This is the highest address at which memory region allocated for image > > may end. > > You saying "end" then means the comment is misleading. > > >> > @@ -178,30 +185,39 @@ efi_multiboot2_proto: > >> > and $~(MULTIBOOT2_TAG_ALIGN-1),%rcx > >> > > >> > 0: > >> > +/* Get Xen image load base address from Multiboot2 information. > >> > */ > >> > +cmpl$MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR,MB2_tag_type(%rcx) > >> > +jne 1f > >> > + > >> > +mov MB2_load_base_addr(%rcx),%r15d > >> > +sub $XEN_IMG_OFFSET,%r15 > >> > +jmp 4f > >> > >> Why do we need to read this from the table? Can't we easily calculate > >> this ourselves? > > > > Potentially yes but why do not use data from boot loader? > > Because it's (a) likely easier to just calculate and (b) we should In 64-bit mode yes but 32-bit mode requires additional call and pop. Is it OK for you? > perhaps trust ourselves more than an external entity? :-))) > >> > +1: > >> > /* Get EFI SystemTable address from Multiboot2 information. */ > >> > cmpl$MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx) > >> > -jne 1f > >> > +jne 2f > >> > > >> > mov MB2_efi64_st(%rcx),%rsi > >> > > >> > /* Do not clear BSS twice and do not go into real mode. */ > >> > movb$1,skip_realmode(%rip) > >> > -jmp 3f > >> > +jmp 4f > >> > > >> > -1: > >> > +2: > >> > /* Get EFI ImageHandle address from Multiboot2 information. */ > >> > cmpl$MULTIBOOT2_TAG_TYPE_EFI64_IH,MB2_tag_type(%rcx) > >> > -jne 2f > >> > +jne 3f > >> > > >> > mov MB2_efi64_ih(%rcx),%rdi > >> > -jmp 3f > >> > +jmp 4f > >> > > >> > -2: > >> > +3: > >> > /* Is it the end of Multiboot2 information? */ > >> > cmpl$MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx) > >> > je run_bs > >> > > >> > -3: > >> > +4: > >> > /* Go to next Multiboot2 information tag. */ > >> > add MB2_tag_size(%rcx),%ecx > >> > add $(MULTIBOOT2_TAG_ALIGN-1),%rcx > >> > >> See why numeric labels are bad in situations like this? The (much) > >> earlier patch should use .L labels here, and the patch here then > >> should simply follow suit. > > > > Then we should change legacy multiboot (v1) code too. Just to be in line > > new stuff here. Does it pays? And I am not sure that patching convenience > > overweight convenience of numeric labels here. > > Well, it's always this same discussion: Bad examples shouldn't be > used as excuse to add further bad examples. If you feel like also > changing the mb1 code - go for it. But if you don't, I'm fine with > just new code avoiding old mistakes. Make sense. However, do you suggest that I should avoid numeric labels at all? Probably they are useful in some situations. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms
On Fri, May 27, 2016 at 03:02:25AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 23:02,wrote: > > On Wed, May 25, 2016 at 03:32:37AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: > >> > bad_cpu: > >> > mov $(sym_phys(.Lbad_cpu_msg)),%esi # Error message > >> > -jmp print_err > >> > +mov $0xB8000,%edi # VGA framebuffer > >> > +jmp 1f > >> > not_multiboot: > >> > mov $(sym_phys(.Lbad_ldr_msg)),%esi # Error message > >> > -print_err: > >> > -mov $0xB8000,%edi # VGA framebuffer > >> > +mov $0xB8000,%edi # VGA framebuffer > >> > +jmp 1f > >> > +mb2_too_old: > >> > +mov $(sym_phys(.Lbad_ldr_mb2)),%esi # Error message > >> > +xor %edi,%edi # No VGA framebuffer > >> > >> Leaving aside that "framebuffer" really is a bad term here (we're > >> talking of text mode output after all), limiting the output to serial > > > > Yep, but then we should change this in other places too. Maybe in separate > > patch. > > Since you touch (move) it, replacing the word "framebuffer" wouldn't > seem like something making the patch any more difficult to understand. > > >> isn't going to be very helpful in the field, I'm afraid. Even more so > >> that there's no guarantee for a UART to be at port 0x3f8. That's > > > > Right but we do not have big choice here at very early boot stage... :-((( > > Excuse me? You're running on EFI, so you have full infrastructure > available to you. As much as in a normal BIOS scenario (and on a > half way normal system) we can assume a text mode screen with > video memory mapped at B8000, under EFI we can assume output > capabilities (whichever the system owner set up in the firmware > setup). Potentially we can do that for bad_cpu only. However, does it pays? I suppose that most, if not all, platforms with UEFI have CPUs with X86_FEATURE_LM. Sadly, It it not feasible for not_multiboot and mb2_too_old. Former means that we were loaded by unknown boot loader and interface is not defined. Hence, we are not able to get anything from EFI. Latter means that we were booted via older multiboot2 version which shutdown boot services. > >> not much of a problem for the other two messages as people are > >> unlikely to try to boot Xen on an unsuitable system, but I view it > >> as quite possible for Xen to be tried to get booted with an > >> unsuitable grub2. > >> > >> IOW - this needs a better solution, presumably using EFI boot > >> service output functions. > > > > No way, here boot services are dead. GRUB2 (or other loader) > > shutdown them... :-((( > > Wasn't one of your grub2 changes being precisely the deferral of > that to Xen? Sure, but if we are here then it means that we were loaded by earlier multiboot2 protocol version which does not understand features added by me. > >> > +cld > >> > + > >> > +/* Check for Multiboot2 bootloader. */ > >> > +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax > >> > +je efi_multiboot2_proto > >> > + > >> > +/* Jump to not_multiboot after switching CPU to x86_32 mode. */ > >> > +lea not_multiboot(%rip),%rdi > >> > +jmp x86_32_switch > >> > >> What I've said above would also eliminate the need to switch to > >> 32-bit mode just for emitting an error message and halting the > >> system. > > > > It is not possible. We just know that we run on EFI platform here. > > However, we are not able to get EFI SystemTable pointer. > > How are we not able? Later C code does it afair, so why would it not > be possible here? Ditto. > >> > +efi_multiboot2_proto: > >> > >> .Lefi_multiboot2_proto > > > > OK if you insist. However, I think that we are loosing helpful > > debug information this way. > > I don't see why or how. Labels persisting in the final symbol table > are useful only for generating stack traces, yet if you crash this > early there won't be any stack trace anyway - you don't even > have an IDT set up yet. OK, but it is much easier to identify addresses for breakpoints if you have proper labels. And this one looks quite useful. > >> > +/* > >> > + * Multiboot2 information address is 32-bit, > >> > + * so, zero higher half of %rbx. > >> > + */ > >> > +mov %ebx,%ebx > >> > >> Wait, no - that's a protocol bug then. We're being entered in 64-bit > >> mode here, so registers should be in 64-bit clean state. > > > > You mean higher half cleared. Right? This is not guaranteed. > > Hence me saying "that's a protocol bug then". Why? Protocol strictly says that "this is not guaranteed". What is the problem with that? Potentially loader can set %rbx higher half to e.g. 0 but I do not think it is needed. > > Please check this: > > http://lists.gnu.org/archive/html/grub-devel/2016-03/msg00304.html > > Other than the
[Xen-devel] [xen-unstable-smoke test] 95149: tolerable all pass - PUSHED
flight 95149 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/95149/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass version targeted for testing: xen 7e81d992b183c47a74eea5ecd613d27950b5cdc3 baseline version: xen 1f8168784ebfd862586d1d02acb3352ec3715d12 Last test of basis95143 2016-06-01 15:03:57 Z0 days Testing same since95149 2016-06-01 17:01:03 Z0 days1 attempts People who touched revisions under test: Ian JacksonStefano Stabellini Vitaly Kuznetsov Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=7e81d992b183c47a74eea5ecd613d27950b5cdc3 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 7e81d992b183c47a74eea5ecd613d27950b5cdc3 + branch=xen-unstable-smoke + revision=7e81d992b183c47a74eea5ecd613d27950b5cdc3 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-4.6-testing + '[' x7e81d992b183c47a74eea5ecd613d27950b5cdc3 = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print
[Xen-devel] [[PATCH v2 1/2] libfsimage: replace deprecated readdir_r() with readdir()
From: Chris PattersonReplace the usage of readdir_r() with readdir() to address a compilation error under glibc due to the deprecation of readdir_r for their next release (2.24) [1, 2]. -- From the GNU libc manual [3]: " It is expected that future versions of POSIX will obsolete readdir_r and mandate the level of thread safety for readdir which is provided by the GNU C Library and other implementations today. " There is a filed bug in the Austin Group Defect Tracker [4] in which 'dalias' proposes (in comment 0001632) that: " I would like to propose an alternate solution. For readdir, replace the text: "The readdir() function need not be thread-safe." with: "If multiple threads call the readdir() function with the same directory stream argument and without synchronization to preclude simultaneous access, then the behavior is undefined." With this change, the clunky readdir_r function is no longer needed or useful, and should probably be deprecated. As the only reasonable way to meet the implementation requirements for readdir is to have the dirent buffer in the DIR structure, this change should not require any change to existing implementations. " [1] https://sourceware.org/ml/libc-alpha/2016-02/msg00093.html [2] https://sourceware.org/bugzilla/show_bug.cgi?id=19056 [3] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html [4] http://austingroupbugs.net/view.php?id=696 -- v2: - Additional detail in commit message - Cleanup additional related (no longer used) code Signed-off-by: Chris Patterson --- tools/libfsimage/common/fsimage_plugin.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/tools/libfsimage/common/fsimage_plugin.c b/tools/libfsimage/common/fsimage_plugin.c index 3fa06c7..5ab8d93 100644 --- a/tools/libfsimage/common/fsimage_plugin.c +++ b/tools/libfsimage/common/fsimage_plugin.c @@ -122,8 +122,7 @@ fail: static int load_plugins(void) { const char *fsdir = getenv("FSIMAGE_FSDIR"); - struct dirent *dp = NULL; - struct dirent *dpp; + struct dirent *de; DIR *dir = NULL; char *tmp = NULL; size_t name_max; @@ -139,22 +138,17 @@ static int load_plugins(void) if ((tmp = malloc(name_max + 1)) == NULL) goto fail; - if ((dp = malloc(sizeof (struct dirent) + name_max + 1)) == NULL) - goto fail; - if ((dir = opendir(fsdir)) == NULL) goto fail; - bzero(dp, sizeof (struct dirent) + name_max + 1); - - while (readdir_r(dir, dp, ) == 0 && dpp != NULL) { - if (strcmp(dpp->d_name, ".") == 0) + while ((de = readdir(dir)) != NULL) { + if (strcmp(de->d_name, ".") == 0) continue; - if (strcmp(dpp->d_name, "..") == 0) + if (strcmp(de->d_name, "..") == 0) continue; (void) snprintf(tmp, name_max, "%s/%s/fsimage.so", fsdir, - dpp->d_name); + de->d_name); if (init_plugin(tmp) != 0) goto fail; @@ -167,7 +161,6 @@ fail: if (dir != NULL) (void) closedir(dir); free(tmp); - free(dp); errno = err; return (ret); } -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/8] arm/vm_event: get/set registers
On Wed, Jun 1, 2016 at 5:24 AM, Julien Grallwrote: > Hi, > > > On 01/06/16 09:41, Jan Beulich wrote: > > On 31.05.16 at 18:28, wrote: >>> >>> On May 31, 2016 01:48, "Jan Beulich" wrote: >>> On 30.05.16 at 21:47, wrote: > > On Mon, May 30, 2016 at 5:50 AM, Jan Beulich wrote: > > On 30.05.16 at 00:37, wrote: >>> >>> +struct vm_event_regs_arm32 { >>> +uint32_t r0_usr; >>> +uint32_t r1_usr; >>> +uint32_t r2_usr; >>> +uint32_t r3_usr; >>> +uint32_t r4_usr; >>> +uint32_t r5_usr; >>> +uint32_t r6_usr; >>> +uint32_t r7_usr; >>> +uint32_t r8_usr; >>> +uint32_t r9_usr; >>> +uint32_t r10_usr; >>> +uint32_t r12_usr; >>> +uint32_t lr_usr; >>> +uint32_t fp; >>> +uint32_t pc; >>> +uint32_t sp_usr; >>> +uint32_t sp_svc; >>> +uint32_t spsr_svc; >>> +}; >> >> >> It would seem more natural for the "ordinary" registers to be >> arranged in the numerical sequence, i.e. fp, r12, sp, lr, pc. > > > Not sure I follow. For one it is quite natural for someone looking at a sequence of register values to assume / expect them to be in natural order. And then, having them in natural (numeric) order allows e.g. extracting the register identifying bits from an instruction to use them as an array index into (part of) this structure. (For some background: I've been bitten a number of times by people sorting x86 registers alphabetically instead or naturally, i.e. EAX, EBX, ECX, EDX instead of EAX, ECX, EDX, EBX). >>> >>> >>> I see, however I believe that would be a very careless use of this struct >>> from the user as the register sizes are not even necessarily match the >>> architecture. For example we only define the 64bit x86 registers, so >>> trying >>> to access it as an array of 32bit registers wouldn't work at all. Plus we >>> are not doing a full set of registers, and I rather not imply that every >>> element in the "natural sequence" is present. It may be, it may be not. >> >> >> Once an ABI is set in stone, and if that ABI allows for optimizations >> (by consumers) like the one mentioned, I don't think this would be >> careless use. The resulting code is very clearly much more efficient >> than e.g. a switch() statement with a case label for each and every >> register. Well, yes, I already hear the "memory is cheap and hence >> code size doesn't matter" argument, but as said elsewhere quite >> recently I don't buy this. > > > I agree with Jan here. > > ARM64 has 31 general purposes registers (x0-x30). The switch to find a > register based on the index will be quite big. > > If you order the register and provide all the general purposes registers (x0 > - x30), you will be able to replace by a single line (for instance see > select_user_reg in arch/arm/traps.c). The issue is that the x86 vm_event struct right now has 32*uint64_t size. So if we would want to transmit all ARM64 GPRs + cpsr, PC and TTBR0/1 we would end up growing this structure beyond what it is currently. I want to avoid that as it affects both ARM32 and x86 introspection applications as well as now we can hold fewer events on the ring. I would say it is better to avoid that then potentially saving some on a switch in ARM64 introspection applications. > > This will also likely speed up your introspection application. Win some, lose some. I would prefer if these changes had no cross-architectural effect unless absolutely required. Razvan, what do you think? Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [[PATCH v2 2/2] libxl: replace deprecated readdir_r() with readdir()
From: Chris PattersonReplace the usage of readdir_r() with readdir() to address a compilation error under glibc due to the deprecation of readdir_r for their next release (2.24) [1, 2]. Remove code specific to usage of readdir_r which is no longer required, such as zalloc_dirent(). -- From the GNU libc manual [3]: " It is expected that future versions of POSIX will obsolete readdir_r and mandate the level of thread safety for readdir which is provided by the GNU C Library and other implementations today. " There is a filed bug in the Austin Group Defect Tracker [4] in which 'dalias' proposes (in comment 0001632) that: " I would like to propose an alternate solution. For readdir, replace the text: "The readdir() function need not be thread-safe." with: "If multiple threads call the readdir() function with the same directory stream argument and without synchronization to preclude simultaneous access, then the behavior is undefined." With this change, the clunky readdir_r function is no longer needed or useful, and should probably be deprecated. As the only reasonable way to meet the implementation requirements for readdir is to have the dirent buffer in the DIR structure, this change should not require any change to existing implementations. " [1] https://sourceware.org/ml/libc-alpha/2016-02/msg00093.html [2] https://sourceware.org/bugzilla/show_bug.cgi?id=19056 [3] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html [4] http://austingroupbugs.net/view.php?id=696 -- v2: - Additional detail in commit message - Fix readdir loop logic (do not treat NULL as error) - Cleanup additional related (no longer used) code Signed-off-by: Chris Patterson --- tools/libxl/libxl_pvusb.c | 42 +++--- tools/libxl/libxl_utils.c | 14 +- 2 files changed, 4 insertions(+), 52 deletions(-) diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c index 9f1e842..01819bd 100644 --- a/tools/libxl/libxl_pvusb.c +++ b/tools/libxl/libxl_pvusb.c @@ -508,19 +508,10 @@ int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, return rc; } -static void *zalloc_dirent(libxl__gc *gc, const char *dirpath) -{ -size_t need = offsetof(struct dirent, d_name) + - pathconf(dirpath, _PC_NAME_MAX) + 1; - -return libxl__zalloc(gc, need); -} - static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr) { DIR *dir; char *busid = NULL; -struct dirent *de_buf; struct dirent *de; /* invalid hostbus or hostaddr */ @@ -533,22 +524,12 @@ static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr) return NULL; } -de_buf = zalloc_dirent(gc, SYSFS_USB_DEV); - -for (;;) { +while ((de = readdir(dir)) != NULL) { char *filename; void *buf; int busnum = -1; int devnum = -1; -int r = readdir_r(dir, de_buf, ); -if (r) { -LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV); -break; -} -if (!de) -break; - if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) continue; @@ -1157,9 +1138,7 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid, { DIR *dir; char *buf; -struct dirent *de_buf; struct dirent *de; -int rc; *intfs = NULL; *num = 0; @@ -1172,19 +1151,7 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid, return ERROR_FAIL; } -de_buf = zalloc_dirent(gc, SYSFS_USB_DEV); - -for (;;) { -int r = readdir_r(dir, de_buf, ); - -if (r) { -LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV); -rc = ERROR_FAIL; -goto out; -} -if (!de) -break; - +while ((de = readdir(dir)) != NULL) { if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) continue; @@ -1196,11 +1163,8 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid, } } -rc = 0; - -out: closedir(dir); -return rc; +return 0; } /* Encode usb interface so that it could be written to xenstore as a key. diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index ceb8825..5730774 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -548,21 +548,9 @@ int libxl__remove_directory(libxl__gc *gc, const char *dirpath) goto out; } -size_t need = offsetof(struct dirent, d_name) + -pathconf(dirpath, _PC_NAME_MAX) + 1; -struct dirent *de_buf = libxl__zalloc(gc, need); struct dirent *de; -for (;;) { -int r = readdir_r(d, de_buf, ); -if (r) { -LOGE(ERROR, "failed to readdir %s for removal", dirpath); -rc =
[Xen-devel] Unable to boot Xen 4.7-rc4 on HiKey
Hi all, following the steps from http://wiki.xen.org/wiki/HiKey I'm unable to get the board to boot Xen. I'm using the Debian reference image from linaro as base (https://builds.96boards.org/releases/hikey/linaro/debian/latest/) and that boots fine both from the SD card directly and through startup.nsh. However, when I try to boot Xen I see only the following: Xen 4.7.0-rc (c/s Mon May 23 12:07:20 2016 +0100 git:8478c94-dirty) EFI loader Using configuration file 'xen.cfg' Image: 0x7a121000-0x7acb8a70 After that no output and dom0 never comes online on the network either. I tried compiling Xen on the device itself in case it was a problem with my cross-compile setup but no difference. Also with and without debug=y. Any tips on what might be going wrong here? Thanks, Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 95143: tolerable all pass - PUSHED
flight 95143 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/95143/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass version targeted for testing: xen 1f8168784ebfd862586d1d02acb3352ec3715d12 baseline version: xen 909bd140bfbfd3c762ae7ebf2bb41da00842c77d Last test of basis95128 2016-06-01 11:00:53 Z0 days Testing same since95143 2016-06-01 15:03:57 Z0 days1 attempts People who touched revisions under test: Julien GrallStefano Stabellini Wei Chen jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=xen-unstable-smoke + revision=1f8168784ebfd862586d1d02acb3352ec3715d12 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 1f8168784ebfd862586d1d02acb3352ec3715d12 + branch=xen-unstable-smoke + revision=1f8168784ebfd862586d1d02acb3352ec3715d12 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=xen + xenbranch=xen-unstable-smoke + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-smoke + prevxenbranch=xen-4.6-testing + '[' x1f8168784ebfd862586d1d02acb3352ec3715d12 = x ']' + : tested/2.6.39.x + . ./ap-common ++ : osst...@xenbits.xen.org +++ getconfig OsstestUpstream +++ perl -e ' use Osstest; readglobalconfig(); print $c{"OsstestUpstream"} or die $!; ' ++ : ++ : git://xenbits.xen.org/xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/xen.git ++ : git://xenbits.xen.org/qemu-xen-traditional.git ++ : git://git.kernel.org ++ : git://git.kernel.org/pub/scm/linux/kernel/git ++ : git ++ : git://xenbits.xen.org/libvirt.git ++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git ++ : git://xenbits.xen.org/libvirt.git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : git ++ : git://xenbits.xen.org/rumpuser-xen.git ++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git +++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src '[fetch=try]' +++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src +++ local 'options=[fetch=try]' getconfig GitCacheProxy perl -e ' use Osstest; readglobalconfig(); print $c{"GitCacheProxy"} or die $!; ' +++ local
Re: [Xen-devel] crash on boot with 4.6.1 on fedora 24
:-( On Wed, 1 Jun 2016, Martin Cerveny wrote: I hit probably the same error with released "XenServer 7.0". - I have Xen4.6.1 (commit d77bac5c064ffb9dbb5b89b55b89853f1b784ebf - update Xen version to 4.6.1) - XS7 (Dundee) beta3 (kernel-3.10.96-479.383024.x86_64.rpm) work OK - XS7 release (kernel-3.10.96-484.383030.x86_64.rpm) crash - patch does not work, arch/x86/xen/mmu.c is very old in 3.10 - Can someone verify error ? Thanks, Martin Cerveny Crash (kernel-3.10.96-479.383024.x86_64.rpm): ^^^ correction: kernel-3.10.96-484.383030.x86_64.rpm about to get started... (XEN) d0v0: unhandled page fault (ec=) (XEN) Pagetable walk from 88010278b080: (XEN) L4[0x110] = 000439a0d067 1a0d (XEN) L3[0x004] = (XEN) domain_crash_sync called from entry.S: fault at 82d08022b2c3 create_bounce_frame+0x12b/0x13a (XEN) Domain 0 (vcpu#0) crashed on cpu#0: (XEN) [ Xen-4.6.1-vgpu x86_64 debug=n Not tainted ] (XEN) CPU:0 (XEN) RIP:e033:[] (XEN) RFLAGS: 0282 EM: 1 CONTEXT: pv guest (d0v0) (XEN) rax: 88010278b080 rbx: 81a1 rcx: 8880 (XEN) rdx: 3000 rsi: 81a01de4 rdi: 00043a95c067 (XEN) rbp: 81a01df8 rsp: 81a01da0 r8: 3000 (XEN) r9: 8800 r10: 0001 r11: 0001 (XEN) r12: 8000 r13: 81a1 r14: (XEN) r15: 0082 cr0: 8005003b cr4: 001526e0 (XEN) cr3: 000439a0c000 cr2: 88010278b080 (XEN) ds: es: fs: gs: ss: e02b cs: e033 (XEN) Guest stack trace from rsp=81a01da0: (XEN)8880 0001 81005dea (XEN)0001e030 00010082 81a01de0 e02b (XEN)000181a1 81a1 8000 81a01e40 (XEN)810067f6 0001 0001 81a1 (XEN)8000 83d7a000 81df (XEN)81a01e78 81aedf2d 0114b000 0100 (XEN) 81a01ef0 (XEN)81add76b 81a01ef0 (XEN)81a01f08 0010 81a01f00 81a01ec0 (XEN) 81b69900 (XEN) 81a01f30 81ad5bb9 (XEN) 81b732c0 81a01f60 (XEN) 81a01f40 81ad55ee (XEN)81a01ff8 81ad8b48 000306e4 000100200800 (XEN)03010032 0005 0020 (XEN) (XEN) (XEN) (XEN) (XEN)0f0060c0c748 c305 (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds. (XEN) Resetting with ACPI MEMORY or I/O RESET_REG. On Thu, 26 May 2016, David Vrabel wrote: On 17/05/16 16:11, David Vrabel wrote: On 11/05/16 11:16, David Vrabel wrote: Why don't we get the RW bits correct when making the pteval when we already have the pfn, instead trying to fix it up afterwards. Kevin, can you try this patch. David 8<- x86/xen: avoid m2p lookup when setting early page table entries When page tables entries are set using xen_set_pte_init() during early boot there is no page fault handler that could handle a fault when performing an M2P lookup. In 64 guest (usually dom0) early_ioremap() would fault in xen_set_pte_init() because an M2P lookup faults because the MFN is in MMIO space and not mapped in the M2P. This lookup is done to see if the PFN in in the range used for the initial page table pages, so that the PTE may be set as read-only. The M2P lookup can be avoided by moving the check (and clear of RW) earlier when the PFN is still available. [ Not entirely happy with this as the 32/64 bit paths diverge even more. Is there some way to unify them instead? ] Boris, Juergen, any opinion on this patch? David --- a/arch/x86/xen/mmu.c +++ b/arch/x86/xen/mmu.c @@ -1562,7 +1562,7 @@ static pte_t __init mask_rw_pte(pte_t *ptep, pte_t pte) return pte; } #else /* CONFIG_X86_64 */ -static pte_t __init mask_rw_pte(pte_t *ptep, pte_t pte) +static pteval_t __init mask_rw_pte(pteval_t pte) { unsigned long pfn; @@ -1577,10 +1577,10 @@ static pte_t __init mask_rw_pte(pte_t *ptep, pte_t pte) * page tables for mapping the p2m list, too, and page
Re: [Xen-devel] [PATCH v2] arm/acpi: Add Server Base System Architecture UART support
Hi Andre, On 01/06/16 16:56, Andre Przywara wrote: I would like to support both the interfaces (ACPI_DBG2_SBSA_32/ACPI_DBG2_SBSA) If you are okay. I am a bit puzzled, so your UART is only supporting 32-bit access (i.e no 16-bit and 8-bit access)? Just to clarify: the SBSA spec is not very clear in this respect, as it only speaks of "a set of 32-bit registers". But this has been interpreted as "supports 32-bit accesses". So there was a patch lately in Linux to change all accesses to SBSA UARTs to 32-bit accessors (writel/readl), because there is at least this one mentioned platform that requires this, while all the other relevant platforms we could get hold of can also cope with 32-bit accesses. This may not be true for all legacy PL011 implementations out there, but for the SBSA subset this is deemed a safe assumption. Thank you for the explanation. So if possible we should switch to 32-bit accessors for the SBSA UART. The driver use 32-bit accessors exclusively. If so your platform is based on SBSA v2.3, and therefore the PL011 driver needs more modification to support properly your platform. For instance, the register UARTMIS is not present in v2.3 but used by the driver. I think this has been changed in the spec lately, it was present in earlier revisions of the spec. If so, I would replace the read to UARTMIS by a combination UARTIMSC and UARTRIS to avoid any issue with the UART based on SBSA v2.3. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator
>>> On 01.06.16 at 17:58,wrote: > On Fri, May 27, 2016 at 02:37:06AM -0600, Jan Beulich wrote: >> >>> On 25.05.16 at 21:48, wrote: >> > On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote: >> >> >>> On 15.04.16 at 14:33, wrote: >> >> > There is a problem with place_string() which is used as early memory >> >> > allocator. It gets memory chunks starting from start symbol and >> >> > going down. Sadly this does not work when Xen is loaded using multiboot2 >> >> > protocol because start lives on 1 MiB address. So, I tried to use >> >> > mem_lower address calculated by GRUB2. However, it works only on some >> >> > machines. There are machines in the wild (e.g. Dell PowerEdge R820) >> >> > which uses first ~640 KiB for boot services code or data... :-((( >> >> > >> >> > In case of multiboot2 protocol we need that place_string() only allocate >> >> > memory chunk for EFI memory map. However, I think that it should be >> >> > fixed >> >> > instead of making another function used just in one case. I thought >> >> > about >> >> > two solutions. >> >> > >> >> > 1) We could use native EFI allocation functions (e.g. AllocatePool() >> >> >or AllocatePages()) to get memory chunk. However, later (somewhere >> >> >in __start_xen()) we must copy its contents to safe place or reserve >> >> >this in e820 memory map and map it in Xen virtual address space. >> >> >In later case we must also care about conflicts with e.g. crash >> >> >kernel regions which could be quite difficult. >> >> >> >> I don't see why that would be: Simply use an allocation type that >> >> doesn't lead to the area getting consumed as normal RAM. Nor do >> >> I see the kexec collision potential. Furthermore (and I think I've >> >> said so before) ARM is already using AllocatePool() - just with an >> >> unsuitable memory type -, so doing so on x86 too would allow for >> > >> > Nope, they are using standard EfiLoaderData. >> >> Note how I said "just with an unsuitable memory type"? > > Could you be more precise? What else do you need? Just have the arch specify the memory type to be used (if ARM really _means_ to use that seemingly wrong type), and make the rest of the code common. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator
On Fri, May 27, 2016 at 02:37:06AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 21:48,wrote: > > On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: [...] > >> > Jan Beulich added 1b) Do away with efi_arch_allocate_mmap_buffer() and > >> > use > >> >AllocatePages() uniformly, perhaps with a per-arch specified memory > >> > type > >> >(by means of which you can control whether the memory contents will > >> > remain > >> >preserved until the time you want to look at it). That will eliminate > >> > the > >> >only place_string() you're concerned about, with a patch with better > >> >diffstat (largely due to the questionable arch hook gone). > >> > > >> > However, this solution does not solve conflicts problem described in #1 > >> > because EFI memory map is needed during Xen runtime after init phase. > >> > So, finally we would get back to #1. Hmmm... Should I check how Linux > >> > and others cope with that problem? > >> > >> Ah, here you mention it actually. Yet you don't explain what conflict > >> potential you see once using EfiRuntimeServicesData for the allocation. > > > > Good point! IMO, if crash kernel region conflicts with EfiRuntimeServices* > > then we should display warning that it cannot be allocated. By the way, > > once you mentioned that you have in your queue (I suppose that it is > > extremely long) kdump patch which adds functionality to automatically > > establish crash kernel region placement. I think that could solve (at > > least partially) problem with conflicts. Could you post it? > > For one, unless asked to be at a specific location, we already dynamically > place that area. The patch that I believe you think of just enhances the Hmmm... I do not know why I always thought that it is not supported in Xen. > placement to have something between "fully dynamic" and "at a fixed > place". I've never posted it (or even ported it to -unstable) because I've > never got positive feedback on it by those who it was originally created > for. If you think it could be useful, I can certainly revive it. Once again, thanks for doing that. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 11/16] efi: build xen.gz with EFI code
>>> On 01.06.16 at 17:48,wrote: > On Fri, May 27, 2016 at 02:31:52AM -0600, Jan Beulich wrote: >> >>> On 25.05.16 at 21:07, wrote: >> > On Wed, May 25, 2016 at 01:53:31AM -0600, Jan Beulich wrote: >> >> >>> On 15.04.16 at 14:33, wrote: >> >> > --- a/xen/common/efi/boot.c >> >> > +++ b/xen/common/efi/boot.c >> >> > @@ -1244,6 +1244,9 @@ void __init efi_init_memory(void) >> >> > } *extra, *extra_head = NULL; >> >> > #endif >> >> > >> >> > +if ( !efi_enabled(EFI_PLATFORM) ) >> >> > +return; >> >> >> >> Arguably such checks would then better be put at the call site, >> >> allowing the respective stubs to just BUG(). >> > >> > Ugh... I am confused. Here >> > http://lists.xen.org/archives/html/xen-devel/2015-08/msg01790.html >> > you asked for what is done above. So, what is your final decision? >> >> Well, in v2 you didn't alter stubs.c at all. It's that connection >> which makes me think using that earlier approach might be better. >> The more that, from a purely abstract pov, it could even allow to >> remove some or all of stubs.c in a truly non-EFI build, provided we >> never build with -O0. > > I am not sure why "provided we never build with -O0". Because a minimal amount of optimization is necessary for dead calls to actually get eliminated. >> >> Also - what's your rule for where to put such efi_enabled() checks? >> >> I would have expected them to get added to everything that has >> >> a counterpart in stubs.c, but things like efi_get_time() or >> >> efi_{halt,reset}_system() don't get any added. If those are >> >> unreachable, I'd at least expect respective ASSERT()s to get added >> >> there. >> > >> > I have added checks to functions which are called from common EFI/BIOS > code. >> >> And how are the ones I named not called from "common" code? > > efi_get_time() call is protected by "if ( efi_enabled(EFI_PLATFORM) )" > in xen/arch/x86/time.c. efi_halt_system() is called from nowhere, so, > it can be removed. I will do that. Please don't. Instead it should get wired up properly (in machine_halt()). > efi_reset_system() call is protected > by different means but EFI related. Where is that being protected? Nothing prevents anyone to boot with "reboot=efi" on a non-EFI system. That's silly, but shouldn't result in a crash during reboot. Right now its stub is intentionally doing nothing (instead of BUG()ing). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator
On Fri, May 27, 2016 at 02:37:06AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 21:48,wrote: > > On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: > >> > There is a problem with place_string() which is used as early memory > >> > allocator. It gets memory chunks starting from start symbol and > >> > going down. Sadly this does not work when Xen is loaded using multiboot2 > >> > protocol because start lives on 1 MiB address. So, I tried to use > >> > mem_lower address calculated by GRUB2. However, it works only on some > >> > machines. There are machines in the wild (e.g. Dell PowerEdge R820) > >> > which uses first ~640 KiB for boot services code or data... :-((( > >> > > >> > In case of multiboot2 protocol we need that place_string() only allocate > >> > memory chunk for EFI memory map. However, I think that it should be fixed > >> > instead of making another function used just in one case. I thought about > >> > two solutions. > >> > > >> > 1) We could use native EFI allocation functions (e.g. AllocatePool() > >> >or AllocatePages()) to get memory chunk. However, later (somewhere > >> >in __start_xen()) we must copy its contents to safe place or reserve > >> >this in e820 memory map and map it in Xen virtual address space. > >> >In later case we must also care about conflicts with e.g. crash > >> >kernel regions which could be quite difficult. > >> > >> I don't see why that would be: Simply use an allocation type that > >> doesn't lead to the area getting consumed as normal RAM. Nor do > >> I see the kexec collision potential. Furthermore (and I think I've > >> said so before) ARM is already using AllocatePool() - just with an > >> unsuitable memory type -, so doing so on x86 too would allow for > > > > Nope, they are using standard EfiLoaderData. > > Note how I said "just with an unsuitable memory type"? Could you be more precise? Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] arm/acpi: Add Server Base System Architecture UART support
Hi, On 01/06/16 16:40, Julien Grall wrote: > Hello Shanker, > > On 01/06/16 16:14, Shanker Donthineni wrote: >> >> >> On 06/01/2016 08:49 AM, Julien Grall wrote: >>> On 31/05/16 15:02, Shanker Donthineni wrote: The ARM Server Base System Architecture describes a generic UART interface. It doesn't support clock control registers, modem control, DMA and hardware flow control features. So, extend the driver probe() to handle SBSA interface and skip the accessing PL011 registers that are not described in SBSA document. >>> >>> Please mention the version of the spec in the commit message. >>> >> Sure, I'll do. Signed-off-by: Shanker Donthineni--- Changes since v1: Don't access UART registers that are not part of SBSA document. Move setting baudrate function to a separate function. xen/drivers/char/pl011.c | 56 >>> +++- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c index 1212d5c..b57f3b0 100644 --- a/xen/drivers/char/pl011.c +++ b/xen/drivers/char/pl011.c @@ -41,6 +41,7 @@ static struct pl011 { /* struct timer timer; */ /* unsigned int timeout_ms; */ /* bool_t probing, intr_works; */ +bool sbsa; /* ARM SBSA generic interface */ } pl011_com = {0}; /* These parity settings can be ORed directly into the LCR. */ @@ -81,17 +82,10 @@ static void pl011_interrupt(int irq, void *data, >>> struct cpu_user_regs *regs) } } -static void __init pl011_init_preirq(struct serial_port *port) +static void __init pl011_init_baudrate(struct serial_port *port) { struct pl011 *uart = port->uart; unsigned int divisor; -unsigned int cr; - -/* No interrupts, please. */ -pl011_write(uart, IMSC, 0); - -/* Definitely no DMA */ -pl011_write(uart, DMACR, 0x0); /* Line control and baud-rate generator. */ if ( uart->baud != BAUD_AUTO ) @@ -114,6 +108,24 @@ static void __init pl011_init_preirq(struct >>> serial_port *port) | FEN | ((uart->stop_bits - 1) << 3) | uart->parity); +} >>> >>> As mentioned on the previous version, the code to set/read the >>> baudrate is just wrong. The clock frequency is hardcoded rather than >>> read from the firmware. >>> >>> However, the baudrate is always set to BAUD_AUTO for this driver, and >>> never used after. So all this code should be dropped. >>> >> SPCR (non-SBSA) spec has baudrate, stop and parity information, I >> think we should support setting the correct baudrate according to SPCR >> table instead of removing the code. >> I need your opinion on this. > > Which is not useful because we don't have the clock frequency in hand to > compute the divisor. > > For ACPI, it just not available in the SPCR. For Device-Tree, we would > need to parse the list of clocks which could be cumbersome. > > I think it is fine to expect the firmware to configure the UART baudrate > properly if required. > >> @@ -315,6 +331,7 @@ static int __init pl011_acpi_uart_init(const void >>> *data) { acpi_status status; struct acpi_table_spcr *spcr = NULL; +bool sbsa; int res; status = acpi_get_table(ACPI_SIG_SPCR, 0, @@ -326,17 +343,21 @@ static int __init pl011_acpi_uart_init(const void >>> *data) return -EINVAL; } +sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32) ? true : false; >>> >>> sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32); >>> >>> However, can you explain why you kept ACPI_DBG2_SBSA_32 and not >>> ACPI_DBG2_SBSA? The former is deprecated whilst the latter is the >>> official one. >>> >> Qualcomm Technologies QDF2XXX ARM SBSA serial port hardware require >> registers access should be 32-bit, so our firmware sets interface type >> to ACPI_DBG2_SBSA_32. >> >> I would like to support both the interfaces >> (ACPI_DBG2_SBSA_32/ACPI_DBG2_SBSA) If you are okay. > > I am a bit puzzled, so your UART is only supporting 32-bit access (i.e > no 16-bit and 8-bit access)? Just to clarify: the SBSA spec is not very clear in this respect, as it only speaks of "a set of 32-bit registers". But this has been interpreted as "supports 32-bit accesses". So there was a patch lately in Linux to change all accesses to SBSA UARTs to 32-bit accessors (writel/readl), because there is at least this one mentioned platform that requires this, while all the other relevant platforms we could get hold of can also cope with 32-bit accesses. This may not be true for all legacy PL011 implementations out there, but for the SBSA subset this
Re: [Xen-devel] [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software
On Wed, Jun 01, 2016 at 09:50:10AM -0600, Jan Beulich wrote: > >>> On 01.06.16 at 17:35,wrote: > > On 01/06/16 16:06, Jan Beulich wrote: > >> @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > >> *eax = *ebx = *ecx = *edx = 0; > >> break; > >> } > >> -/* EBX value of main leaf 0 depends on enabled xsave features */ > >> -if ( count == 0 && v->arch.xcr0 ) > >> -{ > >> -/* reset EBX to default value first */ > >> -*ebx = XSTATE_AREA_MIN_SIZE; > >> -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > >> -{ > >> -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > >> -continue; > >> -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > >> - &_edx); > >> -if ( (_eax + _ebx) > *ebx ) > >> -*ebx = _eax + _ebx; > >> -} > >> -} > >> - > >> -if ( count == 1 ) > >> +switch ( count ) > >> { > >> +case 1: > >> *eax &= hvm_featureset[FEATURESET_Da1]; > >> - > >> -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > >> +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > >> { > >> -uint64_t xfeatures = v->arch.xcr0 | > >> v->arch.hvm_vcpu.msr_xss; > >> - > >> -*ebx = XSTATE_AREA_MIN_SIZE; > >> -if ( xfeatures & ~XSTATE_FP_SSE ) > >> -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > >> -if ( xfeatures & (1ULL << sub_leaf) ) > >> -{ > >> -if ( test_bit(sub_leaf, _align) ) > >> -*ebx = ROUNDUP(*ebx, 64); > >> -*ebx += xstate_sizes[sub_leaf]; > >> -} > >> -} > >> -else > >> *ebx = *ecx = *edx = 0; > >> +break; > >> +} > >> +/* fall through */ > >> +case 0: > >> +/* > >> + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather > >> than > >> + * domain policy. It varies with enabled xstate, and the > >> correct > >> + * xcr0/xss are in context. > >> + */ > >> +cpuid_count(input, count, , ebx, , ); > >> +break; > > > > It would be helpful for my PKU bugfix if you could avoid collapsing this > > into a fallthough, as the fallthough would need to be undone. > > Otherwise, Reviewed-by: Andrew Cooper > > Converting this back is easy to do, but I'll nevertheless wait for > Wei's opinion re 4.7 inclusion, as otherwise I'll eventually need to > rebase on top of yours anyway. > I think this is fine for 4.7. And I will leave it to you two to coordinate the rest. Wei. > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config()
On Wed, Jun 01, 2016 at 04:48:47PM +0100, Ian Jackson wrote: > Wei Liu writes ("[Xen-devel] [PATCH for-4.7] libxl: keep PoD target > adjustment by memory fudge after reload_domain_config()"): > > From: Vitaly Kuznetsov> > > > Commit 56fb5fd623 ("libxl: adjust PoD target by memory fudge, too") > > introduced target_memkb adjustment for HVM PoD domains on create. The > > adjustment is however being reset on reload_domain_config() (e.g. when > > we reboot the guest). For example: > > With George's revised commit message this patch makes sense for 4.7. > > I would like to see this function retain the name "fudge" though: > > > -ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - info->video_memkb > > -- mem_target_fudge); > > +ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - > > +libxl__get_targetmem_difference(gc, info)); > > ie: > > +ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - > +libxl__get_targetmem_fudge(gc, info)); > > This makes it clear that there is still a problem here (and it will > help things like "git grep -G"). > > With that name changed, and George's commit message, you may put my > ack on this. > > Thanks everyone. Updated patch here and I will push it shortly. ---8<--- From b9d63c2da58471a767852ab68111d245ee8d195f Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Wed, 3 Feb 2016 16:53:03 +0100 Subject: [PATCH] libxl: keep PoD target adjustment by memory fudge after reload_domain_config() Commit 56fb5fd623 ("libxl: adjust PoD target by memory fudge, too") introduced target_memkb adjustment for HVM PoD domains on create, wherein the value it wrote to target is always 1MiB lower than the actual target_memkb. Unfortunately, on reboot, it is this value which is read *unmodified* to feed into the next domain creation; from which 1MiB is subtracted *again*. This means that any guest which reboots with memory < maxmem will have its memory target decreased by 1MiB on every boot. This patch makes it so that when reading target on reboot, we adjust the value we read *up* by 1MiB, so that the domain will be build with the appropriate amount of memory and the target will remain the same after reboot. This is still not quite a complete fix, as the 1MiB offset is only subtracted when creating or rebooting; it is not subtracted when 'xl set-memory' is called. But it will prevent any situations where memory is continually increased or decreased. A better fix will have to wait until after the release. Signed-off-by: Vitaly Kuznetsov Signed-off-by: Wei Liu Acked-by: Ian Jackson --- tools/libxl/libxl.c | 8 tools/libxl/libxl_dom.c | 10 ++ tools/libxl/libxl_internal.h | 15 +++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index b9d855b..306984b 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -7229,12 +7229,12 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid, LOG(ERROR, "fail to get memory target for domain %d", domid); goto out; } -/* Target memory in xenstore is different from what user has - * asked for. The difference is video_memkb. See - * libxl_set_memory_target. + +/* libxl__get_targetmem_fudge() calculates the difference from + * what is in xenstore to what we have in the domain build info. */ d_config->b_info.target_memkb = target_memkb + -d_config->b_info.video_memkb; +libxl__get_targetmem_fudge(gc, _config->b_info); d_config->b_info.max_memkb = max_memkb; } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 9b20cf5..805774f 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -490,7 +490,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, xs_transaction_t t; char **ents; int i, rc; -int64_t mem_target_fudge; if (info->num_vnuma_nodes && !info->num_vcpu_soft_affinity) { rc = set_vnuma_affinity(gc, domid, info); @@ -523,17 +522,12 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, } } -mem_target_fudge = -(info->type == LIBXL_DOMAIN_TYPE_HVM && - info->max_memkb > info->target_memkb) -? LIBXL_MAXMEM_CONSTANT : 0; - ents = libxl__calloc(gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *)); ents[0] = "memory/static-max"; ents[1] = GCSPRINTF("%"PRId64, info->max_memkb); ents[2] = "memory/target"; -ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - info->video_memkb -- mem_target_fudge); +ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - +libxl__get_targetmem_fudge(gc, info)); ents[4] =
Re: [Xen-devel] [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software
>>> On 01.06.16 at 17:35,wrote: > On 01/06/16 16:06, Jan Beulich wrote: >> @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig >> *eax = *ebx = *ecx = *edx = 0; >> break; >> } >> -/* EBX value of main leaf 0 depends on enabled xsave features */ >> -if ( count == 0 && v->arch.xcr0 ) >> -{ >> -/* reset EBX to default value first */ >> -*ebx = XSTATE_AREA_MIN_SIZE; >> -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) >> -{ >> -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) >> -continue; >> -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, >> - &_edx); >> -if ( (_eax + _ebx) > *ebx ) >> -*ebx = _eax + _ebx; >> -} >> -} >> - >> -if ( count == 1 ) >> +switch ( count ) >> { >> +case 1: >> *eax &= hvm_featureset[FEATURESET_Da1]; >> - >> -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) >> +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) >> { >> -uint64_t xfeatures = v->arch.xcr0 | >> v->arch.hvm_vcpu.msr_xss; >> - >> -*ebx = XSTATE_AREA_MIN_SIZE; >> -if ( xfeatures & ~XSTATE_FP_SSE ) >> -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) >> -if ( xfeatures & (1ULL << sub_leaf) ) >> -{ >> -if ( test_bit(sub_leaf, _align) ) >> -*ebx = ROUNDUP(*ebx, 64); >> -*ebx += xstate_sizes[sub_leaf]; >> -} >> -} >> -else >> *ebx = *ecx = *edx = 0; >> +break; >> +} >> +/* fall through */ >> +case 0: >> +/* >> + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than >> + * domain policy. It varies with enabled xstate, and the >> correct >> + * xcr0/xss are in context. >> + */ >> +cpuid_count(input, count, , ebx, , ); >> +break; > > It would be helpful for my PKU bugfix if you could avoid collapsing this > into a fallthough, as the fallthough would need to be undone. > Otherwise, Reviewed-by: Andrew Cooper Converting this back is easy to do, but I'll nevertheless wait for Wei's opinion re 4.7 inclusion, as otherwise I'll eventually need to rebase on top of yours anyway. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config()
Wei Liu writes ("[Xen-devel] [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config()"): > From: Vitaly Kuznetsov> > Commit 56fb5fd623 ("libxl: adjust PoD target by memory fudge, too") > introduced target_memkb adjustment for HVM PoD domains on create. The > adjustment is however being reset on reload_domain_config() (e.g. when > we reboot the guest). For example: With George's revised commit message this patch makes sense for 4.7. I would like to see this function retain the name "fudge" though: > -ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - info->video_memkb > -- mem_target_fudge); > +ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - > +libxl__get_targetmem_difference(gc, info)); ie: +ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - +libxl__get_targetmem_fudge(gc, info)); This makes it clear that there is still a problem here (and it will help things like "git grep -G"). With that name changed, and George's commit message, you may put my ack on this. George writes: > better fix will have to wait until after the release. Realistically speaking, that is quite optimistic. Empirically, we keep squelching into the memory accounting swamp, but we have as yet failed to drain it. Instead our code keeps accreting bodges in this area, and the swamp keeps growing... Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 11/16] efi: build xen.gz with EFI code
On Fri, May 27, 2016 at 02:31:52AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 21:07,wrote: > > On Wed, May 25, 2016 at 01:53:31AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: [...] > >> > --- a/xen/common/efi/boot.c > >> > +++ b/xen/common/efi/boot.c > >> > @@ -1244,6 +1244,9 @@ void __init efi_init_memory(void) > >> > } *extra, *extra_head = NULL; > >> > #endif > >> > > >> > +if ( !efi_enabled(EFI_PLATFORM) ) > >> > +return; > >> > >> Arguably such checks would then better be put at the call site, > >> allowing the respective stubs to just BUG(). > > > > Ugh... I am confused. Here > > http://lists.xen.org/archives/html/xen-devel/2015-08/msg01790.html > > you asked for what is done above. So, what is your final decision? > > Well, in v2 you didn't alter stubs.c at all. It's that connection > which makes me think using that earlier approach might be better. > The more that, from a purely abstract pov, it could even allow to > remove some or all of stubs.c in a truly non-EFI build, provided we > never build with -O0. I am not sure why "provided we never build with -O0". > But in the end, starting the sens with "arguably" I mean to express > that this isn't all that important. OK. > >> Also - what's your rule for where to put such efi_enabled() checks? > >> I would have expected them to get added to everything that has > >> a counterpart in stubs.c, but things like efi_get_time() or > >> efi_{halt,reset}_system() don't get any added. If those are > >> unreachable, I'd at least expect respective ASSERT()s to get added > >> there. > > > > I have added checks to functions which are called from common EFI/BIOS code. > > And how are the ones I named not called from "common" code? efi_get_time() call is protected by "if ( efi_enabled(EFI_PLATFORM) )" in xen/arch/x86/time.c. efi_halt_system() is called from nowhere, so, it can be removed. I will do that. efi_reset_system() call is protected by different means but EFI related. So, all of them are not called from "common" code during runtime on BIOS platforms. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software
On Wed, Jun 01, 2016 at 04:35:44PM +0100, Andrew Cooper wrote: > On 01/06/16 16:06, Jan Beulich wrote: > > @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > > *eax = *ebx = *ecx = *edx = 0; > > break; > > } > > -/* EBX value of main leaf 0 depends on enabled xsave features */ > > -if ( count == 0 && v->arch.xcr0 ) > > -{ > > -/* reset EBX to default value first */ > > -*ebx = XSTATE_AREA_MIN_SIZE; > > -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > > -{ > > -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > > -continue; > > -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > > - &_edx); > > -if ( (_eax + _ebx) > *ebx ) > > -*ebx = _eax + _ebx; > > -} > > -} > > - > > -if ( count == 1 ) > > +switch ( count ) > > { > > +case 1: > > *eax &= hvm_featureset[FEATURESET_Da1]; > > - > > -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > > +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > > { > > -uint64_t xfeatures = v->arch.xcr0 | > > v->arch.hvm_vcpu.msr_xss; > > - > > -*ebx = XSTATE_AREA_MIN_SIZE; > > -if ( xfeatures & ~XSTATE_FP_SSE ) > > -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > > -if ( xfeatures & (1ULL << sub_leaf) ) > > -{ > > -if ( test_bit(sub_leaf, _align) ) > > -*ebx = ROUNDUP(*ebx, 64); > > -*ebx += xstate_sizes[sub_leaf]; > > -} > > -} > > -else > > *ebx = *ecx = *edx = 0; > > +break; > > +} > > +/* fall through */ > > +case 0: > > +/* > > + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather > > than > > + * domain policy. It varies with enabled xstate, and the > > correct > > + * xcr0/xss are in context. > > + */ > > +cpuid_count(input, count, , ebx, , ); > > +break; > > It would be helpful for my PKU bugfix if you could avoid collapsing this > into a fallthough, as the fallthough would need to be undone. > Otherwise, Reviewed-by: Andrew Cooper> Release-acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero
On Wed, Jun 01, 2016 at 09:05:56AM -0600, Jan Beulich wrote: > In line with other recent changes, these should be fully white listed, > requiring us to zero them until the obtain a meaning we support. > "they obtain ..." > Without XSAVE support, all xstate sub-leaves should be zero. > > Also move away from checking host XSAVE support - we really ought to > consider the guest flag for that purpose. > > Signed-off-by: Jan Beulich> Release-acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: xen-pciback: Remove create_workqueue
On Wed, Jun 01, 2016 at 07:45:08PM +0530, Bhaktipriya Shridhar wrote: > System workqueues have been able to handle high level of concurrency > for a long time now and there's no reason to use dedicated workqueues > just to gain concurrency. Replace dedicated xen_pcibk_wq with the > use of system_wq. > > Unlike a dedicated per-cpu workqueue created with create_workqueue(), > system_wq allows multiple work items to overlap executions even on > the same CPU; however, a per-cpu workqueue doesn't have any CPU > locality or global ordering guarantees unless the target CPU is > explicitly specified and thus the increase of local concurrency shouldn't > make any difference. > > Since the work items could be pending, flush_work() has been used in > xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev() > which in turn calls xen_pcibk_disconnect() for every pdev to ensure that > there is no pending task while disconnecting the driver. > > Signed-off-by: Bhaktipriya ShridharAcked-by: Tejun Heo Thanks. -- tejun ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/8] monitor: ARM SMC events
On Jun 1, 2016 05:37, "Julien Grall"wrote: > > Hi Tamas, > > > On 29/05/16 23:37, Tamas K Lengyel wrote: >> >> Add support for monitoring ARM SMC events. This patch only adds the required >> bits to enable/disable monitoring and forwarding the event through vm_event. > > > Couple of questions: > > - How the vm-event app will differentiate a SMC64 vs a SMC32 call? Wouldn't cpsr record the guest state when the smc was executed, telling us whether it was 32bit or 64bit? > - How the vm-event app will know the SMC number (i.e the > > In an AArch64 state, those informations are available from the syndrome register (HSR_EL2.ISS). Good question, it should probably be sent alongside the other registers. At the moment my usecase doesn't care about it as I'm just using SMC for inducing traps to the vmm but other applications may indeed need this. Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 10/16] efi: create efi_enabled()
>>> On 01.06.16 at 17:23,wrote: > On Fri, May 27, 2016 at 02:22:39AM -0600, Jan Beulich wrote: >> >>> On 25.05.16 at 19:15, wrote: >> > On Wed, May 25, 2016 at 01:20:23AM -0600, Jan Beulich wrote: >> >> >>> On 15.04.16 at 14:33, wrote: > > [...] > >> >> > --- a/xen/include/xen/efi.h >> >> > +++ b/xen/include/xen/efi.h >> >> > @@ -2,15 +2,17 @@ >> >> > #define __XEN_EFI_H__ >> >> > >> >> > #ifndef __ASSEMBLY__ >> >> > +#include >> >> > #include >> >> > #endif >> >> > >> >> > -extern const bool_t efi_enabled; >> >> > - >> >> > #define EFI_INVALID_TABLE_ADDR (~0UL) >> >> > >> >> > +#define EFI_PLATFORM 0 >> >> >> >> So what does "platform" mean? Did you consider using the more fine >> > >> > It means "EFI platform". It differentiates from "legacy BIOS platform". >> >> Well, that's what was clear from the beginning. The question however >> was (taken together with the second one) what it means functionality >> wise. The later addition makes clear it doesn't mean "loaded directly > > This means that we run on EFI platform and we can use its features, > e.g. runtime services, get info from it about ACPI, SMBIOS, etc. > >> from EFI". But looking at the various flags Linux has here, what > > Yep. > >> functionality does it imply? Does it e.g. mean runtime services are to >> be used? If so, the flag would need to be cleared when their use if > > As above: not only. I.e. we're back at me asking you to make this at least a little more fine grained. >> being suppressed. > > If we need that (e.g. for ARM) then we should create e.g. EFI_RS. Why only then? We already can suppress the use of runtime services. >> >> grained set of flags Linux uses nowadays? That would also eliminate >> > >> > I wish to use just basic idea. However, I am not going to copy all >> > stuff from Linux. We do not need that. >> >> We don't need all of it, sure. But some more fine grained >> identification of what functionality is available / to be used >> would surely benefit us as a whole and your patch series in >> particular. > > As above. Well, above you don't really reason on why this coarse granularity is good enough. Hence my response can only be: As above. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software
On 01/06/16 16:06, Jan Beulich wrote: > @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > *eax = *ebx = *ecx = *edx = 0; > break; > } > -/* EBX value of main leaf 0 depends on enabled xsave features */ > -if ( count == 0 && v->arch.xcr0 ) > -{ > -/* reset EBX to default value first */ > -*ebx = XSTATE_AREA_MIN_SIZE; > -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > -{ > -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > -continue; > -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > - &_edx); > -if ( (_eax + _ebx) > *ebx ) > -*ebx = _eax + _ebx; > -} > -} > - > -if ( count == 1 ) > +switch ( count ) > { > +case 1: > *eax &= hvm_featureset[FEATURESET_Da1]; > - > -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > { > -uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; > - > -*ebx = XSTATE_AREA_MIN_SIZE; > -if ( xfeatures & ~XSTATE_FP_SSE ) > -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > -if ( xfeatures & (1ULL << sub_leaf) ) > -{ > -if ( test_bit(sub_leaf, _align) ) > -*ebx = ROUNDUP(*ebx, 64); > -*ebx += xstate_sizes[sub_leaf]; > -} > -} > -else > *ebx = *ecx = *edx = 0; > +break; > +} > +/* fall through */ > +case 0: > +/* > + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than > + * domain policy. It varies with enabled xstate, and the correct > + * xcr0/xss are in context. > + */ > +cpuid_count(input, count, , ebx, , ); > +break; It would be helpful for my PKU bugfix if you could avoid collapsing this into a fallthough, as the fallthough would need to be undone. Otherwise, Reviewed-by: Andrew Cooper~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] arm/acpi: Add Server Base System Architecture UART support
Hello Shanker, On 01/06/16 16:14, Shanker Donthineni wrote: On 06/01/2016 08:49 AM, Julien Grall wrote: On 31/05/16 15:02, Shanker Donthineni wrote: The ARM Server Base System Architecture describes a generic UART interface. It doesn't support clock control registers, modem control, DMA and hardware flow control features. So, extend the driver probe() to handle SBSA interface and skip the accessing PL011 registers that are not described in SBSA document. Please mention the version of the spec in the commit message. Sure, I'll do. Signed-off-by: Shanker Donthineni--- Changes since v1: Don't access UART registers that are not part of SBSA document. Move setting baudrate function to a separate function. xen/drivers/char/pl011.c | 56 +++- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c index 1212d5c..b57f3b0 100644 --- a/xen/drivers/char/pl011.c +++ b/xen/drivers/char/pl011.c @@ -41,6 +41,7 @@ static struct pl011 { /* struct timer timer; */ /* unsigned int timeout_ms; */ /* bool_t probing, intr_works; */ +bool sbsa; /* ARM SBSA generic interface */ } pl011_com = {0}; /* These parity settings can be ORed directly into the LCR. */ @@ -81,17 +82,10 @@ static void pl011_interrupt(int irq, void *data, struct cpu_user_regs *regs) } } -static void __init pl011_init_preirq(struct serial_port *port) +static void __init pl011_init_baudrate(struct serial_port *port) { struct pl011 *uart = port->uart; unsigned int divisor; -unsigned int cr; - -/* No interrupts, please. */ -pl011_write(uart, IMSC, 0); - -/* Definitely no DMA */ -pl011_write(uart, DMACR, 0x0); /* Line control and baud-rate generator. */ if ( uart->baud != BAUD_AUTO ) @@ -114,6 +108,24 @@ static void __init pl011_init_preirq(struct serial_port *port) | FEN | ((uart->stop_bits - 1) << 3) | uart->parity); +} As mentioned on the previous version, the code to set/read the baudrate is just wrong. The clock frequency is hardcoded rather than read from the firmware. However, the baudrate is always set to BAUD_AUTO for this driver, and never used after. So all this code should be dropped. SPCR (non-SBSA) spec has baudrate, stop and parity information, I think we should support setting the correct baudrate according to SPCR table instead of removing the code. I need your opinion on this. Which is not useful because we don't have the clock frequency in hand to compute the divisor. For ACPI, it just not available in the SPCR. For Device-Tree, we would need to parse the list of clocks which could be cumbersome. I think it is fine to expect the firmware to configure the UART baudrate properly if required. @@ -315,6 +331,7 @@ static int __init pl011_acpi_uart_init(const void *data) { acpi_status status; struct acpi_table_spcr *spcr = NULL; +bool sbsa; int res; status = acpi_get_table(ACPI_SIG_SPCR, 0, @@ -326,17 +343,21 @@ static int __init pl011_acpi_uart_init(const void *data) return -EINVAL; } +sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32) ? true : false; sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32); However, can you explain why you kept ACPI_DBG2_SBSA_32 and not ACPI_DBG2_SBSA? The former is deprecated whilst the latter is the official one. Qualcomm Technologies QDF2XXX ARM SBSA serial port hardware require registers access should be 32-bit, so our firmware sets interface type to ACPI_DBG2_SBSA_32. I would like to support both the interfaces (ACPI_DBG2_SBSA_32/ACPI_DBG2_SBSA) If you are okay. I am a bit puzzled, so your UART is only supporting 32-bit access (i.e no 16-bit and 8-bit access)? If so your platform is based on SBSA v2.3, and therefore the PL011 driver needs more modification to support properly your platform. For instance, the register UARTMIS is not present in v2.3 but used by the driver. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch v6 09/11] vt-d: fix the IOMMU flush issue
>>> On 31.05.16 at 15:57,wrote: > @@ -1404,13 +1438,35 @@ int domain_context_mapping_one( > spin_unlock(>lock); > > /* Context entry was previously non-present (with domid 0). */ > -if ( iommu_flush_context_device(iommu, 0, (((u16)bus) << 8) | devfn, > -DMA_CCMD_MASK_NOBIT, 1) ) > -iommu_flush_write_buffer(iommu); > -else > +rc = iommu_flush_context_device(iommu, 0, PCI_BDF2(bus, devfn), > +DMA_CCMD_MASK_NOBIT, 1); > + > +/* > + * The current logic for rc returns: > + * - positive invoke iommu_flush_write_buffer to flush cache. > + * - zero on success. > + * - negative on failure. Continue to flush IOMMU IOTLB on a > + * best effort basis. > + * > + * Moreover, IOMMU flush handlers flush_context_qi or flush_iotlb_qi > + * (or flush_context_reg and flush_iotlb_reg, deep functions in the > + * call trees of iommu_flush_context_device and iommu_flush_iotlb_dsi) > + * are with the same logic to bubble up positive return value. > + */ > +if ( rc <= 0 ) > { > int flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0; > -iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb); > +int ret; > + > +ret = iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb); Please make this the initializer again (at least one more such case further down). > @@ -1535,6 +1592,7 @@ int domain_context_unmap_one( > iommu_flush_cache_entry(context, sizeof(struct context_entry)); > > iommu_domid= domain_iommu_domid(domain, iommu); > + > if ( iommu_domid == -1 ) Once again a stray addition of a blank line, contradicting point 1 of your v6 list of changes. Please actually _look_ at your patches before sending them out. > @@ -1542,14 +1600,36 @@ int domain_context_unmap_one( > return -EINVAL; > } > > -if ( iommu_flush_context_device(iommu, iommu_domid, > -(((u16)bus) << 8) | devfn, > -DMA_CCMD_MASK_NOBIT, 0) ) > -iommu_flush_write_buffer(iommu); > -else > +rc = iommu_flush_context_device(iommu, iommu_domid, > +PCI_BDF2(bus, devfn), > +DMA_CCMD_MASK_NOBIT, 0); > + > +/* > + * The current logic for rc returns: > + * - positive invoke iommu_flush_write_buffer to flush cache. > + * - zero on success. > + * - negative on failure. Continue to flush IOMMU IOTLB on a > + * best effort basis. > + * > + * Moreover, IOMMU flush handlers flush_context_qi or flush_iotlb_qi > + * (or flush_context_reg and flush_iotlb_reg, deep functions in the > + * call trees of iommu_flush_context_device and iommu_flush_iotlb_dsi) > + * are with the same logic to bubble up positive return value. > + */ This is the 3rd instance of that comment. I'd prefer the latter ones to simply refer to the first one, but I'll obviously leave it to the maintainers to decide. With those cosmetic issues taken care of Reviewed-by: Jen Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 4.7 qemu regression: HVM guests fail to boot from xvda
George Dunlap writes ("Re: [Xen-devel] 4.7 qemu regression: HVM guests fail to boot from xvda"): > On Tue, May 31, 2016 at 12:41 PM, Olaf Heringwrote: > > I think if some domU.cfg for xen-4.3+ has 'vdev=xvd*' and the domU uses > > for some reason kernel names in config files and the domU uses a > > xenlinux kernel, then changing domU.cfg to 'hd*' will allow the guest to > > boot again. But its userland will miss the /dev/xvd* device nodes. > > That probably remained unnoticed during testing the referenced commit if > > a pvops based kernel was used. Linux doctrine, at least, nowadays, is that hd* device names are not stable. On my own colo machine I find that on some boots the actual hard disks are sda and sdb and the emergency usb stick is sdc, but on other boots the disks are sdb and sdc and the usb stick is sda. So some would say that what you are doing is inherently unstable. But I don't think I really agree with that view. I think we should be able to do better. > Really 'vdev' string in the the guest config file is only meant to > tell libxl how it should behave -- it should ideally not have any > effect on what devices you see in the backend. The vdev specifies the virtual block number. See vbd-interface.txt. hda and xvda have different numbers. > And furthermore, it > seems to me that when Linux upstream rejected the idea of the pv > drivers stealing the "hd*" namespace, that SuSE's xenlinux should have > followed suit and had the pv drivers only create devices named xvd*. Right. This is the root cause. vbd-interface.txt is designed to cope with modern PV frontends which never steal the hda minor numbers. I think we should fix this with a syntax for explicitly specifying a pv-only device with the hd* pv block number. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [seabios test] 95124: tolerable FAIL - PUSHED
flight 95124 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/95124/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 94522 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94522 Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass version targeted for testing: seabios 04259c5817edc6d23f0aed76fd20ab220efcddc6 baseline version: seabios 1dc77745774ff7ba95a0c4dc8eb2299a6cde4d98 Last test of basis94522 2016-05-17 15:14:24 Z 15 days Testing same since95124 2016-06-01 09:43:00 Z0 days1 attempts People who touched revisions under test: Alex WilliamsonGerd Hoffmann 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-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-qemuu-nested-intel pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass test-amd64-amd64-xl-qemuu-winxpsp3 pass test-amd64-i386-xl-qemuu-winxpsp3pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=seabios + revision=04259c5817edc6d23f0aed76fd20ab220efcddc6 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push seabios 04259c5817edc6d23f0aed76fd20ab220efcddc6 + branch=seabios + revision=04259c5817edc6d23f0aed76fd20ab220efcddc6 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=seabios + xenbranch=xen-unstable + '[' xseabios =
Re: [Xen-devel] [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero
On 01/06/16 16:05, Jan Beulich wrote: > In line with other recent changes, these should be fully white listed, > requiring us to zero them until the obtain a meaning we support. > > Without XSAVE support, all xstate sub-leaves should be zero. > > Also move away from checking host XSAVE support - we really ought to > consider the guest flag for that purpose. > > Signed-off-by: Jan BeulichReviewed-by: Andrew Cooper , with one suggestion > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig > *edx = v->vcpu_id * 2; > break; > > -case 0xd: > +case XSTATE_CPUID: > +hvm_cpuid(1, NULL, NULL, &_ecx, NULL); > +if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) > +{ > +*eax = *ebx = *ecx = *edx = 0; > +break; > +} > /* EBX value of main leaf 0 depends on enabled xsave features */ > if ( count == 0 && v->arch.xcr0 ) > { > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs > > switch ( leaf ) > { > +uint32_t tmp; > + > case 0x0001: > c &= pv_featureset[FEATURESET_1c]; > d &= pv_featureset[FEATURESET_1d]; > @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs > break; > > case XSTATE_CPUID: > -if ( !cpu_has_xsave ) > +if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) I would recommend extra brackets on this line, to avoid the possible mis-interpretation of !is_control_domain(currd) && (!is_hardware_domain(currd) ? ... > +? ({ > +uint32_t ecx; > + > +domain_cpuid(currd, 1, 0, , , , ); > +ecx & pv_featureset[FEATURESET_1c]; > + }) > +: cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || > + subleaf >= 63 ) This is rather nasty code. I am glad that my longterm plans involve removing it all. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.3-testing test] 95131: trouble: blocked/broken
flight 95131 qemu-upstream-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/95131/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 3 host-install(3) broken REGR. vs. 80927 build-i386-pvops 3 host-install(3) broken REGR. vs. 80927 build-i3863 host-install(3) broken REGR. vs. 80927 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927 Tests which did not succeed, but are not blocking: build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-pv1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pv 1 build-check(1) blocked n/a version targeted for testing: qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1 baseline version: qemuu10c1b763c26feb645627a1639e722515f3e1e876 Last test of basis80927 2016-02-06 13:30:02 Z 116 days Testing same since93977 2016-05-10 11:09:16 Z 22 days 95 attempts People who touched revisions under test: Gerd HoffmannStefano Stabellini jobs: build-amd64 broken build-i386 broken build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopsbroken build-i386-pvops broken test-amd64-amd64-xl blocked test-amd64-i386-xl blocked test-amd64-i386-qemuu-rhel6hvm-amd blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked test-amd64-i386-freebsd10-amd64 blocked test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked test-amd64-amd64-xl-qemuu-win7-amd64 blocked test-amd64-i386-xl-qemuu-win7-amd64 blocked test-amd64-amd64-xl-credit2 blocked test-amd64-i386-freebsd10-i386 blocked test-amd64-i386-qemuu-rhel6hvm-intel blocked test-amd64-amd64-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-xl-multivcpublocked
Re: [Xen-devel] [PATCH v3 10/16] efi: create efi_enabled()
On Fri, May 27, 2016 at 02:22:39AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 19:15,wrote: > > On Wed, May 25, 2016 at 01:20:23AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: [...] > >> > --- a/xen/include/xen/efi.h > >> > +++ b/xen/include/xen/efi.h > >> > @@ -2,15 +2,17 @@ > >> > #define __XEN_EFI_H__ > >> > > >> > #ifndef __ASSEMBLY__ > >> > +#include > >> > #include > >> > #endif > >> > > >> > -extern const bool_t efi_enabled; > >> > - > >> > #define EFI_INVALID_TABLE_ADDR (~0UL) > >> > > >> > +#define EFI_PLATFORM0 > >> > >> So what does "platform" mean? Did you consider using the more fine > > > > It means "EFI platform". It differentiates from "legacy BIOS platform". > > Well, that's what was clear from the beginning. The question however > was (taken together with the second one) what it means functionality > wise. The later addition makes clear it doesn't mean "loaded directly This means that we run on EFI platform and we can use its features, e.g. runtime services, get info from it about ACPI, SMBIOS, etc. > from EFI". But looking at the various flags Linux has here, what Yep. > functionality does it imply? Does it e.g. mean runtime services are to > be used? If so, the flag would need to be cleared when their use if As above: not only. > being suppressed. If we need that (e.g. for ARM) then we should create e.g. EFI_RS. > >> grained set of flags Linux uses nowadays? That would also eliminate > > > > I wish to use just basic idea. However, I am not going to copy all > > stuff from Linux. We do not need that. > > We don't need all of it, sure. But some more fine grained > identification of what functionality is available / to be used > would surely benefit us as a whole and your patch series in > particular. As above. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] arm/acpi: Add Server Base System Architecture UART support
On 06/01/2016 08:49 AM, Julien Grall wrote: > Hello Shanker, > > On 31/05/16 15:02, Shanker Donthineni wrote: >> The ARM Server Base System Architecture describes a generic UART >> interface. It doesn't support clock control registers, modem >> control, DMA and hardware flow control features. So, extend the >> driver probe() to handle SBSA interface and skip the accessing >> PL011 registers that are not described in SBSA document. > > Please mention the version of the spec in the commit message. > Sure, I'll do. >> Signed-off-by: Shanker Donthineni>> --- >> Changes since v1: >> Don't access UART registers that are not part of SBSA document. >> Move setting baudrate function to a separate function. >> >> xen/drivers/char/pl011.c | 56 > +++- >> 1 file changed, 41 insertions(+), 15 deletions(-) >> >> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c >> index 1212d5c..b57f3b0 100644 >> --- a/xen/drivers/char/pl011.c >> +++ b/xen/drivers/char/pl011.c >> @@ -41,6 +41,7 @@ static struct pl011 { >> /* struct timer timer; */ >> /* unsigned int timeout_ms; */ >> /* bool_t probing, intr_works; */ >> +bool sbsa; /* ARM SBSA generic interface */ >> } pl011_com = {0}; >> >> /* These parity settings can be ORed directly into the LCR. */ >> @@ -81,17 +82,10 @@ static void pl011_interrupt(int irq, void *data, > struct cpu_user_regs *regs) >> } >> } >> >> -static void __init pl011_init_preirq(struct serial_port *port) >> +static void __init pl011_init_baudrate(struct serial_port *port) >> { >> struct pl011 *uart = port->uart; >> unsigned int divisor; >> -unsigned int cr; >> - >> -/* No interrupts, please. */ >> -pl011_write(uart, IMSC, 0); >> - >> -/* Definitely no DMA */ >> -pl011_write(uart, DMACR, 0x0); >> >> /* Line control and baud-rate generator. */ >> if ( uart->baud != BAUD_AUTO ) >> @@ -114,6 +108,24 @@ static void __init pl011_init_preirq(struct > serial_port *port) >> | FEN >> | ((uart->stop_bits - 1) << 3) >> | uart->parity); >> +} > > As mentioned on the previous version, the code to set/read the baudrate is > just wrong. The clock frequency is hardcoded rather than read from the > firmware. > > However, the baudrate is always set to BAUD_AUTO for this driver, and never > used after. So all this code should be dropped. > SPCR (non-SBSA) spec has baudrate, stop and parity information, I think we should support setting the correct baudrate according to SPCR table instead of removing the code. I need your opinion on this. >> @@ -315,6 +331,7 @@ static int __init pl011_acpi_uart_init(const void > *data) >> { >> acpi_status status; >> struct acpi_table_spcr *spcr = NULL; >> +bool sbsa; >> int res; >> >> status = acpi_get_table(ACPI_SIG_SPCR, 0, >> @@ -326,17 +343,21 @@ static int __init pl011_acpi_uart_init(const void > *data) >> return -EINVAL; >> } >> >> +sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32) ? true : false; > > sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32); > > However, can you explain why you kept ACPI_DBG2_SBSA_32 and not > ACPI_DBG2_SBSA? The former is deprecated whilst the latter is the official > one. > Qualcomm Technologies QDF2XXX ARM SBSA serial port hardware require registers access should be 32-bit, so our firmware sets interface type to ACPI_DBG2_SBSA_32. I would like to support both the interfaces (ACPI_DBG2_SBSA_32/ACPI_DBG2_SBSA) If you are okay. > Regards, > -- Shanker Donthineni Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero
In line with other recent changes, these should be fully white listed, requiring us to zero them until the obtain a meaning we support. Without XSAVE support, all xstate sub-leaves should be zero. Also move away from checking host XSAVE support - we really ought to consider the guest flag for that purpose. Signed-off-by: Jan Beulich--- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig *edx = v->vcpu_id * 2; break; -case 0xd: +case XSTATE_CPUID: +hvm_cpuid(1, NULL, NULL, &_ecx, NULL); +if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) +{ +*eax = *ebx = *ecx = *edx = 0; +break; +} /* EBX value of main leaf 0 depends on enabled xsave features */ if ( count == 0 && v->arch.xcr0 ) { --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs switch ( leaf ) { +uint32_t tmp; + case 0x0001: c &= pv_featureset[FEATURESET_1c]; d &= pv_featureset[FEATURESET_1d]; @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs break; case XSTATE_CPUID: -if ( !cpu_has_xsave ) +if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) +? ({ +uint32_t ecx; + +domain_cpuid(currd, 1, 0, , , , ); +ecx & pv_featureset[FEATURESET_1c]; + }) +: cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || + subleaf >= 63 ) goto unsupported; switch ( subleaf ) { case 0: -{ -uint32_t tmp; - /* * Always read CPUID.0xD[ECX=0].EBX from hardware, rather than * domain policy. It varies with enabled xstate, and the correct @@ -1101,7 +1108,6 @@ void pv_cpuid(struct cpu_user_regs *regs if ( !is_control_domain(currd) && !is_hardware_domain(currd) ) cpuid_count(leaf, subleaf, , , , ); break; -} case 1: a &= pv_featureset[FEATURESET_Da1]; x86: flush high xstate CPUID sub-leaves to zero In line with other recent changes, these should be fully white listed, requiring us to zero them until the obtain a meaning we support. Without XSAVE support, all xstate sub-leaves should be zero. Also move away from checking host XSAVE support - we really ought to consider the guest flag for that purpose. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig *edx = v->vcpu_id * 2; break; -case 0xd: +case XSTATE_CPUID: +hvm_cpuid(1, NULL, NULL, &_ecx, NULL); +if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) +{ +*eax = *ebx = *ecx = *edx = 0; +break; +} /* EBX value of main leaf 0 depends on enabled xsave features */ if ( count == 0 && v->arch.xcr0 ) { --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs switch ( leaf ) { +uint32_t tmp; + case 0x0001: c &= pv_featureset[FEATURESET_1c]; d &= pv_featureset[FEATURESET_1d]; @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs break; case XSTATE_CPUID: -if ( !cpu_has_xsave ) +if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) +? ({ +uint32_t ecx; + +domain_cpuid(currd, 1, 0, , , , ); +ecx & pv_featureset[FEATURESET_1c]; + }) +: cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || + subleaf >= 63 ) goto unsupported; switch ( subleaf ) { case 0: -{ -uint32_t tmp; - /* * Always read CPUID.0xD[ECX=0].EBX from hardware, rather than * domain policy. It varies with enabled xstate, and the correct @@ -1101,7 +1108,6 @@ void pv_cpuid(struct cpu_user_regs *regs if ( !is_control_domain(currd) && !is_hardware_domain(currd) ) cpuid_count(leaf, subleaf, , , , ); break; -} case 1: a &= pv_featureset[FEATURESET_Da1]; ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 09/16] efi: explicitly define efi struct in xen/arch/x86/efi/stub.c
On Fri, May 27, 2016 at 02:16:09AM -0600, Jan Beulich wrote: > >>> On 25.05.16 at 18:45,wrote: > > On Wed, May 25, 2016 at 01:03:42AM -0600, Jan Beulich wrote: > >> >>> On 15.04.16 at 14:33, wrote: > >> > Existing solution does not allocate space for this symbol and any > >> > references to acpi20, etc. does not make sense. As I saw any efi.* > >> > references are protected by relevant ifs but we should not do that > >> > because it makes code very fragile. If somebody does not know how > >> > efi symbol is created he/she may assume that it always represent > >> > valid structure and do invalid references somewhere. > >> > >> I do not view this as a valid reason for the change. > > > > Why? > > Because there are no accesses to the structure in non-EFI builds? > Even if it's just a small table, I'm generally opposed to adding dead > code or data. I simply do not like the attitude of "memory is cheap" > these days. Following that model leads to quite a bit of useless I concur! > bloat. Plus no matter whether memory is cheap, cache and TLB > bandwidth are precious, and both may get pressure added by such > dead elements. OK, but in the future please add a few words of comment in such cases because it is not obvious why just looking at code. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software
Use hardware output instead, brining HVM behavior in line with PV one in this regard. Signed-off-by: Jan Beulich--- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3362,7 +3362,7 @@ void hvm_cpuid(unsigned int input, unsig switch ( input ) { -unsigned int sub_leaf, _eax, _ebx, _ecx, _edx; +unsigned int _ecx, _edx; case 0x1: /* Fix up VLAPIC details. */ @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig *eax = *ebx = *ecx = *edx = 0; break; } -/* EBX value of main leaf 0 depends on enabled xsave features */ -if ( count == 0 && v->arch.xcr0 ) -{ -/* reset EBX to default value first */ -*ebx = XSTATE_AREA_MIN_SIZE; -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) -{ -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) -continue; -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, - &_edx); -if ( (_eax + _ebx) > *ebx ) -*ebx = _eax + _ebx; -} -} - -if ( count == 1 ) +switch ( count ) { +case 1: *eax &= hvm_featureset[FEATURESET_Da1]; - -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) { -uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; - -*ebx = XSTATE_AREA_MIN_SIZE; -if ( xfeatures & ~XSTATE_FP_SSE ) -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) -if ( xfeatures & (1ULL << sub_leaf) ) -{ -if ( test_bit(sub_leaf, _align) ) -*ebx = ROUNDUP(*ebx, 64); -*ebx += xstate_sizes[sub_leaf]; -} -} -else *ebx = *ecx = *edx = 0; +break; +} +/* fall through */ +case 0: +/* + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than + * domain policy. It varies with enabled xstate, and the correct + * xcr0/xss are in context. + */ +cpuid_count(input, count, , ebx, , ); +break; } break; x86/HVM: don't calculate XSTATE area sizes in software Use hardware output instead, brining HVM behavior in line with PV one in this regard. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3362,7 +3362,7 @@ void hvm_cpuid(unsigned int input, unsig switch ( input ) { -unsigned int sub_leaf, _eax, _ebx, _ecx, _edx; +unsigned int _ecx, _edx; case 0x1: /* Fix up VLAPIC details. */ @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig *eax = *ebx = *ecx = *edx = 0; break; } -/* EBX value of main leaf 0 depends on enabled xsave features */ -if ( count == 0 && v->arch.xcr0 ) -{ -/* reset EBX to default value first */ -*ebx = XSTATE_AREA_MIN_SIZE; -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) -{ -if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) -continue; -domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, - &_edx); -if ( (_eax + _ebx) > *ebx ) -*ebx = _eax + _ebx; -} -} - -if ( count == 1 ) +switch ( count ) { +case 1: *eax &= hvm_featureset[FEATURESET_Da1]; - -if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) +if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) { -uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; - -*ebx = XSTATE_AREA_MIN_SIZE; -if ( xfeatures & ~XSTATE_FP_SSE ) -for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) -if ( xfeatures & (1ULL << sub_leaf) ) -{ -if ( test_bit(sub_leaf, _align) ) -*ebx = ROUNDUP(*ebx, 64); -*ebx += xstate_sizes[sub_leaf]; -} -} -else *ebx = *ecx = *edx = 0; +break; +} +/* fall through */ +case 0: +/* + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than + * domain policy. It varies with enabled xstate, and the correct + * xcr0/xss are in context. + */ +cpuid_count(input,
[Xen-devel] [PATCH 0/2] x86: xstate CPUID guest output adjustments
1: flush high xstate CPUID sub-leaves to zero 2: HVM: don't calculate XSTATE area sizes in software Patch 1 is certainly meant for 4.7. Patch 2 would imo also be nice (as replacing the odd software calculation by whatever the hardware returns can only improve overall output), but I wouldn't insist. Signed-off-by: Jan Beulich___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/14] Xen ARM DomU ACPI support
On 2016年06月01日 22:36, Boris Ostrovsky wrote: > On 06/01/2016 09:53 AM, Shannon Zhao wrote: >> > >> > Actually I think there are two tables can be reused: RSDP and XSDT(while >> > which are smaller codes, I don't think it needs to be mixed with >> > others). The other tables are arch specific because the contents are >> > totally different. So if we want to add some generic generating table >> > funtions, we might pass a lot of arch specific information to these >> > functions which looks like inconvenient and ugly. >> > And this situation for x86 and ARM is similar with QEMU. You can have a >> > look at hw/arm/virt-acpi-build.c and hw/i386/acpi-build.c in QEMU source >> > code. The two only reuse the build_rsdt() function. > Are these differences due to architecture or ACPI version? I can see > that ARM uses 5.1 at least but not sure about i386. Yes, ARM uses 5.1+. -- Shannon ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 95121: regressions - FAIL
flight 95121 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/95121/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail REGR. vs. 94856 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail REGR. vs. 94856 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail in 95089 REGR. vs. 94856 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 6 xen-boot fail in 95089 pass in 95121 test-amd64-amd64-xl-rtds 6 xen-boot fail in 95089 pass in 95121 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 13 guest-localmigrate fail pass in 95089 test-amd64-i386-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail pass in 95089 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-localmigrate fail REGR. vs. 94856 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 94856 test-amd64-amd64-xl-rtds 9 debian-install fail like 94856 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: qemuu500acc9c410bcea17148a1072e323c08d12e6a6b baseline version: qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe Last test of basis94856 2016-05-27 20:14:49 Z4 days Failing since 94983 2016-05-31 09:40:12 Z1 days4 attempts Testing same since94994 2016-05-31 15:42:55 Z0 days3 attempts People who touched revisions under test: Anthony PERARDBenjamin Herrenschmidt Bharata B Rao Chen Fan Cédric Le Goater David Gibson Drew Jones Emilio G. Cota Eric Blake Fam Zheng Gu Zheng Michael Neuling
Re: [Xen-devel] [PATCH v2] xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI
On 01/06/16 15:38, Stefano Stabellini wrote: > as a consequence of 9d77b3c01d1261ce17c10097a1b393f2893ca657 being > reverted. > > Signed-off-by: Stefano StabelliniSome style corrections. > @@ -342,9 +344,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) > unsigned long flags; > int i = 0; > struct vcpu *v_target; > +struct domain *d = v->domain; > > while ( (i = find_next_bit(, 32, i)) < 32 ) { > irq = i + (32 * n); > +/* Set the irq type and route it to guest only for SPI and Dom0 */ > +if( irq_access_permitted(d, irq) && is_hardware_domain(d) && Space after if. > +( irq >= 32 ) && ( !acpi_disabled ) ) Extraneous spaces, and pointless brackets for the acpi_disabled. > +{ > +static int log_once = 0; bool_t. Also, missing a newline between the variable declaration and code. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: xen-pciback: Remove create_workqueue
On 01/06/16 15:15, Bhaktipriya Shridhar wrote: > System workqueues have been able to handle high level of concurrency > for a long time now and there's no reason to use dedicated workqueues > just to gain concurrency. Replace dedicated xen_pcibk_wq with the > use of system_wq. > > Unlike a dedicated per-cpu workqueue created with create_workqueue(), > system_wq allows multiple work items to overlap executions even on > the same CPU; however, a per-cpu workqueue doesn't have any CPU > locality or global ordering guarantees unless the target CPU is > explicitly specified and thus the increase of local concurrency shouldn't > make any difference. > > Since the work items could be pending, flush_work() has been used in > xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev() > which in turn calls xen_pcibk_disconnect() for every pdev to ensure that > there is no pending task while disconnecting the driver. > > Signed-off-by: Bhaktipriya Shridhar> --- > Changes in v2: > -Changed cancel_work_sync to flush_work > -Changed commit description > > Feedback to update a comment was received in v1 from David Vrabel. It has not > be included in v2 since some clarification was required. Will include it in > v3 once the details about the content and the placement of the comment are > received. The comment needed updating iff this continued to use cancel_work_sync() but since it's using flush_work() the comment is fine. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI
On Wed, Jun 01, 2016 at 03:45:28PM +0100, Julien Grall wrote: > > > On 01/06/16 15:38, Stefano Stabellini wrote: > >as a consequence of 9d77b3c01d1261ce17c10097a1b393f2893ca657 being > >reverted. > > > >Signed-off-by: Stefano Stabellini> > Reviewed-by: Julien Grall Release-acked-by: Wei Liu > > >--- > >Changes in v2: > >- fix typo > >- add log_once > >--- > > xen/arch/arm/vgic.c | 15 +++ > > 1 file changed, 15 insertions(+) > > > >diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > >index ee35683..413ff16 100644 > >--- a/xen/arch/arm/vgic.c > >+++ b/xen/arch/arm/vgic.c > >@@ -25,6 +25,8 @@ > > #include > > #include > > #include > >+#include > >+#include > > > > #include > > > >@@ -342,9 +344,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) > > unsigned long flags; > > int i = 0; > > struct vcpu *v_target; > >+struct domain *d = v->domain; > > > > while ( (i = find_next_bit(, 32, i)) < 32 ) { > > irq = i + (32 * n); > >+/* Set the irq type and route it to guest only for SPI and Dom0 */ > >+if( irq_access_permitted(d, irq) && is_hardware_domain(d) && > >+( irq >= 32 ) && ( !acpi_disabled ) ) > >+{ > >+static int log_once = 0; > >+if ( !log_once ) > >+{ > >+gprintk(XENLOG_WARNING, "Routing SPIs to Dom0 on ACPI > >systems is unimplemented.\n"); > >+log_once++; > >+} > >+} > >+ > > v_target = __vgic_get_target_vcpu(v, irq); > > p = irq_to_pending(v_target, irq); > > set_bit(GIC_IRQ_GUEST_ENABLED, >status); > > > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI
On 01/06/16 15:38, Stefano Stabellini wrote: as a consequence of 9d77b3c01d1261ce17c10097a1b393f2893ca657 being reverted. Signed-off-by: Stefano StabelliniReviewed-by: Julien Grall --- Changes in v2: - fix typo - add log_once --- xen/arch/arm/vgic.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index ee35683..413ff16 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include @@ -342,9 +344,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) unsigned long flags; int i = 0; struct vcpu *v_target; +struct domain *d = v->domain; while ( (i = find_next_bit(, 32, i)) < 32 ) { irq = i + (32 * n); +/* Set the irq type and route it to guest only for SPI and Dom0 */ +if( irq_access_permitted(d, irq) && is_hardware_domain(d) && +( irq >= 32 ) && ( !acpi_disabled ) ) +{ +static int log_once = 0; +if ( !log_once ) +{ +gprintk(XENLOG_WARNING, "Routing SPIs to Dom0 on ACPI systems is unimplemented.\n"); +log_once++; +} +} + v_target = __vgic_get_target_vcpu(v, irq); p = irq_to_pending(v_target, irq); set_bit(GIC_IRQ_GUEST_ENABLED, >status); -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/16] x86: add multiboot2 protocol support for relocatable images
>>> On 01.06.16 at 15:35,wrote: > On Wed, May 25, 2016 at 05:03:20AM -0600, Jan Beulich wrote: >> >>> On 15.04.16 at 14:33, wrote: >> > --- a/xen/arch/x86/boot/head.S >> > +++ b/xen/arch/x86/boot/head.S >> > @@ -79,6 +79,13 @@ multiboot2_header_start: >> > /* Align modules at page boundry. */ >> > mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED) >> > >> > +/* Load address preference. */ >> > +mb2ht_init MB2_HT(RELOCATABLE), MB2_HT(OPTIONAL), \ >> > + sym_offset(start), /* Min load address. */ \ >> > + 0x, /* Max load address (4 GiB - 1). */ \ >> >> Hardly - that would allow us to be loaded at 4G - 2M, no matter >> how large the image. Or else the comment is misleading. > > This is the highest address at which memory region allocated for image > may end. You saying "end" then means the comment is misleading. >> > @@ -178,30 +185,39 @@ efi_multiboot2_proto: >> > and $~(MULTIBOOT2_TAG_ALIGN-1),%rcx >> > >> > 0: >> > +/* Get Xen image load base address from Multiboot2 information. */ >> > +cmpl$MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR,MB2_tag_type(%rcx) >> > +jne 1f >> > + >> > +mov MB2_load_base_addr(%rcx),%r15d >> > +sub $XEN_IMG_OFFSET,%r15 >> > +jmp 4f >> >> Why do we need to read this from the table? Can't we easily calculate >> this ourselves? > > Potentially yes but why do not use data from boot loader? Because it's (a) likely easier to just calculate and (b) we should perhaps trust ourselves more than an external entity? >> > +1: >> > /* Get EFI SystemTable address from Multiboot2 information. */ >> > cmpl$MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx) >> > -jne 1f >> > +jne 2f >> > >> > mov MB2_efi64_st(%rcx),%rsi >> > >> > /* Do not clear BSS twice and do not go into real mode. */ >> > movb$1,skip_realmode(%rip) >> > -jmp 3f >> > +jmp 4f >> > >> > -1: >> > +2: >> > /* Get EFI ImageHandle address from Multiboot2 information. */ >> > cmpl$MULTIBOOT2_TAG_TYPE_EFI64_IH,MB2_tag_type(%rcx) >> > -jne 2f >> > +jne 3f >> > >> > mov MB2_efi64_ih(%rcx),%rdi >> > -jmp 3f >> > +jmp 4f >> > >> > -2: >> > +3: >> > /* Is it the end of Multiboot2 information? */ >> > cmpl$MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx) >> > je run_bs >> > >> > -3: >> > +4: >> > /* Go to next Multiboot2 information tag. */ >> > add MB2_tag_size(%rcx),%ecx >> > add $(MULTIBOOT2_TAG_ALIGN-1),%rcx >> >> See why numeric labels are bad in situations like this? The (much) >> earlier patch should use .L labels here, and the patch here then >> should simply follow suit. > > Then we should change legacy multiboot (v1) code too. Just to be in line > new stuff here. Does it pays? And I am not sure that patching convenience > overweight convenience of numeric labels here. Well, it's always this same discussion: Bad examples shouldn't be used as excuse to add further bad examples. If you feel like also changing the mb1 code - go for it. But if you don't, I'm fine with just new code avoiding old mistakes. >> > @@ -313,14 +329,23 @@ multiboot2_proto: >> > and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx >> > >> > 0: >> > +/* Get Xen image load base address from Multiboot2 information. */ >> > +cmpl$MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR,MB2_tag_type(%ecx) >> > +jne 1f >> > + >> > +mov MB2_load_base_addr(%ecx),%esi >> > +sub $XEN_IMG_OFFSET,%esi >> > +jmp 3f >> >> The redundancy once again suggests some form of abstraction >> (helper function, macro, ...). > > Do you suggest that we should macroize multiboot2 header accesses? The whole logic here. And if macros (rather than functions), then I'm thinking of assembler macros more than of C ones. All I really wish is that we don't have the same kind of code in multiple places, even more so when we talk about assembly code. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen/arm: warn the user that we cannot route SPIs to Dom0 on ACPI
as a consequence of 9d77b3c01d1261ce17c10097a1b393f2893ca657 being reverted. Signed-off-by: Stefano Stabellini--- Changes in v2: - fix typo - add log_once --- xen/arch/arm/vgic.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index ee35683..413ff16 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include @@ -342,9 +344,22 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) unsigned long flags; int i = 0; struct vcpu *v_target; +struct domain *d = v->domain; while ( (i = find_next_bit(, 32, i)) < 32 ) { irq = i + (32 * n); +/* Set the irq type and route it to guest only for SPI and Dom0 */ +if( irq_access_permitted(d, irq) && is_hardware_domain(d) && +( irq >= 32 ) && ( !acpi_disabled ) ) +{ +static int log_once = 0; +if ( !log_once ) +{ +gprintk(XENLOG_WARNING, "Routing SPIs to Dom0 on ACPI systems is unimplemented.\n"); +log_once++; +} +} + v_target = __vgic_get_target_vcpu(v, irq); p = irq_to_pending(v_target, irq); set_bit(GIC_IRQ_GUEST_ENABLED, >status); -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/14] Xen ARM DomU ACPI support
On 06/01/2016 09:53 AM, Shannon Zhao wrote: > > Actually I think there are two tables can be reused: RSDP and XSDT(while > which are smaller codes, I don't think it needs to be mixed with > others). The other tables are arch specific because the contents are > totally different. So if we want to add some generic generating table > funtions, we might pass a lot of arch specific information to these > functions which looks like inconvenient and ugly. > And this situation for x86 and ARM is similar with QEMU. You can have a > look at hw/arm/virt-acpi-build.c and hw/i386/acpi-build.c in QEMU source > code. The two only reuse the build_rsdt() function. Are these differences due to architecture or ACPI version? I can see that ARM uses 5.1 at least but not sure about i386. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 4.7 qemu regression: HVM guests fail to boot from xvda
On Wed, Jun 01, Wei Liu wrote: > > Konrad mentioned Solaris, no idea if its frontend driver uses the vdev > > string. BSD should be checked as well. > > > > So you are fine with documenting without reverting the patch? I think yes, If Solaris and BSD domU are not affected by the change. Given that hdtype= is for controller type and vdev= for disk type, one has to live with the possible breakage. Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen: xen-pciback: Remove create_workqueue
System workqueues have been able to handle high level of concurrency for a long time now and there's no reason to use dedicated workqueues just to gain concurrency. Replace dedicated xen_pcibk_wq with the use of system_wq. Unlike a dedicated per-cpu workqueue created with create_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantees unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Since the work items could be pending, flush_work() has been used in xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev() which in turn calls xen_pcibk_disconnect() for every pdev to ensure that there is no pending task while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar--- Changes in v2: -Changed cancel_work_sync to flush_work -Changed commit description Feedback to update a comment was received in v1 from David Vrabel. It has not be included in v2 since some clarification was required. Will include it in v3 once the details about the content and the placement of the comment are received. drivers/xen/xen-pciback/pciback.h | 1 - drivers/xen/xen-pciback/pciback_ops.c | 2 +- drivers/xen/xen-pciback/xenbus.c | 10 +- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h index 4d529f3..7af369b6 100644 --- a/drivers/xen/xen-pciback/pciback.h +++ b/drivers/xen/xen-pciback/pciback.h @@ -55,7 +55,6 @@ struct xen_pcibk_dev_data { /* Used by XenBus and xen_pcibk_ops.c */ extern wait_queue_head_t xen_pcibk_aer_wait_queue; -extern struct workqueue_struct *xen_pcibk_wq; /* Used by pcistub.c and conf_space_quirks.c */ extern struct list_head xen_pcibk_quirks; diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c index 2f19dd7..f8c7775 100644 --- a/drivers/xen/xen-pciback/pciback_ops.c +++ b/drivers/xen/xen-pciback/pciback_ops.c @@ -310,7 +310,7 @@ void xen_pcibk_test_and_schedule_op(struct xen_pcibk_device *pdev) * already processing a request */ if (test_bit(_XEN_PCIF_active, (unsigned long *)>sh_info->flags) && !test_and_set_bit(_PDEVF_op_active, >flags)) { - queue_work(xen_pcibk_wq, >op_work); + schedule_work(>op_work); } /*_XEN_PCIB_active should have been cleared by pcifront. And also make sure xen_pcibk is waiting for ack by checking _PCIB_op_pending*/ diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index c252eb3..5ce878c 100644 --- a/drivers/xen/xen-pciback/xenbus.c +++ b/drivers/xen/xen-pciback/xenbus.c @@ -17,7 +17,6 @@ #include "pciback.h" #define INVALID_EVTCHN_IRQ (-1) -struct workqueue_struct *xen_pcibk_wq; static bool __read_mostly passthrough; module_param(passthrough, bool, S_IRUGO); @@ -76,8 +75,7 @@ static void xen_pcibk_disconnect(struct xen_pcibk_device *pdev) /* If the driver domain started an op, make sure we complete it * before releasing the shared memory */ - /* Note, the workqueue does not use spinlocks at all.*/ - flush_workqueue(xen_pcibk_wq); + flush_work(>op_work); if (pdev->sh_info != NULL) { xenbus_unmap_ring_vfree(pdev->xdev, pdev->sh_info); @@ -733,11 +731,6 @@ const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend; int __init xen_pcibk_xenbus_register(void) { - xen_pcibk_wq = create_workqueue("xen_pciback_workqueue"); - if (!xen_pcibk_wq) { - pr_err("%s: create xen_pciback_workqueue failed\n", __func__); - return -EFAULT; - } xen_pcibk_backend = _pcibk_vpci_backend; if (passthrough) xen_pcibk_backend = _pcibk_passthrough_backend; @@ -747,6 +740,5 @@ int __init xen_pcibk_xenbus_register(void) void __exit xen_pcibk_xenbus_unregister(void) { - destroy_workqueue(xen_pcibk_wq); xenbus_unregister_driver(_pcibk_driver); } -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 0/4] xen/arm: arm64: Widen register access to mpidr to 64-bits
On Wed, Jun 01, 2016 at 11:37:55AM +0100, Julien Grall wrote: > > > On 01/06/16 10:29, Stefano Stabellini wrote: > >Hi Wei Liu, Julien, > > Hi Stefano, > > > > >this series is a bug fix. I think it should go in Xen 4.7, do you agree? > > Some changes in this series impact the emulation of PSCI for guest (see > target_affinity_mask in vpsci.c). Looking at the code again, they should be > low risk as Xen only supports AFF0 and AFF1 for the vmpidr. > > I think it should be fine to go in Xen 4.7, however osstest will not be able > to catch any possible regression due to lack of ARM64 hardware in the colo. > > I just tested on Juno and I did not find any regression. I think Wei tested > on Overdrive (?). Edgar, can you give it a go on the xilinx board? > Hi Julien, Yes, I gave it a try on the ZCU102 and things are working fine. Started dom0 in SMP + a couple of SMP guests. Tested-by: Edgar E. IglesiasBest regards, Edgar ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] 4.7 qemu regression: HVM guests fail to boot from xvda
On Wed, Jun 01, 2016 at 03:34:08PM +0200, Olaf Hering wrote: > On Wed, Jun 01, Wei Liu wrote: > > > So I think the safest option now is to revert the said patch and figure > > out what to do next. > > What did OSStest report when c0c099d got into the tree? Do these test > domU.cfg files contain vdev=hd instead of vdev=xvd, so the boot breakage > was not noticed? > OSStest always has hda for hvm guest. > What are the options we have to define a disk connected to an PIIX or > AHCI controller? I see xl.cfg has hdtype=. Is vdev hd vs. xvd really the > best way to describe a PV-only disk? It looks like the answer is yes. I would think so. > hdtype= describes the controller variant, and vdev= the disk variant. > Yes, you're correct. hdtype= is for controller. > So in the end it needs to be documented properly that moving dom0 to > xen-4.7 requires adjustments to vdev= in domU.cfg (xvd -> hd), and that > this MAY have an impact for domU frontend drivers which use the vdev > string as is. > Namely this affects SLE11, SLE12, SLE12SP1, openSUSE up to Leap 42.1. > Upcoming releases use a pvops based kernel, so the device names are > fixed. > Konrad mentioned Solaris, no idea if its frontend driver uses the vdev > string. BSD should be checked as well. > So you are fine with documenting without reverting the patch? Wei. > Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 13/18] xen: introduce and use 'dom0_rambase_pfn' setting for kernel Dom0
On Tue, May 31, 2016 at 05:04:42PM +0300, Oleksandr Dmytryshyn wrote: > On Fri, May 20, 2016 at 7:05 PM, Edgar E. Iglesias >wrote: > > Hi, > > > > We have similar needs (not exactly the same) in some of our setups. > > We need to map certain OCMs (On Chip Memories) to dom0. Among other things, > > these are used to communicate with remote accelerators/CPUs that have > > "hardcoded" addresses to these RAMs. > > > > Our approach is more along the lines of Juliens second suggestion. We're > > trying to use the mmio-sram DTS bindings to bring in these memories into > > dom0. > > > > IIUC the Ducati FW issue correctly, you need to allocate a chunk of DDR. > > > > Another possible solution: > > I think you could reserve the memory area by simply not mentioning it > > in the main memory node (these nodes support multiple ranges so you can > > introduce gaps). Then you could for example create an mmio-sram node to > > get the memory explicitely mapped 1:1 into dom0. > > > > Just a moment ago, I posted an RFC for the mmio-sram support to the list. > Hi, Edgar. > > How do You access to the mapped OCMs in dom0? > Are genalloc-related functions (gen_pool_get/_alloc/_virt_to_phys) > only way to work with mmio-sram memory? Hi Oleksandr, I'm not familiar enough with the Linux APIs to give a good answer on which APIs are considered OK and which not. There are examples in the tree on other ways of using the srams though. Look for example at this (search for smp-sram): arch/arm/mach-rockchip/platsmp.c Documentation/devicetree/bindings/sram/rockchip-smp-sram.txt The allwinner,sun4i-a10-emac is another example. Best regards, Edgar > > > Cheers, > > Edgar > > > > > >> > >> Regards, > >> > >> [1] > >> http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg01879.html > >> [2] > >> http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg01894.html > >> > >> -- > >> Julien Grall > >> > >> ___ > >> Xen-devel mailing list > >> Xen-devel@lists.xen.org > >> http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] XSA-180 follow-up: repurpose xenconsoled for logging
Hi all During the discussion of XSA-180 Ian came up with the idea that we repurpose xenconsoled to handle logging. I've done some research (and some coding as well!). In my reply to George the other day: > I just read the code of virtlogd and xenconsoled. > > I think xenconsoled is missing at least things. > > From a higher level: > > 1. Abstraction of rotating file. > 2. Abstraction of client. > 3. IPC interface to libxl -- presumably we need to create a socket. > I've done #1 and port existing code to use that -- would be useful in general. #2 is not too hard because xenconsoled already has concept of a domain. I suspect some refactoring will be fine. #3 requires a bit more thinking. Virtlogd has an RPC interface -- I'm pretty sure we *don't* want that for xenconsoled. So I spent some time this morning and write up a draft for a xenstore based protocol. See below. Also there is an implication here: we put xenconsoled in a really critical path. If for some reason it doesn't work all guests are blocked. Do we really want to do this? Wei. XXX DRAFT DRAFT DRAFT XXX Per domain logging via xenconsoled == As of Xen release XXX, xenconsoled is repurposed to handle logging for QEMU. Libxenlight will arrange xenconsoled to create and handle the log file. It's possible to expose API(s) so that the user of libxenlight can leverage such ability, but it is not currently done. Xenstore path and nodes --- Libxenlight communicates with xenconsoled via a simple xenstore based protocol. All information for a specific domain is stored under /libxl/$DOMID/logging. Each log file has its own unique id ($LOGFILEID). Several xenstore nodes are needed (placed under logging/$LOGFILEID). pipe: the absolute path of the logging pipe file: the absolute path of the file to write to limit: the maximum length of the file before it gets rotated copies: the number of copies to keep state: xenconsoled writes "ready" to this node to indicate readiness Xenconsoled will sanitise both pipe and file fields. Pipe has to be placed under XEN_RUN_DIR. File has to be placed under /var/log/xen (XXX doesn't seem to be configurable at the moment, should introduce XEN_LOG_DIR?). Libxenlight and xenconsoled interaction --- Initiate logging 1. Libxenlight: 1. Generates a unique log file id $LOGFILEID 2. Creates a pipe $PIPE 3. Writes parameter to xenstore 4. Wait for readiness indication 2. Xenconsoled 1. Watch global logging and per domain logging xenstore paths 2. Gets notified, read parameters from xenstore 3. Sanitise parameters 4. Create log files 5. Connect to the pipe provided 6. Write "ready" to xenstore state node 3. Libxenlight: 1. Detects ready state from xenconsoled 2. Open the pipe and return relevant handles to user In case of xenconsoled failure, libxenlight will time out and bail. Clean up When doing per domain logging, libxenlight will remove all domain specific xenstore paths when a guest is gone. Xenconsoled use that to do clean up. Libxenlight is responsible for deleting the pipe. Global logging -- Since we don't plan to provide new APIs now, we don't support global logging because that would require us to provide a cleanup API that libxenlight users can call. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()
On Wed, Jun 1, 2016 at 8:06 AM, Ian Jacksonwrote: > Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace > deprecated readdir_r() with readdir()"): >> Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace >> deprecated readdir_r() with readdir()"): >> > 2. There may be good reasons to deviate from a formal specification. >> > Formal specifications can be wrong (for example, they can differ from >> > established practice, or unuseable, or incoherent). But there has >> > been no discussion (at least in this thread on xen-devel) which might >> > suggest that the POSIX specification is wrongheaded here. >> >> I have been helpfully referred by a local irc channel to the following >> attempt to change posix to require that readdir() is threadsafe in the >> senses required by libx, and to deprecate readdir_r(): >> >> http://austingroupbugs.net/view.php?id=696 >> >> I find the comment 0001606 by "dalias" (et seq) totally convincing. >> The published specification of readdir_r is indeed incoherent. And >> only contrived implementations of readdir will not be threadsafe in >> the required sense. > ... >> Accordingly, I think all occurrences of readdir_r in our codebase >> should be replaced by readdir, as proposed by Chris. >> >> However, I think the patch is not quite complete, as the change from >> readdir_r to readdir should also involve removing the local dirent >> variables associated with each call site. > > Also, the commit message needs to be expanded to provide the > rationale. It should restate the reasoning provided by "dalias" and > provide links to the austingroupbugs thread and references to the > comment numbers. > > Ian. Agreed, will post a v2. Thanks for the feedback. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] arm/acpi: Fix the deadlock in function vgic_lock_rank()
On 2016年06月01日 18:49, Julien Grall wrote: > Hi Stefano, > > On 01/06/16 10:54, Stefano Stabellini wrote: >>> spin_is_locked does not work as you expect. The function will not >>> tell you if >>> the lock was taken by the current CPU, but if the lock was taken by >>> *a* CPU. >>> >>> It would be possible to have CPU A calling this function and have CPU >>> B with >>> the lock taken. So the data structure would be accessed by 2 CPUs >>> concurrently, which is unsafe. >> >> Damn, you are right. I don't think we have a spin_lock function which >> tells us if the spin_lock was taken by us. > > Unfortunately not. > >> The only other option I see would be duplicating route_irq_to_guest and >> gic_route_irq_to_guest, introducing a second version of those functions >> which assume that the rank lock was already taken. Very very ugly. I'll >> just revert the commit and wait for better patches from Shannon. > > I am working on a patch series to decouple IRQ configuration and > routing. It should be ready soon. > Thanks, Julien :) -- Shannon ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/14] Xen ARM DomU ACPI support
On 2016年06月01日 21:21, Julien Grall wrote: > > > On 01/06/16 13:59, Boris Ostrovsky wrote: >> On 06/01/2016 08:42 AM, Julien Grall wrote: >>> On 31/05/16 21:51, Boris Ostrovsky wrote: On 05/31/2016 03:42 PM, Konrad Rzeszutek Wilk wrote: > On Tue, May 31, 2016 at 12:43:22PM +0800, Shannon Zhao wrote: >> From: Shannon Zhao>> >> The design of this feature is described as below. >> Firstly, the toolstack (libxl) generates the ACPI tables according >> the >> number of vcpus and gic controller. > CC-ing Boris - who has been working on exposing ACPI tables > for PVH guests. > > Is there some way of re-using some of the code? Indeed it would be good to keep all ACPI code in single place. I sent a patch series a while ago (http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00625.html) but because of release work it hasn't been reviewed yet. Shannon, can you take a look at it and see whether you code can make use of what is proposed there? It builds all the tables that you are building here except for GTDT and provides libxc interface. >>> >>> AFAICT, your library is creating ACPI 1.0/2.0 tables. However the >>> support for ARM has been added in ACPI 5.1. >>> >>> Looking at the list of tables built by the library, we might be able >>> to re-use the code for SRAT, SLIT, FADT, RSDP. The rest is x86 >>> specific (WAET, MADT, HPET, SSDT_{PM,S3,S4}, TCPA (?)). >> >> The interface allows choosing which tables to generate so that shouldn't >> be a problem. > > Would it be possible to opt-out some of the tables at built-time. Maybe > by moving the code in separate files? > >>> >>> In the current state, I think the benefits for ARM is very limited. I >>> agree that having a common library to manipulate ACPI would be nice, >>> however, this would need a better abstraction to support different >>> version and avoid to build unnecessary code. >>> >> >> Can you suggest improvements to that series so that (even if not now but >> at some point later) it is easier to integrate ARM and x86? Again, this >> code is an RFC so now is the time to do it right. > > I agree :). I think the 2 points that would make easier to integrate ARM > are: >- Newer version of ACPI (I know that you need to keep ACPI 1.0/2.0 > for some old guests). >- Possibility to have per-arch tables and fields. For instance the > MADT will be different for ARM. Also, some fields in the FADT are ARM > specific (see arm_boot_flags). > > I have not yet review this series, so it is my best guess. Shannon, any > opinions? Actually I think there are two tables can be reused: RSDP and XSDT(while which are smaller codes, I don't think it needs to be mixed with others). The other tables are arch specific because the contents are totally different. So if we want to add some generic generating table funtions, we might pass a lot of arch specific information to these functions which looks like inconvenient and ugly. And this situation for x86 and ARM is similar with QEMU. You can have a look at hw/arm/virt-acpi-build.c and hw/i386/acpi-build.c in QEMU source code. The two only reuse the build_rsdt() function. Thanks, -- Shannon ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] arm/acpi: Add Server Base System Architecture UART support
Hello Shanker, On 31/05/16 15:02, Shanker Donthineni wrote: The ARM Server Base System Architecture describes a generic UART interface. It doesn't support clock control registers, modem control, DMA and hardware flow control features. So, extend the driver probe() to handle SBSA interface and skip the accessing PL011 registers that are not described in SBSA document. Please mention the version of the spec in the commit message. Signed-off-by: Shanker Donthineni--- Changes since v1: Don't access UART registers that are not part of SBSA document. Move setting baudrate function to a separate function. xen/drivers/char/pl011.c | 56 +++- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c index 1212d5c..b57f3b0 100644 --- a/xen/drivers/char/pl011.c +++ b/xen/drivers/char/pl011.c @@ -41,6 +41,7 @@ static struct pl011 { /* struct timer timer; */ /* unsigned int timeout_ms; */ /* bool_t probing, intr_works; */ +bool sbsa; /* ARM SBSA generic interface */ } pl011_com = {0}; /* These parity settings can be ORed directly into the LCR. */ @@ -81,17 +82,10 @@ static void pl011_interrupt(int irq, void *data, struct cpu_user_regs *regs) } } -static void __init pl011_init_preirq(struct serial_port *port) +static void __init pl011_init_baudrate(struct serial_port *port) { struct pl011 *uart = port->uart; unsigned int divisor; -unsigned int cr; - -/* No interrupts, please. */ -pl011_write(uart, IMSC, 0); - -/* Definitely no DMA */ -pl011_write(uart, DMACR, 0x0); /* Line control and baud-rate generator. */ if ( uart->baud != BAUD_AUTO ) @@ -114,6 +108,24 @@ static void __init pl011_init_preirq(struct serial_port *port) | FEN | ((uart->stop_bits - 1) << 3) | uart->parity); +} As mentioned on the previous version, the code to set/read the baudrate is just wrong. The clock frequency is hardcoded rather than read from the firmware. However, the baudrate is always set to BAUD_AUTO for this driver, and never used after. So all this code should be dropped. @@ -315,6 +331,7 @@ static int __init pl011_acpi_uart_init(const void *data) { acpi_status status; struct acpi_table_spcr *spcr = NULL; +bool sbsa; int res; status = acpi_get_table(ACPI_SIG_SPCR, 0, @@ -326,17 +343,21 @@ static int __init pl011_acpi_uart_init(const void *data) return -EINVAL; } +sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32) ? true : false; sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32); However, can you explain why you kept ACPI_DBG2_SBSA_32 and not ACPI_DBG2_SBSA? The former is deprecated whilst the latter is the official one. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/14] Xen ARM DomU ACPI support
On 06/01/2016 09:21 AM, Julien Grall wrote: > > > On 01/06/16 13:59, Boris Ostrovsky wrote: >> On 06/01/2016 08:42 AM, Julien Grall wrote: >>> On 31/05/16 21:51, Boris Ostrovsky wrote: On 05/31/2016 03:42 PM, Konrad Rzeszutek Wilk wrote: > On Tue, May 31, 2016 at 12:43:22PM +0800, Shannon Zhao wrote: >> From: Shannon Zhao>> >> The design of this feature is described as below. >> Firstly, the toolstack (libxl) generates the ACPI tables >> according the >> number of vcpus and gic controller. > CC-ing Boris - who has been working on exposing ACPI tables > for PVH guests. > > Is there some way of re-using some of the code? Indeed it would be good to keep all ACPI code in single place. I sent a patch series a while ago (http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00625.html) but because of release work it hasn't been reviewed yet. Shannon, can you take a look at it and see whether you code can make use of what is proposed there? It builds all the tables that you are building here except for GTDT and provides libxc interface. >>> >>> AFAICT, your library is creating ACPI 1.0/2.0 tables. However the >>> support for ARM has been added in ACPI 5.1. >>> >>> Looking at the list of tables built by the library, we might be able >>> to re-use the code for SRAT, SLIT, FADT, RSDP. The rest is x86 >>> specific (WAET, MADT, HPET, SSDT_{PM,S3,S4}, TCPA (?)). >> >> The interface allows choosing which tables to generate so that shouldn't >> be a problem. > > Would it be possible to opt-out some of the tables at built-time. > Maybe by moving the code in separate files? You mean per-arch (like what you are saying below)? Sure, if we can identify which tables the we can split them into separate files. -boris > >>> >>> In the current state, I think the benefits for ARM is very limited. I >>> agree that having a common library to manipulate ACPI would be nice, >>> however, this would need a better abstraction to support different >>> version and avoid to build unnecessary code. >>> >> >> Can you suggest improvements to that series so that (even if not now but >> at some point later) it is easier to integrate ARM and x86? Again, this >> code is an RFC so now is the time to do it right. > > I agree :). I think the 2 points that would make easier to integrate > ARM are: >- Newer version of ACPI (I know that you need to keep ACPI 1.0/2.0 > for some old guests). >- Possibility to have per-arch tables and fields. For instance the > MADT will be different for ARM. Also, some fields in the FADT are ARM > specific (see arm_boot_flags). > > I have not yet review this series, so it is my best guess. Shannon, > any opinions? > > Regards, > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel