[Xen-devel] [qemu-mainline test] 105593: regressions - FAIL
flight 105593 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/105593/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm5 xen-buildfail REGR. vs. 105279 build-amd64-xsm 5 xen-buildfail REGR. vs. 105279 build-amd64 5 xen-buildfail REGR. vs. 105279 build-i3865 xen-buildfail REGR. vs. 105279 build-armhf-xsm 5 xen-buildfail REGR. vs. 105279 build-armhf 5 xen-buildfail REGR. vs. 105279 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 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-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(
[Xen-devel] arm: alloc_heap_pages allocates already allocated page
Hi, I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. With 4.8 or staging I see below panic. I have add print in alloc_heap_pages to print page_to_mfn(pg) and before and after calling map_range_to_domain() DT mapping. It shows that 0x83e22a is returned for multiple alloc_heap_page requests. Request 1: (XEN) Going to map - MMIO: 97e0c600 - 9800 P2MType=5 (XEN) In alloc_heap_pages pg start 0x83e22a pg end 0x83e22a <== pg is same here (XEN) - MMIO: 97e0c600 - 9800 P2MType=5 Request 2: (XEN) Going to map - MMIO: 97e005003800 - 97e005003830 P2MType=5 (XEN) In alloc_heap_pages pg start 0x83e22a pg end 0x83e22a <=== pg is same here (XEN) - MMIO: 97e005003800 - 97e005003830 P2MType=5 Request 3: (XEN) Going to map - MMIO: 9490 - 94900200 P2MType=5 (XEN) In alloc_heap_pages pg start 0x83e22a pg end 0x83e22a <=== pg is same here Log: (XEN) DT: one level translation:<3> 97e0<3> c600<3> (XEN) DT: reached root node (XEN) Going to map - MMIO: 97e0c600 - 9800 P2MType=5 (XEN) In alloc_heap_pages pg start 0x83e22a pg end 0x83e22a (XEN) - MMIO: 97e0c600 - 9800 P2MType=5 (XEN) handle /soc@1000/pci@8480/mrml-bridge@1,0 (XEN) dt_irq_number: dev=/soc@1000/pci@8480/mrml-bridge@1,0 (XEN) /soc@1000/pci@8480/mrml-bridge@1,0 passthrough = 1 nirq = 0 naddr = 0 (XEN) Mapping children of /soc@1000/pci@8480/mrml-bridge@1,0 to guest (XEN) dt_for_each_irq_map: par=/soc@1000/pci@8480/mrml-bridge@1,0 cb=00248b30 data=80083ee04000 (XEN) dt_for_each_irq_map: ipar=/interrupt-controller@8010, size=3 (XEN) -> addrsize=2 (XEN) -> no map, ignoring (XEN) dt_for_each_range: dev=mrml-bridge, bus=pci, parent=pci, rlen=8, rone=8 (XEN) DT: ** translation for device /soc@1000/pci@8480/mrml-bridge@1,0 ** (XEN) DT: bus is pci (na=3, ns=2) on /soc@1000/pci@8480 (XEN) DT: translating address:<3> 0300<3> 87e0<3> <3> (XEN) DT: parent bus is default (na=2, ns=2) on /soc@1000 (XEN) DT: walking ranges... (XEN) DT: PCI map, cp=8020, s=60, da=87e0 (XEN) DT: PCI map, cp=8380, s=a0, da=87e0 (XEN) DT: PCI map, cp=8460, s=20, da=87e0 (XEN) DT: PCI map, cp=8680, s=1602400, da=87e0 (XEN) DT: parent translation for:<3> 8680<3> <3> (XEN) DT: with offset: 160 (XEN) DT: one level translation:<3> 87e0<3> <3> (XEN) DT: parent bus is default (na=2, ns=2) on / (XEN) DT: walking ranges... (XEN) DT: default map, cp=8000, s=1000, da=87e0 (XEN) DT: parent translation for:<3> 9000<3> <3> (XEN) DT: with offset: 7e0 (XEN) DT: one level translation:<3> 97e0<3> <3> (XEN) DT: reached root node (XEN) Going to map - MMIO: 97e0 - 97f0 P2MType=5 (XEN) - MMIO: 97e0 - 97f0 P2MType=5 (XEN) handle /soc@1000/pci@8480/mrml-bridge@1,0/mdio-nexus@1,3 (XEN) dt_irq_number: dev=/soc@1000/pci@8480/mrml-bridge@1,0/mdio-nexus@1,3 (XEN) /soc@1000/pci@8480/mrml-bridge@1,0/mdio-nexus@1,3 passthrough = 1 nirq = 0 naddr = 1 (XEN) DT: ** translation for device /soc@1000/pci@8480/mrml-bridge@1,0/mdio-nexus@1,3 ** (XEN) DT: bus is pci (na=3, ns=2) on /soc@1000/pci@8480/mrml-bridge@1,0 (XEN) DT: translating address:<3> 0300<3> 87e0<3> 0500<3> (XEN) DT: parent bus is pci (na=3, ns=2) on /soc@1000/pci@8480 (XEN) DT: walking ranges... (XEN) DT: PCI map, cp=87e0, s=10, da=87e00500 (XEN) DT: parent translation for:<3> 0300<3> 87e0<3> <3> (XEN) DT: with offset: 500 (XEN) DT: one level translation:<3> 0300<3> 87e0<3> 0500<3> (XEN) DT: parent bus is default (na=2, ns=2) on /soc@1000 (XEN) DT: walking ranges... (XEN) DT: PCI map, cp=8020, s=60, da=87e00500 (XEN) DT: PCI map, cp=8380, s=a0, da=87e00500 (XEN) DT: PCI map, cp=8460, s=20, da=87e00500 (XEN) DT: PCI map, cp=8680, s=1602400, da=87e00500 (XEN) DT: parent translation for:<3> 8680<3> <3> (XEN) DT: with offset: 1600500 (XEN) DT: one level translation:<3> 87e0<3> 0500<3> (XEN) DT: parent bus is default (na=2, ns=2) on / (XEN) DT: walking ranges... (XEN) DT: default map, cp=8000, s=1000, da=87e00500 (XEN) DT: parent translation for:<3> 9000<3> <3> (XEN) DT: with offset: 7e00500 (XEN) DT: one level translation:<3> 97e0<3> 0500<3> (XEN) DT: reached root node (XEN) Going to map - MMIO: 97e00500 - 97e00580 P2MType=5 (XEN) In alloc_heap_pages pg start 0x83e2
Re: [Xen-devel] [PATCH v3] xenstore: remove XS_RESTRICT support
> On 1 Feb 2017, at 14:49, Juergen Gross wrote: > > On 27/01/17 12:47, Juergen Gross wrote: >> XS_RESTRICT and the xenstore library function xs_restrict() have never >> been usable in all configurations and there are no known users. >> >> This functionality was thought to limit access rights of device models >> to xenstore in order to avoid affecting other domains in case of a >> security breech. Unfortunately XS_RESTRICT won't help as current >> qemu is requiring access to dom0 only accessible xenstore paths to >> work correctly. So this command is useless and should be removed. >> >> In order to avoid problems in the future remove all support for >> XS_RESTRICT from xenstore. >> >> Signed-off-by: Juergen Gross > > Adding Dave Scott to Cc: list. Looks fine to me: Acked-by: David Scott Cheers, Dave > > > Juergen > >> --- >> V3: remove restrict functions in ocaml as requested by Wei Liu >> >> V2: don't replace XS_RESTRICT enum member with a dummy one as suggested >>by Andrew Cooper >> --- >> tools/ocaml/libs/xb/op.ml | 5 ++--- >> tools/ocaml/libs/xb/xb.mli | 1 - >> tools/ocaml/xenstored/connection.ml | 3 --- >> tools/ocaml/xenstored/logging.ml| 1 - >> tools/ocaml/xenstored/perms.ml | 5 - >> tools/ocaml/xenstored/process.ml| 16 >> tools/xenstore/include/xenstore.h | 9 - >> tools/xenstore/xenstored_core.c | 1 - >> tools/xenstore/xs.c | 8 >> xen/include/public/io/xs_wire.h | 4 ++-- >> 10 files changed, 4 insertions(+), 49 deletions(-) >> >> diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml >> index 69346d8..d4f1f08 100644 >> --- a/tools/ocaml/libs/xb/op.ml >> +++ b/tools/ocaml/libs/xb/op.ml >> @@ -19,7 +19,7 @@ type operation = Debug | Directory | Read | Getperms | >> Transaction_end | Introduce | Release | >> Getdomainpath | Write | Mkdir | Rm | >> Setperms | Watchevent | Error | Isintroduced | >> - Resume | Set_target | Restrict | Reset_watches | >> + Resume | Set_target | Reset_watches | >> Invalid >> >> let operation_c_mapping = >> @@ -28,7 +28,7 @@ let operation_c_mapping = >>Transaction_end; Introduce; Release; >>Getdomainpath; Write; Mkdir; Rm; >>Setperms; Watchevent; Error; Isintroduced; >> - Resume; Set_target; Restrict; Reset_watches |] >> + Resume; Set_target; Reset_watches |] >> let size = Array.length operation_c_mapping >> >> let array_search el a = >> @@ -68,6 +68,5 @@ let to_string ty = >> | Isintroduced -> "IS_INTRODUCED" >> | Resume-> "RESUME" >> | Set_target-> "SET_TARGET" >> -| Restrict -> "RESTRICT" >> | Reset_watches -> "RESET_WATCHES" >> | Invalid -> "INVALID" >> diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli >> index 6c242da..b4d7052 100644 >> --- a/tools/ocaml/libs/xb/xb.mli >> +++ b/tools/ocaml/libs/xb/xb.mli >> @@ -22,7 +22,6 @@ module Op : >> | Isintroduced >> | Resume >> | Set_target >> - | Restrict >> | Reset_watches >> | Invalid >> val operation_c_mapping : operation array >> diff --git a/tools/ocaml/xenstored/connection.ml >> b/tools/ocaml/xenstored/connection.ml >> index 3ffd35b..27fa778 100644 >> --- a/tools/ocaml/xenstored/connection.ml >> +++ b/tools/ocaml/xenstored/connection.ml >> @@ -122,9 +122,6 @@ let close con = >> let get_perm con = >> con.perm >> >> -let restrict con domid = >> -con.perm <- Perms.Connection.restrict con.perm domid >> - >> let set_target con target_domid = >> con.perm <- Perms.Connection.set_target (get_perm con) >> ~perms:[Perms.READ; Perms.WRITE] target_domid >> >> diff --git a/tools/ocaml/xenstored/logging.ml >> b/tools/ocaml/xenstored/logging.ml >> index c52f03d..0c0d03d 100644 >> --- a/tools/ocaml/xenstored/logging.ml >> +++ b/tools/ocaml/xenstored/logging.ml >> @@ -241,7 +241,6 @@ let string_of_access_type = function >> | Xenbus.Xb.Op.Mkdir -> "mkdir" >> | Xenbus.Xb.Op.Rm-> "rm " >> | Xenbus.Xb.Op.Setperms -> "setperms " >> -| Xenbus.Xb.Op.Restrict -> "restrict " >> | Xenbus.Xb.Op.Reset_watches -> "reset watches" >> | Xenbus.Xb.Op.Set_target-> "settarget" >> >> diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml >> index 19bf44c..3ea193e 100644 >> --- a/tools/ocaml/xenstored/perms.ml >> +++ b/tools/ocaml/xenstored/perms.ml >> @@ -119,11 +119,6 @@ let is_owner (connection:t) id = >> let is_dom0 (connection:t) = >> is_owner connection 0 >> >> -let restrict (connection:t) domid = >> -match connection.target, connection.main with >> -| None, (0, perms) -> { connection with main = (domid, perms) } >> -| _-> raise Define.Per
[Xen-devel] [PATCH] libxl: correct xenstore entry for empty cdrom
Specifying an empty cdrom device will result in a Xenstore entry params = aio:(null) as the physical device path isn't existing. This lets a domain booted via OVMF hang as OVMF is checking for "aio:" only in order to detect the empty cdrom case. Use an empty string for the physical device path in this case. As a cdrom device for HVM is always backed by qdisk we only need to cover this backend. Signed-off-by: Juergen Gross --- This is a backport candidate. --- tools/libxl/libxl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index d400fa2..2e134eb 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2254,7 +2254,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, case LIBXL_DISK_BACKEND_QDISK: flexarray_append(back, "params"); flexarray_append(back, GCSPRINTF("%s:%s", - libxl__device_disk_string_of_format(disk->format), disk->pdev_path)); + libxl__device_disk_string_of_format(disk->format), + disk->pdev_path ? : "")); if (libxl_defbool_val(disk->colo_enable)) { flexarray_append(back, "colo-host"); flexarray_append(back, libxl__sprintf(gc, "%s", disk->colo_host)); -- 2.10.2 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 105589: regressions - trouble: blocked/broken/fail/pass
flight 105589 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/105589/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-raw3 host-install(3)broken REGR. vs. 105576 test-amd64-i386-qemut-rhel6hvm-amd 3 host-install(3) broken REGR. vs. 105576 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail REGR. vs. 105576 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 105576 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 105576 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 105576 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 105576 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 105576 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 105576 test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 105576 test-amd64-amd64-xl-rtds 9 debian-install fail like 105576 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 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-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass build-arm64 5 xen-buildfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass build-arm64-pvops 5 kernel-build fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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 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-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass version targeted for testing: xen 2733b800c9a2086d46379d3eb3f480eb5fd433ea baseline version: xen 55a04feaa1f8ab6ef7d723fbb1d39c6b96ad184a Last test of basis 105576 2017-02-06 13:16:36 Z0 days Testing same since 105589 2017-02-07 00:14:29 Z0 days1 attempts People who touched revisions under test: Andrew Cooper George Dunlap Julien Grall jobs: build-amd64-xsm pass
Re: [Xen-devel] IOMMU fault with IGD passthrough setup on XEN 4.8.0
>>> On 07.02.17 at 03:12, wrote: > On Mon, Feb 6, 2017 at 7:43 PM, Jan Beulich wrote: > On 05.02.17 at 06:51, wrote: >>> Please find the full log in the attachment. >> >> Sadly that one is only a partial log again. I'd really need to see the >> boot messages too, in particular to (hopefully) be able to judge >> whether your system uses shared or separate EPT and VT-d tables. >> > In the dom0.xz attachment (the second one on Feb 5th), the xl dmesg > info from boot stage is retained. > If that's not good enough, please instruct on how to generate the desired > log. That's good enough; sorry for not having checked there. > Quote some log snippets here, please find the full log in the old > attachment: > It appears that the system uses separated EPT && VT-d tables. > Is this good or bad? Neither - it is merely relevant. In particular (and the new debug patch may help understand this) it may mean that while EPT tables get updated, IOMMU page tables may not be (even if the code suggests this is being taken care of). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 105597: regressions - FAIL
flight 105597 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/105597/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm5 xen-buildfail REGR. vs. 105279 build-amd64-xsm 5 xen-buildfail REGR. vs. 105279 build-amd64 5 xen-buildfail REGR. vs. 105279 build-i3865 xen-buildfail REGR. vs. 105279 build-armhf-xsm 5 xen-buildfail REGR. vs. 105279 build-armhf 5 xen-buildfail REGR. vs. 105279 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 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-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
>>> On 07.02.17 at 00:32, wrote: > Commit c7bdecae42 ("x86/apicv: fix RTC periodic timer and apicv issue") has > added a assertion that intack.vector is the highest priority vector. But > according to the osstest, the assertion failed sometimes. More discussion can > be found in the thread > (https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01019.html). > > The assertion failure is hard to reproduce. In order to root cause issue, this > patch is to add logs to dump PIR and vIRR when failure takes place. It should > be reverted once the root cause is found. > > Signed-off-by: Chao Gao If this wasn't just a temporary thing, I'd have some comments, but it's good enough (perhaps with a little massaging when committing) for this purpose. Reviewed-by: Jan Beulich Ian, Wei, is there a way to automatically scan osstest logs for occurrences of the extra output, or does someone need to look through the logs of every spurious failure? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
>>> On 07.02.17 at 07:48, wrote: > Some comment from QEMU/KVM code, in /arch/x86/kvm/lapic.c, > > int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu) > { > /* This may race with setting of irr in __apic_accept_irq() and >* value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq >* will cause vmexit immediately and the value will be recalculated >* on the next vmentry. >*/ > ... > } > > I am afraid, there may be a similar race with setting of vIRR.. I think this is unrelated: If another interrupt came in, the highest set bit in vIRR can only increase. Plus pt_update_irq(), before returning, calls vlapic_set_irq(), which ought to result in pt_vector's vIRR bit to be set (either directly or via setting its PIR bit). I.e. the highest priority interrupt should still have a vector >= pt_vector. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
On Tue, Feb 07, 2017 at 02:51:54AM -0700, Jan Beulich wrote: > >>> On 07.02.17 at 00:32, wrote: > > Commit c7bdecae42 ("x86/apicv: fix RTC periodic timer and apicv issue") has > > added a assertion that intack.vector is the highest priority vector. But > > according to the osstest, the assertion failed sometimes. More discussion > > can > > be found in the thread > > (https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01019.html). > > > > The assertion failure is hard to reproduce. In order to root cause issue, > > this > > patch is to add logs to dump PIR and vIRR when failure takes place. It > > should > > be reverted once the root cause is found. > > > > Signed-off-by: Chao Gao > > If this wasn't just a temporary thing, I'd have some comments, but > it's good enough (perhaps with a little massaging when committing) > for this purpose. > > Reviewed-by: Jan Beulich > > Ian, Wei, is there a way to automatically scan osstest logs for > occurrences of the extra output, or does someone need to look > through the logs of every spurious failure? > I don't think there is automatically scanning in place. > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 4/9] xen/pvh: Bootstrap PVH guest
On 06/02/17 21:37, Boris Ostrovsky wrote: > On 02/06/2017 02:29 PM, kbuild test robot wrote: >> Hi Boris, >> >> [auto build test WARNING on xen-tip/linux-next] >> [also build test WARNING on v4.10-rc7] >> [cannot apply to next-20170206] >> [if your patch is applied to the wrong git tree, please drop us a note to >> help improve the system] >> >> url: >> https://github.com/0day-ci/linux/commits/Boris-Ostrovsky/x86-boot-32-Convert-the-32-bit-pgtable-setup-code-from-assembly-to-C/20170207-014642 >> base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git >> linux-next >> config: x86_64-randconfig-it0-02062149 (attached as .config) >> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 >> reproduce: >> # save the attached .config to linux build tree >> make ARCH=x86_64 >> >> All warnings (new ones prefixed by >>): >> >>>> arch/x86/xen/xen-pvh.o: warning: objtool: pvh_start_xen()+0x57: call >>>> without frame pointer save/setup > > pvh_start_xen() is never called so this needs > > diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S > index 512fda0..5e24671 100644 > --- a/arch/x86/xen/xen-pvh.S > +++ b/arch/x86/xen/xen-pvh.S > @@ -133,7 +133,7 @@ ENTRY(pvh_start_xen) > > ljmp $__BOOT_CS, $_pa(startup_32) > #endif > -ENDPROC(pvh_start_xen) > +END(pvh_start_xen) > > .section ".init.data","aw" > .balign 8 > > (I don't think it's worth re-sending the series with just this change). With this change folded in: Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On 07/02/2017 08:18, Vijay Kilari wrote: Hi, Hello, I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. I am a bit confused by what you are saying. Xen on ARM does not yet support NUMA. I also know you are working on NUMA support. So does the BUG happen on upstream xen or upstream xen + your patches? If the latter please provide more details on your modifications. With 4.8 or staging I see below panic. I would recommend you to try bisecting to see if you can pin point a specific commit. I have add print in alloc_heap_pages to print page_to_mfn(pg) and before and after calling map_range_to_domain() DT mapping. It shows that 0x83e22a is returned for multiple alloc_heap_page requests. This is fine as long as there is a free_heap_pages between the 2 calls of alloc_heap_pages. I would recommend you to look if free_heap_pages has been called. [] (XEN) Xen BUG at page_alloc.c:870 I was not able to spot a BUG at line 870 but 827. I think, the BUG_ON(PGC_state_free) is here to catch potential bug in the allocator, all the pages allocated should always be free. Furthermore, we haven't made much changes in page_alloc.c recently. So this 43 lines differences made me wonder if you did other changes than adding debug? Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable baseline-only test] 68528: tolerable trouble: blocked/broken/fail/pass
This run is configured for baseline tests only. flight 68528 xen-unstable real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/68528/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 68512 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 68512 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 68512 test-amd64-amd64-qemuu-nested-intel 17 capture-logs/l1(17) fail like 68512 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail like 68512 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 68512 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-installfail like 68512 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 9 windows-installfail like 68512 test-amd64-amd64-xl-qemut-winxpsp3 9 windows-install fail like 68512 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64 2 hosts-allocate broken never pass build-arm64-pvops 2 hosts-allocate broken never pass build-arm64-xsm 2 hosts-allocate broken never pass build-arm64 3 capture-logs broken never pass build-arm64-xsm 3 capture-logs broken never pass build-arm64-pvops 3 capture-logs broken 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-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-midway 12 migrate-support-checkfail never pass test-armhf-armhf-xl-midway 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 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 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-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-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass version targeted for testing: xen 55a04feaa1f8ab6ef7d723fbb1d39c6b96ad184a baseline version: xen ce62b8edd5de7e69ebcd5af0626f63ac6386ad2b Last test of basis68512 2017-02-04 10:47:06 Z2 days Testing same since68528 2017-02-06 23:47:15 Z0 days1 attempts People who touched revisi
Re: [Xen-devel] [PATCH v3 5/9] xen/pvh: Make sure we don't use ACPI_IRQ_MODEL_PIC for SCI
On 06/02/17 18:00, Boris Ostrovsky wrote: > Since we are not using PIC and (at least currently) don't have IOAPIC > we want to make sure that acpi_irq_model doesn't stay set to > ACPI_IRQ_MODEL_PIC (which is the default value). If we allowed it to > stay then acpi_os_install_interrupt_handler() would try (and fail) to > request_irq() for PIC. > > Instead we set the model to ACPI_IRQ_MODEL_PLATFORM which will prevent > this from happening. > > Signed-off-by: Boris Ostrovsky Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vvmx: Improvements to INVEPT instruction handling
>>> On 06.02.17 at 17:55, wrote: > * Latch current once at the start. > * Avoid the memory operand read for INVEPT_ALL_CONTEXT. Experimentally, this >is how hardware behaves, and avoids an unnecessary pagewalk. Hmm, so you say #GP is being raised for all possible reasons, but #PF can't result here? That would be pretty unusual instruction semantics. But if it's that way (to be confirmed by Intel), and ... > * Reject Reg/Reg encodings of the instruction. ... if this is possible to occur at all (I'd expect #UD to result instead of a VM exit), then ... > * Audit eptp against maxphysaddr. > * Introduce and use VMX_INSN_INVALID_INV_OPERAND to correct the vmfail >semantics. > * Add extra newlines for clarity > > Also, introduce some TODOs for further checks which should be performed. > These checks are hard to perform at the moment, as there is no easy way to see > which MSR values where given to the guest. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich with one minor adjustment request: > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1770,33 +1770,64 @@ int nvmx_handle_vmwrite(struct cpu_user_regs *regs) > > int nvmx_handle_invept(struct cpu_user_regs *regs) > { > +struct vcpu *curr = current; > +struct domain *currd = curr->domain; > struct vmx_inst_decoded decode; > -unsigned long eptp; > int ret; > > -if ( (ret = decode_vmx_inst(regs, &decode, &eptp, 0)) != X86EMUL_OKAY ) > +if ( (ret = decode_vmx_inst(regs, &decode, NULL, 0)) != X86EMUL_OKAY ) > return ret; > > +/* TODO - reject instruction completely if not configured. */ > + > +/* Instruction must have a memory operand. */ > +if ( decode.type != VMX_INST_MEMREG_TYPE_MEMORY ) > +{ > +hvm_inject_hw_exception(TRAP_invalid_op, 0); > +return X86EMUL_EXCEPTION; > +} > + > switch ( reg_read(regs, decode.reg2) ) > { > case INVEPT_SINGLE_CONTEXT: > { > -struct p2m_domain *p2m = p2m_get_nestedp2m(current, eptp); > +struct p2m_domain *p2m; > +pagefault_info_t pfinfo; > +unsigned long eptp; > + > +/* TODO - reject SINGLE_CONTEXT if not configured. */ > + > +ret = hvm_copy_from_guest_linear(&eptp, decode.mem, 8, 0, &pfinfo); Please use sizeof(eptp) here. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] x86/time: correctly honor late clearing of TSC related feature flags
On 06/02/17 08:39, Jan Beulich wrote: > As such clearing of flags may have an impact on the selected rendezvous > function, defer the establishing of a rendezvous function other than > the initial default one (std) until after all APs have been brought up. > > But don't allow such feature flags to be cleared during CPU hotplug: > Platform and local system times may have diverged significantly by > then, potentially causing noticeably (even if only temporary) strange > behavior. As we're anyway expecting only sufficiently similar CPUs to > appear during hotplug, this shouldn't be introducing new limitations. > > Reported-by: Joao Martins > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper > --- > v3: Drop original approach entirely - defer everything to > verify_tsc_reliability(), making for quite a bit smaller a patch. > > Note: Considering that tsc_check_writability() checks > TSC_RELIABLE, it being run before that feature flag has obtained > its final value seems problematic too. Should we defer that call > too? Do we know which CPUs this applies to? Might we be safe by being only 64bit these days? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vvmx: Improvements to INVEPT instruction handling
On 07/02/17 10:19, Jan Beulich wrote: > >>> On 06.02.17 at 17:55, wrote: >> * Latch current once at the start. >> * Avoid the memory operand read for INVEPT_ALL_CONTEXT. Experimentally, >> this >>is how hardware behaves, and avoids an unnecessary pagewalk. > Hmm, so you say #GP is being raised for all possible reasons, but > #PF can't result here? That would be pretty unusual instruction > semantics. But if it's that way (to be confirmed by Intel), and ... No. The memory operand is entirely ignored. No #PF, or #GP or #SS for bad segment or canonical setups. > >> * Reject Reg/Reg encodings of the instruction. > ... if this is possible to occur at all (I'd expect #UD to result instead > of a VM exit), then ... I'd hope so as well. This addition is partly from an entirely emulation point of view, as well as a proper sanity check of the decode.mem union before use. > >> * Audit eptp against maxphysaddr. >> * Introduce and use VMX_INSN_INVALID_INV_OPERAND to correct the vmfail >>semantics. >> * Add extra newlines for clarity >> >> Also, introduce some TODOs for further checks which should be performed. >> These checks are hard to perform at the moment, as there is no easy way to >> see >> which MSR values where given to the guest. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Jan Beulich > > with one minor adjustment request: > >> --- a/xen/arch/x86/hvm/vmx/vvmx.c >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >> @@ -1770,33 +1770,64 @@ int nvmx_handle_vmwrite(struct cpu_user_regs *regs) >> >> int nvmx_handle_invept(struct cpu_user_regs *regs) >> { >> +struct vcpu *curr = current; >> +struct domain *currd = curr->domain; >> struct vmx_inst_decoded decode; >> -unsigned long eptp; >> int ret; >> >> -if ( (ret = decode_vmx_inst(regs, &decode, &eptp, 0)) != X86EMUL_OKAY ) >> +if ( (ret = decode_vmx_inst(regs, &decode, NULL, 0)) != X86EMUL_OKAY ) >> return ret; >> >> +/* TODO - reject instruction completely if not configured. */ >> + >> +/* Instruction must have a memory operand. */ >> +if ( decode.type != VMX_INST_MEMREG_TYPE_MEMORY ) >> +{ >> +hvm_inject_hw_exception(TRAP_invalid_op, 0); >> +return X86EMUL_EXCEPTION; >> +} >> + >> switch ( reg_read(regs, decode.reg2) ) >> { >> case INVEPT_SINGLE_CONTEXT: >> { >> -struct p2m_domain *p2m = p2m_get_nestedp2m(current, eptp); >> +struct p2m_domain *p2m; >> +pagefault_info_t pfinfo; >> +unsigned long eptp; >> + >> +/* TODO - reject SINGLE_CONTEXT if not configured. */ >> + >> +ret = hvm_copy_from_guest_linear(&eptp, decode.mem, 8, 0, &pfinfo); > Please use sizeof(eptp) here. Ok, but in that case eptp needs to become uint64_t to match the ABI. In fact, I probably need to read all 128 bytes and perform the MBZ check on the 2nd word. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] x86/time: correctly honor late clearing of TSC related feature flags
>>> On 07.02.17 at 11:27, wrote: > On 06/02/17 08:39, Jan Beulich wrote: >> As such clearing of flags may have an impact on the selected rendezvous >> function, defer the establishing of a rendezvous function other than >> the initial default one (std) until after all APs have been brought up. >> >> But don't allow such feature flags to be cleared during CPU hotplug: >> Platform and local system times may have diverged significantly by >> then, potentially causing noticeably (even if only temporary) strange >> behavior. As we're anyway expecting only sufficiently similar CPUs to >> appear during hotplug, this shouldn't be introducing new limitations. >> >> Reported-by: Joao Martins >> Signed-off-by: Jan Beulich > > Acked-by: Andrew Cooper Thanks. >> --- >> v3: Drop original approach entirely - defer everything to >> verify_tsc_reliability(), making for quite a bit smaller a patch. >> >> Note: Considering that tsc_check_writability() checks >> TSC_RELIABLE, it being run before that feature flag has obtained >> its final value seems problematic too. Should we defer that call >> too? > > Do we know which CPUs this applies to? Might we be safe by being only > 64bit these days? amd.c sets it when ITSC and model != 0x11, which I think includes 64-bit CPUs. intel.c sets it unconditionally when ITSC. However, I don't see how all this matters, as we clear it as the result of the TSC warp producing a non-zero tsc_max_warp, and that check is purely a software thing. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/vvmx: Improvements to INVEPT instruction handling
>>> On 07.02.17 at 11:39, wrote: > On 07/02/17 10:19, Jan Beulich wrote: >> >>> On 06.02.17 at 17:55, wrote: >>> * Latch current once at the start. >>> * Avoid the memory operand read for INVEPT_ALL_CONTEXT. Experimentally, >>> this >>>is how hardware behaves, and avoids an unnecessary pagewalk. >> Hmm, so you say #GP is being raised for all possible reasons, but >> #PF can't result here? That would be pretty unusual instruction >> semantics. But if it's that way (to be confirmed by Intel), and ... > > No. The memory operand is entirely ignored. No #PF, or #GP or #SS for > bad segment or canonical setups. But then the #GP related checks in decode_vmx_inst() would also need skipping. >>> * Reject Reg/Reg encodings of the instruction. >> ... if this is possible to occur at all (I'd expect #UD to result instead >> of a VM exit), then ... > > I'd hope so as well. This addition is partly from an entirely emulation > point of view, as well as a proper sanity check of the decode.mem union > before use. The "entirely emulation point of view" is not realy applicable here, as we don't decode the instruction a 2nd time (after the hardware had done so). Sanity checking hardware produced values of course is reasonable in places like this; I'm just not sure whether reporting such issues as #UD to the guest is appropriate - I'd rather see the guest killed if hardware doesn't behave itself. >>> --- a/xen/arch/x86/hvm/vmx/vvmx.c >>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >>> @@ -1770,33 +1770,64 @@ int nvmx_handle_vmwrite(struct cpu_user_regs *regs) >>> >>> int nvmx_handle_invept(struct cpu_user_regs *regs) >>> { >>> +struct vcpu *curr = current; >>> +struct domain *currd = curr->domain; >>> struct vmx_inst_decoded decode; >>> -unsigned long eptp; >>> int ret; >>> >>> -if ( (ret = decode_vmx_inst(regs, &decode, &eptp, 0)) != X86EMUL_OKAY ) >>> +if ( (ret = decode_vmx_inst(regs, &decode, NULL, 0)) != X86EMUL_OKAY ) >>> return ret; >>> >>> +/* TODO - reject instruction completely if not configured. */ >>> + >>> +/* Instruction must have a memory operand. */ >>> +if ( decode.type != VMX_INST_MEMREG_TYPE_MEMORY ) >>> +{ >>> +hvm_inject_hw_exception(TRAP_invalid_op, 0); >>> +return X86EMUL_EXCEPTION; >>> +} >>> + >>> switch ( reg_read(regs, decode.reg2) ) >>> { >>> case INVEPT_SINGLE_CONTEXT: >>> { >>> -struct p2m_domain *p2m = p2m_get_nestedp2m(current, eptp); >>> +struct p2m_domain *p2m; >>> +pagefault_info_t pfinfo; >>> +unsigned long eptp; >>> + >>> +/* TODO - reject SINGLE_CONTEXT if not configured. */ >>> + >>> +ret = hvm_copy_from_guest_linear(&eptp, decode.mem, 8, 0, &pfinfo); >> Please use sizeof(eptp) here. > > Ok, but in that case eptp needs to become uint64_t to match the ABI. In > fact, I probably need to read all 128 bytes and perform the MBZ check on > the 2nd word. Oh, indeed, the more that this may also effect eventual exceptions needing raising. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] fuzz/x86emul: remove bogus check against fuzzer msr index
The "reg" variable in fuzz_read_msr stores the real MSR index, not an index within the fuzzer. The rest of that function already handles things correctly. We just need to remove the bogus check. Signed-off-by: Wei Liu --- tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c index 3b6d33aa25..4a2bdbe392 100644 --- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -347,9 +347,6 @@ static int fuzz_read_msr( { unsigned int idx; -if ( reg >= MSR_INDEX_MAX ) -return X86EMUL_UNHANDLEABLE; - switch ( reg ) { case MSR_TSC_AUX: -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] fuzz/x86emul: remove bogus check against fuzzer msr index
On 07/02/17 11:02, Wei Liu wrote: > The "reg" variable in fuzz_read_msr stores the real MSR index, not an > index within the fuzzer. > > The rest of that function already handles things correctly. We just need > to remove the bogus check. "Spotted by Coverity." > Signed-off-by: Wei Liu Reviewed-by: Andrew Cooper > --- > tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > index 3b6d33aa25..4a2bdbe392 100644 > --- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c > @@ -347,9 +347,6 @@ static int fuzz_read_msr( > { > unsigned int idx; > > -if ( reg >= MSR_INDEX_MAX ) > -return X86EMUL_UNHANDLEABLE; > - > switch ( reg ) > { > case MSR_TSC_AUX: ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 105599: all pass - PUSHED
flight 105599 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/105599/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 7c609a144b6636577dd60fbaa5fc64efeecd7baf baseline version: ovmf 8a399fab0af391945f0eaa251eaf4efe2d71bb3e Last test of basis 105558 2017-02-06 03:33:39 Z1 days Testing same since 105599 2017-02-07 09:08:58 Z0 days1 attempts People who touched revisions under test: Alexei Alexei Fedorov Evan Lloyd jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=ovmf + revision=7c609a144b6636577dd60fbaa5fc64efeecd7baf + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 7c609a144b6636577dd60fbaa5fc64efeecd7baf + branch=ovmf + revision=7c609a144b6636577dd60fbaa5fc64efeecd7baf + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=ovmf + xenbranch=xen-unstable + '[' xovmf = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-unstable + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable + prevxenbranch=xen-4.8-testing + '[' x7c609a144b6636577dd60fbaa5fc64efeecd7baf = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git ++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git ++ : osst...@xenbits.xen.org
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
>>> On 06.02.17 at 15:57, wrote: > Any fail during the original __vmwrite() leads to BUG() which can be > easily exploited from a guest in the nested vmx mode. > > The new function returns error code depending on the outcome: > > VMsucceed: 0 > VMfailValid: VM Instruction Error Number > VMfailInvalid: a new VMX_INSN_FAIL_INVALID > > A new macro GAS_VMX_OP is introduced in order to improve the > readability of asm. Existing ASM_FLAG_OUT macro is reused and copied > into asm_defns.h > > Signed-off-by: Sergey Dyasli > --- Please can you have the revision info for the individual patches here. I know you've put it in the overview mail, but for reviewers it's far more useful to (also) be here. > --- a/xen/include/asm-x86/hvm/vmx/vmcs.h > +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h > @@ -526,6 +526,7 @@ enum vmx_insn_errno > VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9, > VMX_INSN_UNSUPPORTED_VMCS_COMPONENT= 12, > VMX_INSN_VMXON_IN_VMX_ROOT = 15, > +VMX_INSN_FAIL_INVALID = ~0, > }; The main reason for me to ask for the type change here was to ... > @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long field, > unsigned long *value) > return okay; > } > > +static always_inline unsigned long vmwrite_safe(unsigned long field, > +unsigned long value) > +{ > +unsigned long ret = 0; > +bool fail_invalid, fail_valid; > + > +asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t", > + VMWRITE_OPCODE MODRM_EAX_ECX) > + ASM_FLAG_OUT(, "setc %[invalid]\n\t") > + ASM_FLAG_OUT(, "setz %[valid]\n\t") > + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), > + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) > + : [field] GAS_VMX_OP("r", "a") (field), > + [value] GAS_VMX_OP("rm", "c") (value)); > + > +if ( unlikely(fail_invalid) ) > +ret = VMX_INSN_FAIL_INVALID; > +else if ( unlikely(fail_valid) ) > +__vmread(VM_INSTRUCTION_ERROR, &ret); > + > +return ret; > +} ... allow the function to return enum vmx_insn_errno, and that to not be a 64-bit quantity. As you're presumably aware, dealing with 32-bit quantities is on the average slightly more efficient than dealing with 64-bit ones. The code above should imo still BUG() if the value read from VM_INSTRUCTION_ERROR doesn't fit in 32 bits (as it's a 32-bit field only anyway). Also, looking at the entire asm() above another time, wouldn't it end up a bit less convoluted if you simply used CMOVC for the "invalid" code path? Similarly I wonder whether the second VMREAD wouldn't better be moved into the asm(), utilizing the UNLIKELY_START() et al constructs to get that code path entirely out of the main path. These aren't requirements though, just aspects to think about. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall wrote: > On 07/02/2017 08:18, Vijay Kilari wrote: >> >> Hi, > > > Hello, > >>I am seeing below panic with NUMA during DT mappings in >> alloc_heap_pages() >> BUG_ON(pg[i].count_info != PGC_state_free); >> However, this issue is not there with 4.7 version. The same NUMA board >> boots fine. > > > I am a bit confused by what you are saying. Xen on ARM does not yet support > NUMA. I also know you are working on NUMA support. So does the BUG happen on > upstream xen or upstream xen + your patches? I was testing with Andre ITS patches (RFC version 1) + my NUMA patches + upstream xen. However now I tested with upstream xen + Andre ITS patches (staging branch) on NUMA board. I see panic (similar to what I see with my patches). Log are here. http://pastebin.com/QJqUBvD9 The same plain upstream xen + Andre ITS patches boots fine with non-NUMA board. > > If the latter please provide more details on your modifications. > >> With 4.8 or staging I see below panic. > > > I would recommend you to try bisecting to see if you can pin point a > specific commit. > >> >> I have add print in alloc_heap_pages to print page_to_mfn(pg) and >> before and after calling >> map_range_to_domain() DT mapping. >> It shows that 0x83e22a is returned for multiple alloc_heap_page requests. > > > This is fine as long as there is a free_heap_pages between the 2 calls of > alloc_heap_pages. I would recommend you to look if free_heap_pages has been > called. > > [] > > >> (XEN) Xen BUG at page_alloc.c:870 > > > I was not able to spot a BUG at line 870 but 827. I think, the > BUG_ON(PGC_state_free) is here to catch potential bug in the allocator, all > the pages allocated should always be free. > > Furthermore, we haven't made much changes in page_alloc.c recently. So this > 43 lines differences made me wonder if you did other changes than adding > debug? Yes, I added debug prints. > > Regards, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xenstore: remove XS_RESTRICT support
On Tue, Feb 07, 2017 at 09:06:55AM +, David Scott wrote: > > > On 1 Feb 2017, at 14:49, Juergen Gross wrote: > > > > On 27/01/17 12:47, Juergen Gross wrote: > >> XS_RESTRICT and the xenstore library function xs_restrict() have never > >> been usable in all configurations and there are no known users. > >> > >> This functionality was thought to limit access rights of device models > >> to xenstore in order to avoid affecting other domains in case of a > >> security breech. Unfortunately XS_RESTRICT won't help as current > >> qemu is requiring access to dom0 only accessible xenstore paths to > >> work correctly. So this command is useless and should be removed. > >> > >> In order to avoid problems in the future remove all support for > >> XS_RESTRICT from xenstore. > >> > >> Signed-off-by: Juergen Gross > > > > Adding Dave Scott to Cc: list. > > Looks fine to me: > > Acked-by: David Scott Applied. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On 07/02/2017 11:10, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall wrote: On 07/02/2017 08:18, Vijay Kilari wrote: I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. I am a bit confused by what you are saying. Xen on ARM does not yet support NUMA. I also know you are working on NUMA support. So does the BUG happen on upstream xen or upstream xen + your patches? I was testing with Andre ITS patches (RFC version 1) + my NUMA patches + upstream xen. However now I tested with upstream xen + Andre ITS patches (staging branch) on NUMA board. The RFC v1 is quite an old version. Please give a try using the latest version [1]. I see panic (similar to what I see with my patches). Log are here. Well the panic is different now. An ASSERT in list_del is hit this time. This looks like a memory corruption to me. http://pastebin.com/QJqUBvD9 The same plain upstream xen + Andre ITS patches boots fine with non-NUMA board. I know that DOM0 cannot boot without ITS on your platform. But as you don't reach DOM0, have you tried to boot without the ITS series on NUMA board? Regards, [1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg03276.html -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] page_alloc: clear nr_bootmem_regions in end_boot_allocator()
On 02/02/17 16:20, Jan Beulich wrote: On 02.02.17 at 16:41, wrote: >> On 02/02/17 15:25, Jan Beulich wrote: >>> --- a/xen/common/page_alloc.c >>> +++ b/xen/common/page_alloc.c >>> @@ -329,13 +329,16 @@ unsigned long __init alloc_boot_pages( >>> unsigned long nr_pfns, unsigned long pfn_align) >>> { >>> unsigned long pg, _e; >>> -int i; >>> +unsigned int i = nr_bootmem_regions; >>> >>> -for ( i = nr_bootmem_regions - 1; i >= 0; i-- ) >>> +BOOT_BUG_ON(!nr_bootmem_regions); >> Can this just be a plain BUG_ON() to avoid adding further work which >> needs to undone for livepatching purposes? > Well, for one I don't like adding inconsistency here. And then I'm > not convinced switching over to BUG_ON() is a good idea, so I'd > rather leave that discussion for when someone indeed wants to > make that change. In particular I'm not convinced that during > very early boot all the register and stack dumping functions > reliably, in which case a simple panic() is more likely to produce > at least no confusing output. Well - the change is definitely needed. BOOT_BUG_ON() has an embedded __LINE__ which causes problems making livepatches. The early register/stack functions should work correctly. I did test that when rearranging the x86 IDT handling several releases ago. As to consistency, I would prefer if the situation wasn't made worse, but if you really insist, then my R-by stands. ~Andrew > >> Otherwise, Reviewed-by: Andrew Cooper > Let me know whether this stands even without making the > requested change. > > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] x86/time: correctly honor late clearing of TSC related feature flags
On 02/07/2017 10:47 AM, Jan Beulich wrote: On 07.02.17 at 11:27, wrote: >> On 06/02/17 08:39, Jan Beulich wrote: >>> As such clearing of flags may have an impact on the selected rendezvous >>> function, defer the establishing of a rendezvous function other than >>> the initial default one (std) until after all APs have been brought up. >>> >>> But don't allow such feature flags to be cleared during CPU hotplug: >>> Platform and local system times may have diverged significantly by >>> then, potentially causing noticeably (even if only temporary) strange >>> behavior. As we're anyway expecting only sufficiently similar CPUs to >>> appear during hotplug, this shouldn't be introducing new limitations. >>> >>> Reported-by: Joao Martins >>> Signed-off-by: Jan Beulich >> >> Acked-by: Andrew Cooper > > Thanks. > >>> --- >>> v3: Drop original approach entirely - defer everything to >>> verify_tsc_reliability(), making for quite a bit smaller a patch. >>> >>> Note: Considering that tsc_check_writability() checks >>> TSC_RELIABLE, it being run before that feature flag has obtained >>> its final value seems problematic too. Should we defer that call >>> too? >> >> Do we know which CPUs this applies to? Might we be safe by being only >> 64bit these days? > > amd.c sets it when ITSC and model != 0x11, which I think includes > 64-bit CPUs. intel.c sets it unconditionally when ITSC. However, I > don't see how all this matters, as we clear it as the result of the > TSC warp producing a non-zero tsc_max_warp, and that check is > purely a software thing. Considering that TSC_RELIABLE bit (or the lack of it) is only truly valid after tsc reliability checks have been done, then it probably makes sense to move the writability check after verify_tsc_reliability (or at the end of it) one as you seem to hint above. Joao ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/28] ARM: GICv3 ITS: introduce ITS command handling
Hi Andre, On 06/02/2017 19:16, Julien Grall wrote: On 30/01/17 18:31, Andre Przywara wrote: +/* Wait for an ITS to become quiescient (all ITS operations completed). */ s/quiescient/quiescent/ +static int gicv3_its_wait_quiescient(struct host_its *hw_its) s/quiescient/quiescent/ +{ +uint32_t reg; +s_time_t deadline = NOW() + MILLISECS(1000); So that sounds fine for handling a couple of command, but what about thousands at the same time? Please ignore this question. I just noticed that I commented on the wrong function. Sorry for that. + +reg = readl_relaxed(hw_its->its_base + GITS_CTLR); +if ( (reg & (GITS_CTLR_QUIESCENT | GITS_CTLR_ENABLE)) == GITS_CTLR_QUIESCENT ) +return 0; + +writel_relaxed(reg & ~GITS_CTLR_ENABLE, hw_its->its_base + GITS_CTLR); + +do { +reg = readl_relaxed(hw_its->its_base + GITS_CTLR); +if ( reg & GITS_CTLR_QUIESCENT ) +return 0; + +cpu_relax(); +udelay(1); +} while ( NOW() <= deadline ); + +dprintk(XENLOG_ERR, "ITS not quiescient\n"); s/quiescient/quiescent/ + newline. +return -ETIMEDOUT; +} + Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] libxl: don't segfault when creating domain with invalid pvusb device
Creating a domain with an invalid controller specification for a pvusb device will currently segfault. Avoid this by bailing out early in case of a mandatory xenstore path not existing. Signed-of-by: Juergen Gross --- This patch is a backport candidate for 4.8 --- tools/libxl/libxl_usb.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c index ea7a2ab..b235507 100644 --- a/tools/libxl/libxl_usb.c +++ b/tools/libxl/libxl_usb.c @@ -651,12 +651,13 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid, usbctrlinfo->devid = usbctrl->devid; -#define READ_SUBPATH(path, subpath) ({ \ -rc = libxl__xs_read_checked(gc, XBT_NULL, \ -GCSPRINTF("%s/" subpath, path), \ -&tmp); \ -if (rc) goto out; \ -(char *)tmp;\ +#define READ_SUBPATH(path, subpath) ({ \ +tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/" subpath, path)); \ +if (!tmp) { \ +rc = ERROR_FAIL;\ +goto out; \ +} \ +(char *)tmp;\ }) #define READ_SUBPATH_INT(path, subpath) ({ \ -- 2.10.2 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
On 07/02/17 11:09, Jan Beulich wrote: > >> @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long field, >> unsigned long *value) >> return okay; >> } >> >> +static always_inline unsigned long vmwrite_safe(unsigned long field, >> +unsigned long value) >> +{ >> +unsigned long ret = 0; >> +bool fail_invalid, fail_valid; >> + >> +asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t", >> + VMWRITE_OPCODE MODRM_EAX_ECX) >> + ASM_FLAG_OUT(, "setc %[invalid]\n\t") >> + ASM_FLAG_OUT(, "setz %[valid]\n\t") >> + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), >> + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) >> + : [field] GAS_VMX_OP("r", "a") (field), >> + [value] GAS_VMX_OP("rm", "c") (value)); >> + >> +if ( unlikely(fail_invalid) ) >> +ret = VMX_INSN_FAIL_INVALID; >> +else if ( unlikely(fail_valid) ) >> +__vmread(VM_INSTRUCTION_ERROR, &ret); >> + >> +return ret; >> +} > ... allow the function to return enum vmx_insn_errno, and that > to not be a 64-bit quantity. As you're presumably aware, dealing > with 32-bit quantities is on the average slightly more efficient than > dealing with 64-bit ones. The code above should imo still BUG() if > the value read from VM_INSTRUCTION_ERROR doesn't fit in 32 > bits (as it's a 32-bit field only anyway). > > Also, looking at the entire asm() above another time, wouldn't it > end up a bit less convoluted if you simply used CMOVC for the > "invalid" code path? Similarly I wonder whether the second > VMREAD wouldn't better be moved into the asm(), utilizing the > UNLIKELY_START() et al constructs to get that code path > entirely out of the main path. These aren't requirements though, > just aspects to think about. Embedding two vm*** instruction is substantially harder in the non GAS_VMX_OP() case. It either involves manual register scheduling, or a separate ModRM and different explicit register fields. As for extra logic, I have some further plans which would allow the compiler to elide the __vmread() on some paths, which it can only for logic exposed in C. From this point of view, the less code in the asm block, the better. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/28] ARM: GICv3 ITS: introduce ITS command handling
Hi Andre, Continuing the review where I left it yesterday. On 30/01/2017 18:31, Andre Przywara wrote: [...] +/* Wait for an ITS to become quiescient (all ITS operations completed). */ +static int gicv3_its_wait_quiescient(struct host_its *hw_its) +{ +uint32_t reg; +s_time_t deadline = NOW() + MILLISECS(1000); + +reg = readl_relaxed(hw_its->its_base + GITS_CTLR); +if ( (reg & (GITS_CTLR_QUIESCENT | GITS_CTLR_ENABLE)) == GITS_CTLR_QUIESCENT ) It would be clearer if you rewrite this: (reg & GITS_CTLR_QUIESCENT) && !(reg & GITS_CTLR_ENABLE). +return 0; + +writel_relaxed(reg & ~GITS_CTLR_ENABLE, hw_its->its_base + GITS_CTLR); It feels a bit odd to disable the ITS in a function containing "wait" in the name. You may want to rename the function to reflect the behavior. + +do { +reg = readl_relaxed(hw_its->its_base + GITS_CTLR); +if ( reg & GITS_CTLR_QUIESCENT ) +return 0; + +cpu_relax(); +udelay(1); +} while ( NOW() <= deadline ); + +dprintk(XENLOG_ERR, "ITS not quiescient\n"); +return -ETIMEDOUT; +} + static unsigned int max_its_device_bits = CONFIG_MAX_PHYS_ITS_DEVICE_BITS; integer_param("max_its_device_bits", max_its_device_bits); int gicv3_its_init(struct host_its *hw_its) { uint64_t reg; -int i; +int i, ret; hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); if ( !hw_its->its_base ) return -ENOMEM; +ret = gicv3_its_wait_quiescient(hw_its); +if ( ret ) +return ret; + +reg = readq_relaxed(hw_its->its_base + GITS_TYPER); +if ( reg & GITS_TYPER_PTA ) +hw_its->flags |= HOST_ITS_USES_PTA; + for ( i = 0; i < GITS_BASER_NR_REGS; i++ ) { void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; @@ -196,6 +322,20 @@ int gicv3_its_init(struct host_its *hw_its) return -ENOMEM; writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); [...] diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c index e2fc901..5911b91 100644 --- a/xen/arch/arm/gic-v3-lpi.c +++ b/xen/arch/arm/gic-v3-lpi.c @@ -30,11 +30,31 @@ static struct { unsigned int host_lpi_bits; } lpi_data; +/* Physical redistributor address */ +static DEFINE_PER_CPU(paddr_t, redist_addr); +/* Redistributor ID */ +static DEFINE_PER_CPU(int, redist_id); s/int/unsigned int/ /* Pending table for each redistributor */ static DEFINE_PER_CPU(void *, pending_table); Rather than defining 3 per-cpu variables, could we merge all in a single structure? #define MAX_PHYS_LPIS (BIT_ULL(lpi_data.host_lpi_bits) - LPI_OFFSET) +/* Stores this redistributor's physical address and ID in a per-CPU variable */ +void gicv3_set_redist_address(paddr_t address, int redist_id) s/int/unsigned int/ +{ +this_cpu(redist_addr) = address; +this_cpu(redist_id) = redist_id; +} + +/* Returns a redistributor's ID (either as an address or as an ID) */ +uint64_t gicv3_get_redist_address(int cpu, bool use_pta) s/int/unsigned int/ +{ +if ( use_pta ) +return per_cpu(redist_addr, cpu) & GENMASK(51, 16); +else +return per_cpu(redist_id, cpu) << 16; What if the function is called before the CPU has been setup? If it cannot happen, please document it. +} + uint64_t gicv3_lpi_allocate_pendtable(void) { uint64_t reg; diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index 440c079..5f825a6 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -644,7 +644,7 @@ static int gicv3_rdist_init_lpis(void __iomem * rdist_base) return -ENOMEM; writeq_relaxed(table_reg, rdist_base + GICR_PROPBASER); -return 0; +return gicv3_its_setup_collection(smp_processor_id()); } static int __init gicv3_populate_rdist(void) @@ -692,7 +692,21 @@ static int __init gicv3_populate_rdist(void) if ( typer & GICR_TYPER_PLPIS ) { -int ret; +paddr_t rdist_addr; +int procnum, ret; procnum should be unsigned. + +rdist_addr = gicv3.rdist_regions[i].base; +rdist_addr += ptr - gicv3.rdist_regions[i].map_base; +procnum = (typer & GICR_TYPER_PROC_NUM_MASK); +procnum >>= GICR_TYPER_PROC_NUM_SHIFT; + +/* + * The ITS refers to redistributors either by their physical + * address or by their ID. Determine those two values and + * let the ITS code store them in per host CPU variables to + * later be able to address those redistributors. + */ This comment does not look useful and is misleading as the code to get/set the redistributor information is living in gic-v3-lpi.c and not gic-v3-its.c. +gicv3_set_redist_address(rdist_addr, procnum); ret = gicv3_rdist_ini
Re: [Xen-devel] [DRAFT C] PVH CPU hotplug design document
Hello Al, Thanks for your comments, please see below. On Mon, Feb 06, 2017 at 04:06:45PM -0700, Al Stone wrote: > On 01/24/2017 07:20 AM, Boris Ostrovsky wrote: > > > >> Yes, the only remaining risk is some vendor using _SB.XEN0, and AFAICT > >> there's > >> no way to reserve anything in there (mostly because it's assumed that ACPI > >> tables will be created by a single entity I guess). > >> > >> I think that the chance of this happening is 0%, and that there's no single > >> system out there with a _SB.XEN0 node. I've been wondering whether I > >> should try > >> to post this to the ACPI working group, and try to get some feedback there. > > > > If you end up asking there, I'd suggest including Rafael Wysocki and Len > > Brown (raf...@kernel.org and l...@kernel.org) and maybe > > linux-a...@vger.kernel.org as well. > > > > -boris > > > > My apologies for not leaping into this discussion earlier; real life has been > somewhat complicated lately. Hopefully I won't annoy too many people. > > So, I am on the ASWG (ACPI Spec Working Group) as a Red Hat and/or Linaro > representative. To clarify something mentioned quite some time ago, the STAO > and XENV tables are in the ACPI in a special form. Essentially, there are two > classes of tables within ACPI: official tables defined in the spec itself that > are meant to be used anywhere ACPI is used, and, tables whose names are to be > recognized but whose content is defined elsewhere. The STAO and XENV belong > to this second class -- the spec reserved their signatures so that others do > not use them, but then points to an external source -- Xen, specifically -- > for > the definition. The practical implication is that Xen can change definitions > as they wish, without direct oversight of the ASWG. Just the same, it is > considered bad form to do so, however, so new revisions should at least be > sent > to the ASWG for discussion (it may make sense to pull the table into the spec > itself...). Stefano and I worked together to get the original reservation > made > for the STAO and XENV tables. > > The other thing I've noticed so far in the discussion is that everything > discussed may work on x86 or ia64, but will not work at all on arm64. The > HARDWARE_REDUCED flag in the FADT was mentioned -- this is the crux of the > problem. For arm64, that flag is required to be set, so overloading it is > most > definitely an issue. More problematic, however, is the notion of using GPE > blocks; when the HARDWARE_REDUCED flag is set, the spec requires GPE block > definitions are to be ignored. Yes, this document is specific to x86. I believe that the difference between x86 and ARM regarding ACPI would make it too complicated to come up with a solution that's usable on both, mainly because ACPI tables on ARM and x86 are already too different. > Then it gets messy :). The APIC and/or x2APIC subtables of the MADT are not > likely to exist on arm64; chances are just about zero, actually. There are > other similar MADT subtables for arm64, but APIC, x2APIC and many more just > won't be there. There is some overlap with ia64, but not entirely. ia64 is also out of the picture here, the more that Xen doesn't support it, and it doesn't look like anyone is working on it. > The other issue is that a separate name space for the added CPUs would have > to be very carefully done. If not, then the processor hierarchy information > in the AML either becomes useless, or at the least inconsistent, and OSPMs > are just now beginning to use some of that info to make scheduling decisions. > It would be possible to just assume the hot plug CPUs are outside of any > existing processor hierarchy, but I would then worry that power management > decisions made by the OSPM might be wrong; I can imagine a scenario where > a CPU is inserted and shares a power rail with an existing CPU, but the > existing CPU is idle so it decides to power off since it's the last in the > hierarchy, so the power rail isn't needed, and now the power gets turned off > to the unit just plugged in because the OSPM doesn't realize it shares power. Well, my suggestion was to add the processor objects of the virtual CPUs inside an ACPI Module Device that has the _SB.XEN0 namespace. However, AFAIK there's no way to reserve the _SB.XEN0 namespace, so a vendor could use that for something else. I think the chances of that happening are very low, but it's not impossible. Is there anyway in ACPI to reserve a namespace for a certain usage? (ie: would it be possible to somehow reserve _SB.XEN0 for Xen usage?) Or if we want to go more generic, we could reserve _SB.VIRT for generic hypervisor usage. > So at a minimum, it sounds like there would need to be a solution for each > architecture, with maybe some fiddling around on ia64, too. Unfortunately, > I believe the ACPI spec provides a way to handle all of the things wanted, > but an ASL interpreter would be required because it does rely on exec
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On Tue, Feb 7, 2017 at 4:58 PM, Julien Grall wrote: > On 07/02/2017 11:10, Vijay Kilari wrote: >> >> On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall wrote: >>> >>> On 07/02/2017 08:18, Vijay Kilari wrote: I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. >>> >>> >>> >>> I am a bit confused by what you are saying. Xen on ARM does not yet >>> support >>> NUMA. I also know you are working on NUMA support. So does the BUG happen >>> on >>> upstream xen or upstream xen + your patches? >> >> >> I was testing with Andre ITS patches (RFC version 1) + my NUMA patches >> + upstream xen. >> However now I tested with upstream xen + Andre ITS patches (staging >> branch) on NUMA board. > > > The RFC v1 is quite an old version. Please give a try using the latest > version [1]. > >> I see panic (similar to what I see with my patches). Log are here. > > > Well the panic is different now. An ASSERT in list_del is hit this time. > This looks like a memory corruption to me. > >> >> http://pastebin.com/QJqUBvD9 >> >> The same plain upstream xen + Andre ITS patches boots fine with non-NUMA >> board. > > > I know that DOM0 cannot boot without ITS on your platform. But as you don't > reach DOM0, have you tried to boot without the ITS series on NUMA board? Yes, without ITS patches it is previously (first) reported panic at "Xen BUG at page_alloc.c:827" > > Regards, > > [1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg03276.html > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On 07/02/2017 12:41, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 4:58 PM, Julien Grall wrote: On 07/02/2017 11:10, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall wrote: On 07/02/2017 08:18, Vijay Kilari wrote: I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. I am a bit confused by what you are saying. Xen on ARM does not yet support NUMA. I also know you are working on NUMA support. So does the BUG happen on upstream xen or upstream xen + your patches? I was testing with Andre ITS patches (RFC version 1) + my NUMA patches + upstream xen. However now I tested with upstream xen + Andre ITS patches (staging branch) on NUMA board. The RFC v1 is quite an old version. Please give a try using the latest version [1]. I see panic (similar to what I see with my patches). Log are here. Well the panic is different now. An ASSERT in list_del is hit this time. This looks like a memory corruption to me. http://pastebin.com/QJqUBvD9 The same plain upstream xen + Andre ITS patches boots fine with non-NUMA board. I know that DOM0 cannot boot without ITS on your platform. But as you don't reach DOM0, have you tried to boot without the ITS series on NUMA board? Yes, without ITS patches it is previously (first) reported panic at "Xen BUG at page_alloc.c:827" Can you please paste the full log from xen upstream (no debug, no ITS, no NUMA) and device tree memory node? Also, please disable CONFIG_DEBUG_DEVICE_TREE it pollutes the logs and I don't think the option is necessary to solve this problem. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On 07/02/2017 12:47, Julien Grall wrote: On 07/02/2017 12:41, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 4:58 PM, Julien Grall wrote: On 07/02/2017 11:10, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall wrote: On 07/02/2017 08:18, Vijay Kilari wrote: I am seeing below panic with NUMA during DT mappings in alloc_heap_pages() BUG_ON(pg[i].count_info != PGC_state_free); However, this issue is not there with 4.7 version. The same NUMA board boots fine. I am a bit confused by what you are saying. Xen on ARM does not yet support NUMA. I also know you are working on NUMA support. So does the BUG happen on upstream xen or upstream xen + your patches? I was testing with Andre ITS patches (RFC version 1) + my NUMA patches + upstream xen. However now I tested with upstream xen + Andre ITS patches (staging branch) on NUMA board. The RFC v1 is quite an old version. Please give a try using the latest version [1]. I see panic (similar to what I see with my patches). Log are here. Well the panic is different now. An ASSERT in list_del is hit this time. This looks like a memory corruption to me. http://pastebin.com/QJqUBvD9 The same plain upstream xen + Andre ITS patches boots fine with non-NUMA board. I know that DOM0 cannot boot without ITS on your platform. But as you don't reach DOM0, have you tried to boot without the ITS series on NUMA board? Yes, without ITS patches it is previously (first) reported panic at "Xen BUG at page_alloc.c:827" Can you please paste the full log from xen upstream (no debug, no ITS, no NUMA) and device tree memory node? Also, please disable CONFIG_DEBUG_DEVICE_TREE it pollutes the logs and I don't think the option is necessary to solve this problem. One more thing, if Xen 4.7 was able to go up to booting Dom0 without any patches on a NUMA board. I would recommend to try to bisect and see if you can find an offending commit. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] dom0pvh issue with XEN 4.8.0
On Mon, Feb 06, 2017 at 06:25:57PM +0800, G.R. wrote: > On Mon, Feb 6, 2017 at 5:33 PM, Pasi Kärkkäinen wrote: > > Hi, > > > > On Sun, Feb 05, 2017 at 04:05:32PM +0800, G.R. wrote: > >> Hi all, > >> dom0pvh=1 is not working well for me with XEN 4.8.0 + linux kernel 4.9.2. > >> > >> The system boots with no obvious issue. > >> But many user mode application are suffering from segfault, which > >> makes the dom0 not useable: The segfault always come from libc-2.24.so > >> while it works just fine in PV dom0. > >> I have no idea why, but those segfault would kill my ssh connection > >> while sshd is not showing up in the victim list. > >> > >> Some examples: > >> Feb 5 14:25:28 gaia kernel: [ 123.446346] getty[3044]: segfault at 0 > >> ip 7f5e769e6c60 sp 7ffc57bc0a98 error 6 in > >> libc-2.24.so[7f5e769b7000+195000] > >> Feb 5 14:29:04 gaia kernel: [ 339.671742] grep[4195]: segfault at 0 > >> ip 7f5d3b95ac60 sp 7ffcc1620bb8 error 6 in > >> libc-2.24.so[7f5d3b92b000+195000] > >> Feb 5 14:29:23 gaia kernel: [ 358.495888] tail[4203]: segfault at 0 > >> ip 7f751314bc60 sp 7fffe5ce5e48 error 6 in > >> libc-2.24.so[7f751311c000+195000] > >> Feb 5 14:35:06 gaia kernel: [ 701.314247] bash[4323]: segfault at 0 > >> ip 7f3fef30ec60 sp 7ffd48cc2058 error 6 in > >> libc-2.24.so[7f3fef2df000+195000] > >> Feb 5 14:48:43 gaia kernel: [ 1518.809924] ls[4910]: segfault at 0 ip > >> 7f29e9bc1c60 sp 7ffd712752b8 error 6 in > >> libc-2.24.so[7f29e9b92000+195000] > >> > >> Any suggestion on how to get this fixed? > >> I don't think I can do live debug since the userspace is quite unstable. > >> On the other hand, dmesg from both dom0 && XEN looks just fine. > >> > >> PS: I'm using a custom compiled dom0 kernel. Is there any specific > >> kernel config is required to get dom0pvh=1 work? > >> > > > > I think the plan is to replace/rewrite the PVH (dom0) support with PVHv2, > > see Roger's recent series here on xen-devel mailinglist.. > > > > Thanks for all your input. > Just had another check on the feature sheet, really didn't notice that > PVH is still an 'preview' feature. > Had the wrong impression since the feature had been announced for 2~3 > years anyway. > Will avoid this for the moment. Hello, Yes, it's best to avoid it for the time being, but I'm still concerned by this. The classic PVH mode is used by FreeBSD, and I have never received any reports of SEGFAULTs in user-space applications, so it looks like an issue on the Linux kernel and not Xen itself. Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
>>> On 07.02.17 at 12:59, wrote: > On 07/02/17 11:09, Jan Beulich wrote: >> >>> @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long >>> field, > unsigned long *value) >>> return okay; >>> } >>> >>> +static always_inline unsigned long vmwrite_safe(unsigned long field, >>> +unsigned long value) >>> +{ >>> +unsigned long ret = 0; >>> +bool fail_invalid, fail_valid; >>> + >>> +asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t", >>> + VMWRITE_OPCODE MODRM_EAX_ECX) >>> + ASM_FLAG_OUT(, "setc %[invalid]\n\t") >>> + ASM_FLAG_OUT(, "setz %[valid]\n\t") >>> + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), >>> + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) >>> + : [field] GAS_VMX_OP("r", "a") (field), >>> + [value] GAS_VMX_OP("rm", "c") (value)); >>> + >>> +if ( unlikely(fail_invalid) ) >>> +ret = VMX_INSN_FAIL_INVALID; >>> +else if ( unlikely(fail_valid) ) >>> +__vmread(VM_INSTRUCTION_ERROR, &ret); >>> + >>> +return ret; >>> +} >> ... allow the function to return enum vmx_insn_errno, and that >> to not be a 64-bit quantity. As you're presumably aware, dealing >> with 32-bit quantities is on the average slightly more efficient than >> dealing with 64-bit ones. The code above should imo still BUG() if >> the value read from VM_INSTRUCTION_ERROR doesn't fit in 32 >> bits (as it's a 32-bit field only anyway). >> >> Also, looking at the entire asm() above another time, wouldn't it >> end up a bit less convoluted if you simply used CMOVC for the >> "invalid" code path? Similarly I wonder whether the second >> VMREAD wouldn't better be moved into the asm(), utilizing the >> UNLIKELY_START() et al constructs to get that code path >> entirely out of the main path. These aren't requirements though, >> just aspects to think about. > > Embedding two vm*** instruction is substantially harder in the non > GAS_VMX_OP() case. It either involves manual register scheduling, or a > separate ModRM and different explicit register fields. Ah, right, that wouldn't be very nice indeed. > As for extra logic, I have some further plans which would allow the > compiler to elide the __vmread() on some paths, which it can only for > logic exposed in C. From this point of view, the less code in the asm > block, the better. Well, okay then. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 105604: all pass - PUSHED
flight 105604 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/105604/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf a316d7ac91d302085b5c35d76db703f2208ec026 baseline version: ovmf 7c609a144b6636577dd60fbaa5fc64efeecd7baf Last test of basis 105599 2017-02-07 09:08:58 Z0 days Testing same since 105604 2017-02-07 11:47:02 Z0 days1 attempts People who touched revisions under test: Laszlo Ersek jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : + branch=ovmf + revision=a316d7ac91d302085b5c35d76db703f2208ec026 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x '!=' x/home/osstest/repos/lock ']' ++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock ++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf a316d7ac91d302085b5c35d76db703f2208ec026 + branch=ovmf + revision=a316d7ac91d302085b5c35d76db703f2208ec026 + . ./cri-lock-repos ++ . ./cri-common +++ . ./cri-getconfig +++ umask 002 +++ getrepos getconfig Repos perl -e ' use Osstest; readglobalconfig(); print $c{"Repos"} or die $!; ' +++ local repos=/home/osstest/repos +++ '[' -z /home/osstest/repos ']' +++ '[' '!' -d /home/osstest/repos ']' +++ echo /home/osstest/repos ++ repos=/home/osstest/repos ++ repos_lock=/home/osstest/repos/lock ++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']' + . ./cri-common ++ . ./cri-getconfig ++ umask 002 + select_xenbranch + case "$branch" in + tree=ovmf + xenbranch=xen-unstable + '[' xovmf = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-unstable + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable + prevxenbranch=xen-4.8-testing + '[' xa316d7ac91d302085b5c35d76db703f2208ec026 = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git ++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops
Re: [Xen-devel] [PATCH] page_alloc: clear nr_bootmem_regions in end_boot_allocator()
>>> On 07.02.17 at 12:38, wrote: > On 02/02/17 16:20, Jan Beulich wrote: > On 02.02.17 at 16:41, wrote: >>> On 02/02/17 15:25, Jan Beulich wrote: --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -329,13 +329,16 @@ unsigned long __init alloc_boot_pages( unsigned long nr_pfns, unsigned long pfn_align) { unsigned long pg, _e; -int i; +unsigned int i = nr_bootmem_regions; -for ( i = nr_bootmem_regions - 1; i >= 0; i-- ) +BOOT_BUG_ON(!nr_bootmem_regions); >>> Can this just be a plain BUG_ON() to avoid adding further work which >>> needs to undone for livepatching purposes? >> Well, for one I don't like adding inconsistency here. And then I'm >> not convinced switching over to BUG_ON() is a good idea, so I'd >> rather leave that discussion for when someone indeed wants to >> make that change. In particular I'm not convinced that during >> very early boot all the register and stack dumping functions >> reliably, in which case a simple panic() is more likely to produce >> at least no confusing output. > > Well - the change is definitely needed. BOOT_BUG_ON() has an embedded > __LINE__ which causes problems making livepatches. There are ways other than converting to BUG_ON() to deal with that, e.g. allowing a non-ambiguous string to be handed (e.g. ASSERT()-like) and included in the panic() output. > The early register/stack functions should work correctly. I did test > that when rearranging the x86 IDT handling several releases ago. What about page walks? And ARM? > As to consistency, I would prefer if the situation wasn't made worse, > but if you really insist, then my R-by stands. Thanks. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On Tue, Feb 7, 2017 at 6:30 PM, Julien Grall wrote: > > > On 07/02/2017 12:47, Julien Grall wrote: >> >> >> >> On 07/02/2017 12:41, Vijay Kilari wrote: >>> >>> On Tue, Feb 7, 2017 at 4:58 PM, Julien Grall >>> wrote: On 07/02/2017 11:10, Vijay Kilari wrote: > > > On Tue, Feb 7, 2017 at 3:37 PM, Julien Grall > wrote: >> >> >> On 07/02/2017 08:18, Vijay Kilari wrote: >>> >>> >>>I am seeing below panic with NUMA during DT mappings in >>> alloc_heap_pages() >>> BUG_ON(pg[i].count_info != PGC_state_free); >>> However, this issue is not there with 4.7 version. The same NUMA >>> board >>> boots fine. >> >> >> >> >> I am a bit confused by what you are saying. Xen on ARM does not yet >> support >> NUMA. I also know you are working on NUMA support. So does the BUG >> happen >> on >> upstream xen or upstream xen + your patches? > > > > I was testing with Andre ITS patches (RFC version 1) + my NUMA patches > + upstream xen. > However now I tested with upstream xen + Andre ITS patches (staging > branch) on NUMA board. The RFC v1 is quite an old version. Please give a try using the latest version [1]. > I see panic (similar to what I see with my patches). Log are here. Well the panic is different now. An ASSERT in list_del is hit this time. This looks like a memory corruption to me. > > http://pastebin.com/QJqUBvD9 > > The same plain upstream xen + Andre ITS patches boots fine with > non-NUMA > board. I know that DOM0 cannot boot without ITS on your platform. But as you don't reach DOM0, have you tried to boot without the ITS series on NUMA board? >>> >>> >>> Yes, without ITS patches it is previously (first) reported panic at >>> "Xen BUG at page_alloc.c:827" >> >> >> Can you please paste the full log from xen upstream (no debug, no ITS, >> no NUMA) and device tree memory node? >> >> Also, please disable CONFIG_DEBUG_DEVICE_TREE it pollutes the logs and I >> don't think the option is necessary to solve this problem. >> DT Memory node info: memory@0 { device_type = "memory"; numa-node-id = <0x0>; reg = <0x0 0x140 0xf 0xfec0>; }; memory@100 { device_type = "memory"; numa-node-id = <0x1>; reg = <0x100 0x40 0xf 0xffc0>; }; Log: Xen 4.9-unstable (c/s Mon Feb 6 13:54:03 2017 + git:2733b80) EFI loader Image_xen_memfix: 0x010ff57b3000-0x010ff6617200 - UART enabled - - CPU booting - - Current EL 0008 - - Xen starting at EL2 - - Zero BSS - - Setting up control registers - - Turning on paging - - Ready - (XEN) Checking for initrd in /chosen (XEN) RAM: 0140 - fffecfff (XEN) RAM: 0001 - 000ff7ff (XEN) RAM: 000ff800 - 000ff801 (XEN) RAM: 000ff802 - 000fffa9cfff (XEN) RAM: 000fffa9d000 - 000f (XEN) RAM: 0140 - 010ff57b2fff (XEN) RAM: 010ff6618000 - 010ff6ff0fff (XEN) RAM: 010ff6ff1000 - 010ff721 (XEN) RAM: 010ff7333000 - 010ff73fbfff (XEN) RAM: 010ff73fc000 - 010ff74defff (XEN) RAM: 010ff74df000 - 010ff9718fff (XEN) RAM: 010ff97a2000 - 010ff97adfff (XEN) RAM: 010ff97bf000 - 010ff97e8fff (XEN) RAM: 010ff97e9000 - 010ff97f0fff (XEN) RAM: 010ff97f1000 - 010ff97f7fff (XEN) RAM: 010ff97f9000 - 010ff9813fff (XEN) RAM: 010ff9814000 - 010ff9819fff (XEN) RAM: 010ff981a000 - 010ff984afff (XEN) RAM: 010ff984c000 - 010ff9851fff (XEN) RAM: 010ff9935000 - 010ffaeb5fff (XEN) RAM: 010ffaff5000 - 010ffb008fff (XEN) RAM: 010ffb009000 - 010fffe28fff (XEN) RAM: 010fffe29000 - 010fffe70fff (XEN) RAM: 010fffe71000 - 010b8fff (XEN) RAM: 010ff000 - 010f (XEN) (XEN) MODULE[0]: 010ff97ae000 - 010ff97bf000 Device Tree (XEN) MODULE[1]: 010ff57b3000 - 010ff6617200 Kernel console=hvc0 earlycon=pl011,0x87e02400 debug=y rw root=/dev/sdb2 xen.fifo_events=0 (XEN) (XEN) Command line: xen_new no-bootscrub loglvl=all iommu=no console=dtuart dtuart=serial0 earlyprintk=pl011,0x87e02400 debug=y maxcpus=8 dom0_mem=16384M dom0_max_vcpus=8 (XEN) Placing Xen at 0x010fffc0-0x010fffe0 (XEN) Update BOOTMOD_XEN from 010ff722-010ff7332d81 => 010fffc0-010fffd12d81 (XEN) PFN compression on bits 24...27 (XEN) Domain heap initialised (XEN) Platform: Generic System (XEN) Looking for dtuart at "serial0", options "" Xen 4.9-unstable (XEN) Xen version 4.9-unstable (ubuntu@) (aarch64-linux-gnu-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.2) 5.4.0 20160609) debug=y Tue Feb 7 08:01:18 EST 2017 (XEN) Latest ChangeSet: Mon Feb 6 13:5
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On 07/02/2017 13:25, Vijay Kilari wrote: On Tue, Feb 7, 2017 at 6:30 PM, Julien Grall wrote: One more thing, if Xen 4.7 was able to go up to booting Dom0 without any patches on a NUMA board. I would recommend to try to bisect and see if you can find an offending commit. Yes, with plain 4.7 panic is not seen Can you please bisect Xen? It could save us a bit of time to understand what's going on. I will look at the log and answer in a separate e-mail. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
On Tue, Feb 07, 2017 at 03:04:32AM -0700, Jan Beulich wrote: On 07.02.17 at 07:48, wrote: >> Some comment from QEMU/KVM code, in /arch/x86/kvm/lapic.c, >> >> int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu) >> { >> /* This may race with setting of irr in __apic_accept_irq() and >> * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq >> * will cause vmexit immediately and the value will be recalculated >> * on the next vmentry. >> */ >> ... >> } >> >> I am afraid, there may be a similar race with setting of vIRR.. > >I think this is unrelated: If another interrupt came in, the highest >set bit in vIRR can only increase. Plus pt_update_irq(), before >returning, calls vlapic_set_irq(), which ought to result in pt_vector's >vIRR bit to be set (either directly or via setting its PIR bit). I.e. the >highest priority interrupt should still have a vector >= pt_vector. I have noticed that pt_vector was 0x38 and the hightest vector was 0x30 when the assertion failed. In the process of debugging pt_update_irq() when I was working on another feature, I noticed that 0x30 is always the vector of IRQ2. I suspect that the source of the periodic timer interrupt is not lapic. If that, pt_update_irq() reads the vioapic entry twice, one in hvm_isa_irq_assert() and the other in pt_irq_vector(). If guest changed the content in viopaic entry between the two read (ie. change the vector from 0x30 to 0x38), the assertion would fail. Do you think it is a reasonable explanation? Thanks, chao > >Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
>>> On 07.02.17 at 07:32, wrote: > On Tue, Feb 07, 2017 at 03:04:32AM -0700, Jan Beulich wrote: > On 07.02.17 at 07:48, wrote: >>> Some comment from QEMU/KVM code, in /arch/x86/kvm/lapic.c, >>> >>> int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu) >>> { >>> /* This may race with setting of irr in __apic_accept_irq() and >>> * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq >>> * will cause vmexit immediately and the value will be recalculated >>> * on the next vmentry. >>> */ >>> ... >>> } >>> >>> I am afraid, there may be a similar race with setting of vIRR.. >> >>I think this is unrelated: If another interrupt came in, the highest >>set bit in vIRR can only increase. Plus pt_update_irq(), before >>returning, calls vlapic_set_irq(), which ought to result in pt_vector's >>vIRR bit to be set (either directly or via setting its PIR bit). I.e. the >>highest priority interrupt should still have a vector >= pt_vector. > > I have noticed that pt_vector was 0x38 and the hightest vector was 0x30 > when the assertion failed. In the process of debugging pt_update_irq() > when I was working on another feature, I noticed that 0x30 is always > the vector of IRQ2. I suspect that the source of the periodic timer interrupt > is not lapic. If that, pt_update_irq() reads the vioapic entry twice, > one in hvm_isa_irq_assert() and the other in pt_irq_vector(). > If guest changed the content in viopaic entry between the two read > (ie. change the vector from 0x30 to 0x38), the assertion would fail. > Do you think it is a reasonable explanation? IRQ2? If this was going via the PIC, that would be impossible, as that's the cascade pin. IRQ0, otoh, normally arrives at pin2 of the IO-APIC (and we follow this model - see tools/libacpi/build.c). How did you determine that 0x30 is the vector of IRQ2? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RESEND v5 07/24] x86: refactor psr: implement get value flow.
On Tue, Feb 07, 2017 at 10:47:01AM +0800, Yi Sun wrote: > Hi, > > Thanks for reviewing! I agree with your comments except below one. Could you > please check my response? > > On 17-01-31 15:29:34, Konrad Rzeszutek Wilk wrote: > > On Thu, Jan 19, 2017 at 02:01:09PM +0800, Yi Sun wrote: > > > +int psr_get_val(struct domain *d, unsigned int socket, > > > +uint64_t *val, enum cbm_type type) > > > { > > > -return 0; > > > +const struct psr_socket_info *info = get_socket_info(socket); > > > +unsigned int cos = d->arch.psr_cos_ids[socket]; > > > +const struct feat_node *feat; > > > +enum psr_feat_type feat_type; > > > + > > > +if ( IS_ERR(info) ) > > > +return PTR_ERR(info); > > > + > > > +feat_type = psr_cbm_type_to_feat_type(type); > > > +list_for_each_entry(feat, &info->feat_list, list) > > > +{ > > > +if ( feat->feature != feat_type ) > > > +continue; > > > + > > > +if ( feat->ops.get_val(feat, cos, type, val) ) > > > +/* Found */ > > > > No need. The 'psr_get_info' does not have this. > > > > > +return 0; > > > +} > > > + > > > +return -ENOENT; > > > > This function looks quite similar to 'psr_get_info'. > > > > Perhaps it may make sense to have an common one that has an > > extra argument (whether to call get_val or get_feat_info)? > > > > And then psr_get_val and psr_get_info can call in this common > > code with this extra argument attached? > > > Yes, the both functions are almost same. But I feel not good to combine them > to > one function. I think it makes the interface not explicit. As there are only 3 > interfaces exposed by psr.c, I want to keep current implementation. Is that > acceptable to you? Keep the interface as is. You would have _three_ functions: psr_get_val psr_get_info and the one both of them would call which would be called: __psr_get which would do the heavy lifting. > > Thanks, > Sun Yi ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RESEND v5 09/24] x86: refactor psr: set value: assemble features value array.
On Tue, Feb 07, 2017 at 10:51:27AM +0800, Yi Sun wrote: > On 17-01-31 15:57:26, Konrad Rzeszutek Wilk wrote: > > > static int assemble_val_array(uint64_t *val, > > > @@ -550,7 +641,25 @@ static int assemble_val_array(uint64_t *val, > > >const struct psr_socket_info *info, > > >unsigned int old_cos) > > > { > > > -return -EINVAL; > > > +const struct feat_node *feat; > > > +int ret; > > > +uint64_t *val_tmp = val; > > > + > > > +if ( !val ) > > > +return -EINVAL; > > > + > > > +/* Get all features current values according to old_cos. */ > > > +list_for_each_entry(feat, &info->feat_list, list) > > > +{ > > > +/* value getting order is same as feature list */ > > > +ret = feat->ops.get_old_val(val_tmp, feat, old_cos); > > > > Shouldn't we check ret for negative values? > > > + > > > +val_tmp += feat->ops.get_cos_num(feat); > > > +if ( val_tmp - val > array_len) > > > +return -EINVAL; > > > > > > Perhaps: ENOSPC ? > > > > Also this function does do any assembling. It just verifies. > > Perhaps other patches add extra work here? In which case you may > > want to mention that in the commit description. > > > Array "assembling" is done here. But maybe the "assembling" is not accurate. > In this function, I want to get all features old values and put them into > the array according to every feature's position in feature list. combine? > > Do you have more accurate word than "assemble"? Thanks! ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] blkfront resource use
On Mon, Feb 06, 2017 at 03:53:58AM -0700, Jan Beulich wrote: > Konrad, Roger, > > we've recently had a report of stuck I/O (with slightly over a hundred > frontend instances in a single guest), which turned out to be a simple > lack of configured grants on the system. This raises two questions: > > 1) Shouldn't blkfront cope with this situation, by throttling I/O (but not > getting stuck), perhaps by forcing requests to be split (or splitting > them itself) if gnttab_alloc_grant_references() continues to fail over > a measurable time period? Right, this was more problematic to implement before, but now that ARM introduced the code to split bios in blkfront, it shouldn't be that hard to split a bio into smaller ones if not enough grants can be allocated for the whole request. > 2) Isn't the current grant hunger of blkfront a little extreme? An > instance with all defaults can consume 1k grants (32 segs times > 32 reqs), but the scaling with ring size, queue count, and number > of segs can easily get this to 64k or more. A default configured If blkback is using the default values, a single frontend should only use up to 1056 persistent grants (see max_persistent_grants/xen_blkif_max_pgrants) per queue. TBH, I didn't really like the change to account persistent grants per-queue instead of globally [0], because it makes it harder to impose limits on grant utilization. > host, however, allows only for 128k grants (256 frames each > holding 512 entries). Interestingly I've found > https://groups.google.com/forum/#!topic/linux.kernel/N6Q171xkIkM > when looking around - is there a reason this or something similar > never made it into the driver? Without such adjustment a single > spike in I/O can lead to a significant amount of grants to be "lost" > in the queue of a single frontend instance. IIRC we didn't go for that solution and instead implemented a limit in blkback that can be set by the system administrator. But yes, it is still possible for a single blkfront instance to use a huge amount of grants, although only temporarily. When the IO spike is done (ie: the bio is done) blkfront should release the grants. If this is a system doing a huge amount of IO the default amount of grant tables pages should probably be increased. Roger. [0] https://github.com/torvalds/linux/commit/73716df7da4f60dd2d59a9302227d0394f1b8fcc ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 07/28] ARM: GICv3 ITS: introduce device mapping
Hi Andre, On 30/01/2017 18:31, Andre Przywara wrote: The ITS uses device IDs to map LPIs to a device. Dom0 will later use those IDs, which we directly pass on to the host. For this we have to map each device that Dom0 may request to a host ITS device with the same identifier. Allocate the respective memory and enter each device into an rbtree to later be able to iterate over it or to easily teardown guests. Signed-off-by: Andre Przywara --- xen/arch/arm/gic-v3-its.c| 188 ++- xen/arch/arm/vgic-v3.c | 3 + xen/include/asm-arm/domain.h | 3 + xen/include/asm-arm/gic_v3_its.h | 28 ++ 4 files changed, 221 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 6578e8a..4a3a394 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -21,8 +21,10 @@ #include #include #include -#include Why did you drop the include xen/mm.h? +#include +#include #include +#include All the header looks to be included in an alphabetical order except this one. Why? #include #include #include @@ -94,6 +96,21 @@ static int its_send_cmd_mapc(struct host_its *its, int collection_id, int cpu) return its_send_command(its, cmd); } +static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid, + int size, uint64_t itt_addr, bool valid) Please use unsigned for the size. Also it could be uint8_t. Also, itt_addr should technically be a paddr_t. +{ +uint64_t cmd[4]; + +cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32); +cmd[1] = size & GENMASK(4, 0); The mask here and the one below are not necessary and could hide a programming error. It would make much sense to have an ASSERT(size < GITS_TYPER.IDbits) here and someone checking that the value is correct. +cmd[2] = itt_addr & GENMASK(51, 8); For this one, we only want to make sure the bits 7:0 are all zeroes as the address is provided by the caller. So I would replace the mask by an ASSERT(!(its_addr & GENMASK(7, 0)). +if ( valid ) +cmd[2] |= GITS_VALID_BIT; +cmd[3] = 0x00; + +return its_send_command(its, cmd); +} + /* Set up the (1:1) collection mapping for the given host CPU. */ int gicv3_its_setup_collection(int cpu) { @@ -293,6 +310,7 @@ int gicv3_its_init(struct host_its *hw_its) reg = readq_relaxed(hw_its->its_base + GITS_TYPER); if ( reg & GITS_TYPER_PTA ) hw_its->flags |= HOST_ITS_USES_PTA; +hw_its->itte_size = GITS_TYPER_ITT_SIZE(reg); for ( i = 0; i < GITS_BASER_NR_REGS; i++ ) { @@ -339,6 +357,173 @@ int gicv3_its_init(struct host_its *hw_its) return 0; } +static void remove_mapped_guest_device(struct its_devices *dev) +{ +if ( dev->hw_its ) +its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false); I would propagate the return of the ITS command as it may have failed because, for instance, the command queue is full. + +xfree(dev->itt_addr); +xfree(dev); +} + +int gicv3_its_map_guest_device(struct domain *d, int host_devid, + int guest_devid, int bits, bool valid) Please use uint32_t for the host_devid and guest_devid. Also, what is bits? It looks like to me this is the number of bit supported for the event. It think it would be clearer to pass the number of events and compute the number of bits within the function. Furthermore, looking at the code, I think it would be better to have two separate functions: one to add a device, the other to remove. Overall the code looks quite different for both and some parameter are not useful. Lastly, I don't see any code here which prevent a device to be assigned to another domain or even wrong host/guest DeviceID and wrong number of events bits. Who will do that? For checking if the device has been assigned to another, I think this will be done could be done outside. In any case, as requested by Stefano on the previous version, a TODO in the code will be needed to avoid forgetting. +{ +void *itt_addr = NULL; +struct its_devices *dev, *temp; +struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL; +struct host_its *hw_its; +int ret; + +/* check for already existing mappings */ +spin_lock(&d->arch.vgic.its_devices_lock); +while (*new) Coding style: while ( *new ). +{ +temp = rb_entry(*new, struct its_devices, rbnode); + +if ( temp->guest_devid == guest_devid ) +{ +if ( !valid ) +rb_erase(&temp->rbnode, &d->arch.vgic.its_devices); + +spin_unlock(&d->arch.vgic.its_devices_lock); + +if ( valid ) A printk(XENLOG_GUEST...) here would be useful to know which host DeviceID was associated to the guest deviceID. +return -EBUSY; + +remove_mapped_guest_device(temp); See my comment on the function about th
Re: [Xen-devel] blkfront resource use
>>> On 07.02.17 at 14:59, wrote: > On Mon, Feb 06, 2017 at 03:53:58AM -0700, Jan Beulich wrote: >> Interestingly I've found >> https://groups.google.com/forum/#!topic/linux.kernel/N6Q171xkIkM >> when looking around - is there a reason this or something similar >> never made it into the driver? Without such adjustment a single >> spike in I/O can lead to a significant amount of grants to be "lost" >> in the queue of a single frontend instance. > > IIRC we didn't go for that solution and instead implemented a limit in blkback > that can be set by the system administrator. But yes, it is still possible for > a single blkfront instance to use a huge amount of grants, although only > temporarily. When the IO spike is done (ie: the bio is done) blkfront should > release the grants. If this is a system doing a huge amount of IO the default > amount of grant tables pages should probably be increased. What would enforce that releasing? The driver itself doesn't appear to actively do anything here. Is that because the backend would limit the number of grants it keeps mapped (which the frontend then notices, releasing the grants)? In the end, any number of grants not used over an extended periods of time are a waste of resources. Once again, the situation all this came up with was a guest with over a hundred block devices. If for every one of them the driver keeps meaningful amount of grants in its internal queues, there could (in the default config) be over 100k grants no-one can make use of. In the worst case even splitting requests may then not help, when enough grants aren't available for I/O of just a single sector. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-linus test] 105594: regressions - FAIL
flight 105594 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/105594/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-credit2 14 guest-saverestore fail REGR. vs. 59254 test-amd64-amd64-xl 16 guest-saverestore.2 fail REGR. vs. 59254 test-amd64-i386-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-amd64-xl-multivcpu 16 guest-saverestore.2 fail REGR. vs. 59254 test-amd64-i386-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 59254 test-amd64-amd64-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-armhf-armhf-libvirt 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-credit2 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 13 guest-localmigrate fail REGR. vs. 59254 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate fail REGR. vs. 59254 test-amd64-amd64-xl-qemut-debianhvm-amd64 15 guest-localmigrate/x10 fail REGR. vs. 59254 test-armhf-armhf-xl 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-libvirt-xsm 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-arndale 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-multivcpu 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-xsm 6 xen-boot fail REGR. vs. 59254 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-rtds 9 debian-installfail REGR. vs. 59254 test-armhf-armhf-libvirt-raw 6 xen-bootfail baseline untested test-armhf-armhf-xl-vhd 6 xen-bootfail baseline untested test-amd64-i386-libvirt 14 guest-saverestorefail blocked in 59254 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 59254 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64 5 xen-buildfail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail 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-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 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 version targeted for testing: linux8b1b41ee74f9712c355d66dc105bbea663ae0afd baseline version: linux45820c294fe1b1a9df495d57f40585ef2d069a39 Last test of basis59254 2015-07-09 04:20:48 Z 579 days Failing since 59348 2015-07-10 04:24:05 Z 578 days 255 attempts Testing same since 105594 2017-02-07 06:18:56 Z0 days1 attempts 7562 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm fail build-armhf-xsm pass build-i386-xsm pass build-amd64
[Xen-devel] [xen-unstable-smoke test] 105605: tolerable trouble: broken/fail/pass - PUSHED
flight 105605 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/105605/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-pvops 5 kernel-build fail 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 dbc84d2983969bb47d294131ed9e6bbbdc2aec49 baseline version: xen 2733b800c9a2086d46379d3eb3f480eb5fd433ea Last test of basis 105584 2017-02-06 18:19:28 Z0 days Testing same since 105605 2017-02-07 12:01:43 Z0 days1 attempts People who touched revisions under test: David Scott Juergen Gross jobs: build-amd64 pass build-arm64 fail build-armhf pass build-amd64-libvirt pass build-arm64-pvopsfail test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken 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=dbc84d2983969bb47d294131ed9e6bbbdc2aec49 + . ./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 dbc84d2983969bb47d294131ed9e6bbbdc2aec49 + branch=xen-unstable-smoke + revision=dbc84d2983969bb47d294131ed9e6bbbdc2aec49 + . ./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.8-testing + '[' xdbc84d2983969bb47d294131ed9e6bbbdc2aec49 = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
On Tue, Feb 07, 2017 at 06:46:16AM -0700, Jan Beulich wrote: On 07.02.17 at 07:32, wrote: >> On Tue, Feb 07, 2017 at 03:04:32AM -0700, Jan Beulich wrote: >> On 07.02.17 at 07:48, wrote: Some comment from QEMU/KVM code, in /arch/x86/kvm/lapic.c, int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu) { /* This may race with setting of irr in __apic_accept_irq() and * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq * will cause vmexit immediately and the value will be recalculated * on the next vmentry. */ ... } I am afraid, there may be a similar race with setting of vIRR.. >>> >>>I think this is unrelated: If another interrupt came in, the highest >>>set bit in vIRR can only increase. Plus pt_update_irq(), before >>>returning, calls vlapic_set_irq(), which ought to result in pt_vector's >>>vIRR bit to be set (either directly or via setting its PIR bit). I.e. the >>>highest priority interrupt should still have a vector >= pt_vector. >> >> I have noticed that pt_vector was 0x38 and the hightest vector was 0x30 >> when the assertion failed. In the process of debugging pt_update_irq() >> when I was working on another feature, I noticed that 0x30 is always >> the vector of IRQ2. I suspect that the source of the periodic timer interrupt >> is not lapic. If that, pt_update_irq() reads the vioapic entry twice, >> one in hvm_isa_irq_assert() and the other in pt_irq_vector(). >> If guest changed the content in viopaic entry between the two read >> (ie. change the vector from 0x30 to 0x38), the assertion would fail. >> Do you think it is a reasonable explanation? > >IRQ2? If this was going via the PIC, that would be impossible, as >that's the cascade pin. IRQ0, otoh, normally arrives at pin2 of the >IO-APIC (and we follow this model - see tools/libacpi/build.c). How Sorry, I meaned IRQ0. I was confused with IRQ0 and pin2 of the IO-APIC. >did you determine that 0x30 is the vector of IRQ2? By monitoring the write operation to IO-APIC redirection entry. From experimental data, linux kernel always writes vector 0x30 to redirection entry of pin2 of IO-APIC. Thank, chao > >Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 105602: regressions - FAIL
flight 105602 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/105602/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm5 xen-buildfail REGR. vs. 105279 build-amd64-xsm 5 xen-buildfail REGR. vs. 105279 build-amd64 5 xen-buildfail REGR. vs. 105279 build-i3865 xen-buildfail REGR. vs. 105279 build-armhf-xsm 5 xen-buildfail REGR. vs. 105279 build-armhf 5 xen-buildfail REGR. vs. 105279 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 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-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(
[Xen-devel] [PATCH v2] xen-netfront: Improve error handling during initialization
This fixes a crash when running out of grant refs when creating many queues across many netdevs. * If creating queues fails (i.e. there are no grant refs available), call xenbus_dev_fatal() to ensure that the xenbus device is set to the closed state. * If no queues are created, don't call xennet_disconnect_backend as netdev->real_num_tx_queues will not have been set correctly. * If setup_netfront() fails, ensure that all the queues created are cleaned up, not just those that have been set up. * If any queues were set up and an error occurs, call xennet_destroy_queues() to clean up the napi context. * If any fatal error occurs, unregister and destroy the netdev to avoid leaving around a half setup network device. Signed-off-by: Ross Lagerwall --- Changed in V2: * Retested on top of v4.10-rc7 + "xen-netfront: Delete rx_refill_timer in xennet_disconnect_backend()". * Don't move setup_timer as it is not necessary. drivers/net/xen-netfront.c | 33 +++-- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 722fe9f..5399a86 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1823,27 +1823,23 @@ static int talk_to_netback(struct xenbus_device *dev, xennet_destroy_queues(info); err = xennet_create_queues(info, &num_queues); - if (err < 0) - goto destroy_ring; + if (err < 0) { + xenbus_dev_fatal(dev, err, "creating queues"); + if (num_queues > 0) { + goto destroy_ring; + } else { + kfree(info->queues); + info->queues = NULL; + goto out; + } + } /* Create shared ring, alloc event channel -- for each queue */ for (i = 0; i < num_queues; ++i) { queue = &info->queues[i]; err = setup_netfront(dev, queue, feature_split_evtchn); - if (err) { - /* setup_netfront() will tidy up the current -* queue on error, but we need to clean up -* those already allocated. -*/ - if (i > 0) { - rtnl_lock(); - netif_set_real_num_tx_queues(info->netdev, i); - rtnl_unlock(); - goto destroy_ring; - } else { - goto out; - } - } + if (err) + goto destroy_ring; } again: @@ -1933,9 +1929,10 @@ static int talk_to_netback(struct xenbus_device *dev, xenbus_transaction_end(xbt, 1); destroy_ring: xennet_disconnect_backend(info); - kfree(info->queues); - info->queues = NULL; + xennet_destroy_queues(info); out: + unregister_netdev(info->netdev); + xennet_free_netdev(info->netdev); return err; } -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
On Tue, 2017-02-07 at 04:09 -0700, Jan Beulich wrote: > > > > On 06.02.17 at 15:57, wrote: > > > > Any fail during the original __vmwrite() leads to BUG() which can be > > easily exploited from a guest in the nested vmx mode. > > > > The new function returns error code depending on the outcome: > > > > VMsucceed: 0 > > VMfailValid: VM Instruction Error Number > > VMfailInvalid: a new VMX_INSN_FAIL_INVALID > > > > A new macro GAS_VMX_OP is introduced in order to improve the > > readability of asm. Existing ASM_FLAG_OUT macro is reused and copied > > into asm_defns.h > > > > Signed-off-by: Sergey Dyasli > > --- > > Please can you have the revision info for the individual patches > here. I know you've put it in the overview mail, but for reviewers > it's far more useful to (also) be here. > > > --- a/xen/include/asm-x86/hvm/vmx/vmcs.h > > +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h > > @@ -526,6 +526,7 @@ enum vmx_insn_errno > > VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9, > > VMX_INSN_UNSUPPORTED_VMCS_COMPONENT= 12, > > VMX_INSN_VMXON_IN_VMX_ROOT = 15, > > +VMX_INSN_FAIL_INVALID = ~0, > > }; > > The main reason for me to ask for the type change here was to ... > > > @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long > > field, unsigned long *value) > > return okay; > > } > > > > +static always_inline unsigned long vmwrite_safe(unsigned long field, > > +unsigned long value) > > +{ > > +unsigned long ret = 0; > > +bool fail_invalid, fail_valid; > > + > > +asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t", > > + VMWRITE_OPCODE MODRM_EAX_ECX) > > + ASM_FLAG_OUT(, "setc %[invalid]\n\t") > > + ASM_FLAG_OUT(, "setz %[valid]\n\t") > > + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), > > + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) > > + : [field] GAS_VMX_OP("r", "a") (field), > > + [value] GAS_VMX_OP("rm", "c") (value)); > > + > > +if ( unlikely(fail_invalid) ) > > +ret = VMX_INSN_FAIL_INVALID; > > +else if ( unlikely(fail_valid) ) > > +__vmread(VM_INSTRUCTION_ERROR, &ret); > > + > > +return ret; > > +} > > ... allow the function to return enum vmx_insn_errno, and that > to not be a 64-bit quantity. As you're presumably aware, dealing > with 32-bit quantities is on the average slightly more efficient than > dealing with 64-bit ones. The code above should imo still BUG() if > the value read from VM_INSTRUCTION_ERROR doesn't fit in 32 > bits (as it's a 32-bit field only anyway). If I understood correctly, you are suggesting the following change: diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index 24fbbd4..f9b3bf1 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long field, Â return ret; Â } Â -static always_inline unsigned long vmwrite_safe(unsigned long field, -unsigned long value) +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field, +Â Â unsigned long value) Â { Â unsigned long ret = 0; Â bool fail_invalid, fail_valid; @@ -440,11 +440,16 @@ static always_inline unsigned long vmwrite_safe(unsigned long field, Â Â [value] GAS_VMX_OP("rm", "c") (value)); Â Â if ( unlikely(fail_invalid) ) +{ Â ret = VMX_INSN_FAIL_INVALID; +} Â else if ( unlikely(fail_valid) ) +{ Â __vmread(VM_INSTRUCTION_ERROR, &ret); +BUG_ON(ret >= ~0U); +} Â -return ret; +return (enum vmx_insn_errno) ret; Â } And I have noticed one inconsistency: vmwrite_safe() is "always_inline" while vmread_safe() is plain "inline". I believe that plain inline is enough here, what do you think? -- Thanks, Sergey ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [ovmf baseline-only test] 68531: all pass
This run is configured for baseline tests only. flight 68531 ovmf real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/68531/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 7c609a144b6636577dd60fbaa5fc64efeecd7baf baseline version: ovmf 8a399fab0af391945f0eaa251eaf4efe2d71bb3e Last test of basis68519 2017-02-06 07:46:41 Z1 days Testing same since68531 2017-02-07 11:17:52 Z0 days1 attempts People who touched revisions under test: Alexei Alexei Fedorov Evan Lloyd jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. commit 7c609a144b6636577dd60fbaa5fc64efeecd7baf Author: Alexei Date: Mon Feb 6 19:05:54 2017 + ArmPkg/GenericWatchdogDxe: Declare MMIO PCDs as UINT64 PcdGenericWatchdogControlBase & PcdGenericWatchdogRefreshBase are declared as UINT32 values in ArmPkg.dec, but for platforms with addresses in the memory range above 4GB this causes build error F000: Too large PCD value for datum type [UINT32] of PCD gArmTokenSpaceGuid.PcdGenericWatchdogControlBase Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alexei Fedorov Signed-off-by: Evan Lloyd Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=361 Reviewed-by: Ard Biesheuvel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
That way we have one person who can: a) poke other maintainers or pull them in with new drivers are introduced, b) we have one maintainer who can shepherd the patches along instead of depending on the REST maintainers which may be busy with other responsibilities. Acked-by: Stefano Stabellini Acked-by: George Dunlap Acked-by: Wei Liu Signed-off-by: Konrad Rzeszutek Wilk --- v1: Initial patch v2: - Added Acks - changed description in MAINTAINERS - changed file directory in MAINTAINERS --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index fb26be3..bfcbd7b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -304,6 +304,11 @@ X: xen/arch/x86/acpi/lib.c F: xen/drivers/cpufreq/ F: xen/include/acpi/cpufreq/ +PUBLIC INTERFACES +M: Konrad Rzeszutek Wilk +S: Supported +F: xen/include/public/ + QEMU-DM M: Ian Jackson S: Supported -- 2.9.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
On 07/02/17 15:06, Sergey Dyasli wrote: > On Tue, 2017-02-07 at 04:09 -0700, Jan Beulich wrote: > On 06.02.17 at 15:57, wrote: >>> Any fail during the original __vmwrite() leads to BUG() which can be >>> easily exploited from a guest in the nested vmx mode. >>> >>> The new function returns error code depending on the outcome: >>> >>> VMsucceed: 0 >>> VMfailValid: VM Instruction Error Number >>> VMfailInvalid: a new VMX_INSN_FAIL_INVALID >>> >>> A new macro GAS_VMX_OP is introduced in order to improve the >>> readability of asm. Existing ASM_FLAG_OUT macro is reused and copied >>> into asm_defns.h >>> >>> Signed-off-by: Sergey Dyasli >>> --- >> Please can you have the revision info for the individual patches >> here. I know you've put it in the overview mail, but for reviewers >> it's far more useful to (also) be here. >> >>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h >>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h >>> @@ -526,6 +526,7 @@ enum vmx_insn_errno >>> VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9, >>> VMX_INSN_UNSUPPORTED_VMCS_COMPONENT= 12, >>> VMX_INSN_VMXON_IN_VMX_ROOT = 15, >>> +VMX_INSN_FAIL_INVALID = ~0, >>> }; >> The main reason for me to ask for the type change here was to ... >> >>> @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long >>> field, unsigned long *value) >>> return okay; >>> } >>> >>> +static always_inline unsigned long vmwrite_safe(unsigned long field, >>> +unsigned long value) >>> +{ >>> +unsigned long ret = 0; >>> +bool fail_invalid, fail_valid; >>> + >>> +asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t", >>> + VMWRITE_OPCODE MODRM_EAX_ECX) >>> + ASM_FLAG_OUT(, "setc %[invalid]\n\t") >>> + ASM_FLAG_OUT(, "setz %[valid]\n\t") >>> + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), >>> + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) >>> + : [field] GAS_VMX_OP("r", "a") (field), >>> + [value] GAS_VMX_OP("rm", "c") (value)); >>> + >>> +if ( unlikely(fail_invalid) ) >>> +ret = VMX_INSN_FAIL_INVALID; >>> +else if ( unlikely(fail_valid) ) >>> +__vmread(VM_INSTRUCTION_ERROR, &ret); >>> + >>> +return ret; >>> +} >> ... allow the function to return enum vmx_insn_errno, and that >> to not be a 64-bit quantity. As you're presumably aware, dealing >> with 32-bit quantities is on the average slightly more efficient than >> dealing with 64-bit ones. The code above should imo still BUG() if >> the value read from VM_INSTRUCTION_ERROR doesn't fit in 32 >> bits (as it's a 32-bit field only anyway). > If I understood correctly, you are suggesting the following change: > > diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h > b/xen/include/asm-x86/hvm/vmx/vmx.h > index 24fbbd4..f9b3bf1 100644 > --- a/xen/include/asm-x86/hvm/vmx/vmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long > field, > return ret; > } > > -static always_inline unsigned long vmwrite_safe(unsigned long field, > -unsigned long value) > +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field, > + unsigned long value) > { > unsigned long ret = 0; > bool fail_invalid, fail_valid; > @@ -440,11 +440,16 @@ static always_inline unsigned long > vmwrite_safe(unsigned long field, > [value] GAS_VMX_OP("rm", "c") (value)); > > if ( unlikely(fail_invalid) ) > +{ > ret = VMX_INSN_FAIL_INVALID; > +} > else if ( unlikely(fail_valid) ) > +{ > __vmread(VM_INSTRUCTION_ERROR, &ret); > +BUG_ON(ret >= ~0U); I really don't think the BUG_ON() is necessary. If hardware already guarentees to hand us back a 32bit quantity, and if hardware is malfunctioning, we have already lost. Also, this BUG_ON() will prevent inlining the function if alway_inline is reduced to inline (which is a good idea). ~Andrew > +} > > -return ret; > +return (enum vmx_insn_errno) ret; > } > > And I have noticed one inconsistency: vmwrite_safe() is "always_inline" > while vmread_safe() is plain "inline". I believe that plain inline is > enough here, what do you think? > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] blkfront resource use
On Tue, Feb 07, 2017 at 07:13:57AM -0700, Jan Beulich wrote: > >>> On 07.02.17 at 14:59, wrote: > > On Mon, Feb 06, 2017 at 03:53:58AM -0700, Jan Beulich wrote: > >> Interestingly I've found > >> https://groups.google.com/forum/#!topic/linux.kernel/N6Q171xkIkM > >> when looking around - is there a reason this or something similar > >> never made it into the driver? Without such adjustment a single > >> spike in I/O can lead to a significant amount of grants to be "lost" > >> in the queue of a single frontend instance. > > > > IIRC we didn't go for that solution and instead implemented a limit in > > blkback > > that can be set by the system administrator. But yes, it is still possible > > for > > a single blkfront instance to use a huge amount of grants, although only > > temporarily. When the IO spike is done (ie: the bio is done) blkfront should > > release the grants. If this is a system doing a huge amount of IO the > > default > > amount of grant tables pages should probably be increased. > > What would enforce that releasing? The driver itself doesn't appear > to actively do anything here. Is that because the backend would > limit the number of grants it keeps mapped (which the frontend then > notices, releasing the grants)? Yes, see blkif_completion in blkfront (the call to gnttab_query_foreign_access). > In the end, any number of grants not used over an extended periods > of time are a waste of resources. Once again, the situation all this > came up with was a guest with over a hundred block devices. If for > every one of them the driver keeps meaningful amount of grants in > its internal queues, there could (in the default config) be over 100k > grants no-one can make use of. In the worst case even splitting > requests may then not help, when enough grants aren't available > for I/O of just a single sector. I agree. The best way to solve that AFAIK would be to keep a time stamp of when a grant was last used, and have a threshold (let's say 5s), if the grant has not been used for that threshold it should be unmaped by blkback. blkfront should also have a periodic task that scans the list of grants and releases those no longer mapped by blkback. Or maybe we should just disable persistent grants by default, by setting xen_blkif_max_pgrants = 0. This feature doesn't scale well, and interacts badly with other things (like indirect descriptors). The amount of code required to maintain this feature in a sane state is getting quite big. Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] IOMMU fault with IGD passthrough setup on XEN 4.8.0
On Mon, Feb 6, 2017 at 8:40 PM, Jan Beulich wrote: On 05.02.17 at 06:51, wrote: >> I finally get some spare time to collect the debug info. > > As I continue to be puzzled, best I could come up with is an > extension to the debug patch. Please use the attached one > in place of the earlier one, ideally on top of > https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00448.html > to reduce the overall amount of output (and help readability). Please see attached... dmsg2.xz Description: application/xz ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] x86/vvmx: correctly emulate VMREAD
On Tue, 2017-02-07 at 06:52 +, Tian, Kevin wrote: > > From: Sergey Dyasli [mailto:sergey.dya...@citrix.com] > > Sent: Monday, February 06, 2017 10:58 PM > > > > There is an issue with the original __vmread() in nested vmx mode: > > emulation of a guest's VMREAD with invalid arguments leads to BUG(). > > > > Fix this by using vmread_safe() and reporting any kind of VMfail back > > to the guest. > > > > A new safe versions of get_vvmcs() macro and related functions are > > introduced because of new function signatures and lots of existing > > users. > > > > Signed-off-by: Sergey Dyasli > > after reading this patch I realized my earlier ack to 3/4 might not hold, > since now you have mismatched get/set pairs: > > get_vvmcs > get_vvmcs_safe > set_vvmcs > > suggest to also introduce a set_vvmcs_safe counterpart in 3/4, as > there are still many existing callers on set_vvmcs which don't > expect error. It is true that my patch introduces a change in behaviour: invalid set_vvmcs() calls will silently fail instead of triggering BUG(). I agree it would be better to introduce set_vvmcs_safe() for now. -- Thanks, Sergey ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] arm: alloc_heap_pages allocates already allocated page
On Tue, Feb 7, 2017 at 6:57 PM, Julien Grall wrote: > > > On 07/02/2017 13:25, Vijay Kilari wrote: >> >> On Tue, Feb 7, 2017 at 6:30 PM, Julien Grall wrote: >>> >>> >>> One more thing, if Xen 4.7 was able to go up to booting Dom0 without any >>> patches on a NUMA board. I would recommend to try to bisect and see if >>> you >>> can find an offending commit. >> >> >> Yes, with plain 4.7 panic is not seen > > > Can you please bisect Xen? It could save us a bit of time to understand > what's going on. ubuntu@ubuntu:~/xen_upstream_new/xen$ git bisect bad 493f535a06b5b4041c0745e954780dd5d6f80581 is the first bad commit commit 493f535a06b5b4041c0745e954780dd5d6f80581 Author: Julien Grall Date: Thu Sep 15 12:28:36 2016 +0100 xen/arm: p2m: Re-implement p2m_insert_mapping using p2m_set_entry The function p2m_insert_mapping can be re-implemented using the generic function p2m_set_entry. Note that the mapping is not reverted anymore if Xen fails to insert a mapping. This was added to ensure the MMIO are not kept half-mapped in case of failure and to follow the x86 counterpart. This was removed on the x86 part by commit c3c756bd "x86/p2m: use large pages for MMIO mappings" and I think we should let the caller taking care of it. Finally drop the operation INSERT in apply_* as nobody is using it anymore. Note that the functions could have been dropped in one go at the end, however I find easier to drop the operations one by one avoiding a big deletion in the patch that convert the last operation. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini Tested-by: Tamas K Lengyel :04 04 4fb3f181a7b0f9b46f9b573e7a328b4b77dcaa78 d3fc51e4175b86504eac644084ee45a18492641e Mxen > > I will look at the log and answer in a separate e-mail. > > Regards, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()
>>> On 07.02.17 at 08:28, wrote: > On Tue, Feb 07, 2017 at 06:46:16AM -0700, Jan Beulich wrote: > On 07.02.17 at 07:32, wrote: >>> On Tue, Feb 07, 2017 at 03:04:32AM -0700, Jan Beulich wrote: >>> On 07.02.17 at 07:48, wrote: > Some comment from QEMU/KVM code, in /arch/x86/kvm/lapic.c, > > int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu) > { > /* This may race with setting of irr in __apic_accept_irq() and >* value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq >* will cause vmexit immediately and the value will be recalculated >* on the next vmentry. >*/ > ... > } > > I am afraid, there may be a similar race with setting of vIRR.. I think this is unrelated: If another interrupt came in, the highest set bit in vIRR can only increase. Plus pt_update_irq(), before returning, calls vlapic_set_irq(), which ought to result in pt_vector's vIRR bit to be set (either directly or via setting its PIR bit). I.e. the highest priority interrupt should still have a vector >= pt_vector. >>> >>> I have noticed that pt_vector was 0x38 and the hightest vector was 0x30 >>> when the assertion failed. In the process of debugging pt_update_irq() >>> when I was working on another feature, I noticed that 0x30 is always >>> the vector of IRQ2. I suspect that the source of the periodic timer > interrupt >>> is not lapic. If that, pt_update_irq() reads the vioapic entry twice, >>> one in hvm_isa_irq_assert() and the other in pt_irq_vector(). >>> If guest changed the content in viopaic entry between the two read >>> (ie. change the vector from 0x30 to 0x38), the assertion would fail. >>> Do you think it is a reasonable explanation? >> >>IRQ2? If this was going via the PIC, that would be impossible, as >>that's the cascade pin. IRQ0, otoh, normally arrives at pin2 of the >>IO-APIC (and we follow this model - see tools/libacpi/build.c). How > > Sorry, I meaned IRQ0. I was confused with IRQ0 and pin2 of the IO-APIC. > >>did you determine that 0x30 is the vector of IRQ2? > > By monitoring the write operation to IO-APIC redirection entry. > From experimental data, linux kernel always writes vector 0x30 to > redirection entry of pin2 of IO-APIC. But if it goes through the IO-APIC, it'll also go through the LAPIC (and in the ExtInt case you wouldn't be able to read a valid vector from the RTE). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
>>> On 07.02.17 at 16:06, wrote: > If I understood correctly, you are suggesting the following change: Mostly. > --- a/xen/include/asm-x86/hvm/vmx/vmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long > field, > return ret; > } > > -static always_inline unsigned long vmwrite_safe(unsigned long field, > -unsigned long value) > +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field, > + unsigned long value) > { > unsigned long ret = 0; > bool fail_invalid, fail_valid; > @@ -440,11 +440,16 @@ static always_inline unsigned long > vmwrite_safe(unsigned long field, > [value] GAS_VMX_OP("rm", "c") (value)); > > if ( unlikely(fail_invalid) ) > +{ > ret = VMX_INSN_FAIL_INVALID; > +} No need to add braces here and ... > else if ( unlikely(fail_valid) ) > +{ > __vmread(VM_INSTRUCTION_ERROR, &ret); > +BUG_ON(ret >= ~0U); > +} > > -return ret; > +return (enum vmx_insn_errno) ret; ... no need for the cast here. (See Andrew's reply for the BUG_ON().) > And I have noticed one inconsistency: vmwrite_safe() is "always_inline" > while vmread_safe() is plain "inline". I believe that plain inline is > enough here, what do you think? I would assume plain inline to be enough, but maybe the VMX maintainers know why always_inline was used. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 105600: tolerable FAIL - PUSHED
flight 105600 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/105600/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-raw 3 host-install(3) broken in 105589 pass in 105600 test-amd64-i386-qemut-rhel6hvm-amd 3 host-install(3) broken in 105589 pass in 105600 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 105589 pass in 105600 test-armhf-armhf-xl-xsm 15 guest-start/debian.repeat fail pass in 105589 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail pass in 105589 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 105576 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 105576 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 105576 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 105576 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 105576 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 105576 test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 105576 test-amd64-amd64-xl-rtds 9 debian-install fail like 105576 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 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-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass build-arm64 5 xen-buildfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass build-arm64-pvops 5 kernel-build fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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 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 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass version targeted for testing: xen 2733b800c9a2086d46379d3eb3f480eb5fd433ea baseline version: xen 55a04feaa1f8ab6ef7d723fbb1d39c6b96ad184a Last test of basis 105576 2017-02-06 13:16:36 Z1 days Testing same since 105589 2017-02-07 00:14:29 Z0 days2 attempts -
Re: [Xen-devel] blkfront resource use
>>> On 07.02.17 at 16:24, wrote: > Or maybe we should just disable persistent grants by default, by setting > xen_blkif_max_pgrants = 0. This feature doesn't scale well, and interacts > badly > with other things (like indirect descriptors). The amount of code required to > maintain this feature in a sane state is getting quite big. This sounds very reasonable. In fact I've been surprised that one can't even disable it in the frontend; I had to create a patch to add a respective command line option. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
>>> On 07.02.17 at 16:28, wrote: > v2: - Added Acks > - changed description in MAINTAINERS > - changed file directory in MAINTAINERS Are you sure? I can't see a difference to v1, so my earlier comment still holds. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
On 07/02/17 16:22, Jan Beulich wrote: On 07.02.17 at 16:06, wrote: >> If I understood correctly, you are suggesting the following change: > Mostly. > >> --- a/xen/include/asm-x86/hvm/vmx/vmx.h >> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h >> @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long >> field, >> return ret; >> } >> >> -static always_inline unsigned long vmwrite_safe(unsigned long field, >> -unsigned long value) >> +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field, >> + unsigned long value) >> { >> unsigned long ret = 0; >> bool fail_invalid, fail_valid; >> @@ -440,11 +440,16 @@ static always_inline unsigned long >> vmwrite_safe(unsigned long field, >> [value] GAS_VMX_OP("rm", "c") (value)); >> >> if ( unlikely(fail_invalid) ) >> +{ >> ret = VMX_INSN_FAIL_INVALID; >> +} > No need to add braces here and ... > >> else if ( unlikely(fail_valid) ) >> +{ >> __vmread(VM_INSTRUCTION_ERROR, &ret); >> +BUG_ON(ret >= ~0U); >> +} >> >> -return ret; >> +return (enum vmx_insn_errno) ret; > ... no need for the cast here. (See Andrew's reply for the BUG_ON().) > >> And I have noticed one inconsistency: vmwrite_safe() is "always_inline" >> while vmread_safe() is plain "inline". I believe that plain inline is >> enough here, what do you think? > I would assume plain inline to be enough, but maybe the VMX > maintainers know why always_inline was used. The always_inline was my doing IIRC, because the use of unlikely sections caused GCC to create a separate identical functions in each translation unit, in an attempt to minimise the quantity of out-of-line code. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
On Tue, Feb 07, 2017 at 09:31:29AM -0700, Jan Beulich wrote: > >>> On 07.02.17 at 16:28, wrote: > > v2: - Added Acks > > - changed description in MAINTAINERS > > - changed file directory in MAINTAINERS > > Are you sure? I can't see a difference to v1, so my earlier comment > still holds. Argh. I forgot to actually do the git add part! Here it is inline: From df509f240389fc6d6bb164731d32e6f60f98c297 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Fri, 18 Nov 2016 11:18:24 -0500 Subject: [PATCH v3] MAINTAINERS: Add myself as the public API "Czar" That way we have one person who can: a) poke other maintainers or pull them in with new drivers are introduced, b) we have one maintainer who can shepherd the patches along instead of depending on the REST maintainers which may be busy with other responsibilities. Acked-by: Stefano Stabellini Acked-by: George Dunlap Acked-by: Wei Liu Signed-off-by: Konrad Rzeszutek Wilk --- v1: Initial patch v2: - Added Acks - changed description in MAINTAINERS - changed file directory in MAINTAINERS v3: - actually change the description and file directory in MAINTAINERS --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index fb26be3..8c2d41e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -304,6 +304,11 @@ X: xen/arch/x86/acpi/lib.c F: xen/drivers/cpufreq/ F: xen/include/acpi/cpufreq/ +PUBLIC INTERFACES AND PV DRIVERS DESIGNS +M: Konrad Rzeszutek Wilk +S: Supported +F: xen/include/public/io/ + QEMU-DM M: Ian Jackson S: Supported -- 2.9.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] IOMMU fault with IGD passthrough setup on XEN 4.8.0
>>> On 07.02.17 at 16:44, wrote: > On Mon, Feb 6, 2017 at 8:40 PM, Jan Beulich wrote: > On 05.02.17 at 06:51, wrote: >>> I finally get some spare time to collect the debug info. >> >> As I continue to be puzzled, best I could come up with is an >> extension to the debug patch. Please use the attached one >> in place of the earlier one, ideally on top of >> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00448.html >> to reduce the overall amount of output (and help readability). > > Please see attached... Thanks, this confirms my suspicion that EPT tables get updated, but IOMMU ones don't. I'll have to go over the code yet another time to see if I can spot what's wrong, or whether I need to further extend the debugging patch. Are you using a PVH Dom0 here, btw? If so, is the problem connected to that? The fact that things work there may be attributable to the mappings already being there even before the identity mapping calls. I further note that there's an apparently legitimate IOMMU fault before the first guest gets started. This may indicate an issue with the Dom0 kernel. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
>>> On 07.02.17 at 17:38, wrote: > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -304,6 +304,11 @@ X: xen/arch/x86/acpi/lib.c > F: xen/drivers/cpufreq/ > F: xen/include/acpi/cpufreq/ > > +PUBLIC INTERFACES AND PV DRIVERS DESIGNS > +M: Konrad Rzeszutek Wilk > +S: Supported > +F: xen/include/public/io/ I guess "PUBLIC INTERFACES" is no longer accurate now. How about "PUBLIC I/O INTERFACES AND ..."? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
>>> On 07.02.17 at 17:34, wrote: > On 07/02/17 16:22, Jan Beulich wrote: > On 07.02.17 at 16:06, wrote: >>> If I understood correctly, you are suggesting the following change: >> Mostly. >> >>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h >>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h >>> @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long > field, >>> return ret; >>> } >>> >>> -static always_inline unsigned long vmwrite_safe(unsigned long field, >>> -unsigned long value) >>> +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field, >>> + unsigned long value) >>> { >>> unsigned long ret = 0; >>> bool fail_invalid, fail_valid; >>> @@ -440,11 +440,16 @@ static always_inline unsigned long > vmwrite_safe(unsigned long field, >>> [value] GAS_VMX_OP("rm", "c") (value)); >>> >>> if ( unlikely(fail_invalid) ) >>> +{ >>> ret = VMX_INSN_FAIL_INVALID; >>> +} >> No need to add braces here and ... >> >>> else if ( unlikely(fail_valid) ) >>> +{ >>> __vmread(VM_INSTRUCTION_ERROR, &ret); >>> +BUG_ON(ret >= ~0U); >>> +} >>> >>> -return ret; >>> +return (enum vmx_insn_errno) ret; >> ... no need for the cast here. (See Andrew's reply for the BUG_ON().) >> >>> And I have noticed one inconsistency: vmwrite_safe() is "always_inline" >>> while vmread_safe() is plain "inline". I believe that plain inline is >>> enough here, what do you think? >> I would assume plain inline to be enough, but maybe the VMX >> maintainers know why always_inline was used. > > The always_inline was my doing IIRC, because the use of unlikely > sections caused GCC to create a separate identical functions in each > translation unit, in an attempt to minimise the quantity of out-of-line > code. In which case it's not needed in these new flavors. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
Hello, Setting, e.g. 16 VCPUs for a HVM guest, ends up blocking the guest completely when subscribing to vm_events, apparently because of this code in xen/common/vm_event.c: 315 /* Give this vCPU a black eye if necessary, on the way out. 316 * See the comments above wake_blocked() for more information 317 * on how this mechanism works to avoid waiting. */ 318 avail_req = vm_event_ring_available(ved); 319 if( current->domain == d && avail_req < d->max_vcpus ) 320 vm_event_mark_and_pause(current, ved); It would appear that even if the guest only has 2 online VCPUs, the "avail_req < d->max_vcpus" condition will pause current, and we eventually end up with all the VCPUs paused. An ugly hack ("avail_req < 2") has allowed booting a guest with many VCPUs (max_vcpus, the guest only brings 2 VCPUs online), however that's just to prove that that was the culprit - a real solution to this needs more in-depth understading of the issue and potential solution. That's basically very old code (pre-2012 at least) that got moved around into the current shape of Xen today - please CC anyone relevant to the discussion that you're aware of. Thoughts? Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] IOMMU fault with IGD passthrough setup on XEN 4.8.0
>>> On 07.02.17 at 16:44, wrote: > On Mon, Feb 6, 2017 at 8:40 PM, Jan Beulich wrote: > On 05.02.17 at 06:51, wrote: >>> I finally get some spare time to collect the debug info. >> >> As I continue to be puzzled, best I could come up with is an >> extension to the debug patch. Please use the attached one >> in place of the earlier one, ideally on top of >> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00448.html >> to reduce the overall amount of output (and help readability). > > Please see attached... Okay, that's because p2m_get_iommu_flags() returns zero for p2m_mmio_direct, which iirc we already have a patch floating around for (needed for PVHv2). Roger - what's the status of that? Would it make sense to split this out of your series, to get in rather sooner? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 105609: tolerable trouble: broken/fail/pass - PUSHED
flight 105609 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/105609/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-pvops 5 kernel-build fail 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 63e1d01b8fd948b3e0fa3beea494e407668aa43b baseline version: xen dbc84d2983969bb47d294131ed9e6bbbdc2aec49 Last test of basis 105605 2017-02-07 12:01:43 Z0 days Testing same since 105609 2017-02-07 15:03:57 Z0 days1 attempts People who touched revisions under test: Andrew Cooper David Woodhouse Jan Beulich Kevin Tian Stefano Stabellini Venu Busireddy jobs: build-amd64 pass build-arm64 fail build-armhf pass build-amd64-libvirt pass build-arm64-pvopsfail test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken 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=63e1d01b8fd948b3e0fa3beea494e407668aa43b + . ./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 63e1d01b8fd948b3e0fa3beea494e407668aa43b + branch=xen-unstable-smoke + revision=63e1d01b8fd948b3e0fa3beea494e407668aa43b + . ./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.8-testing + '[' x63e1d01b8fd948b3e0fa3beea494e407668aa43b = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git +
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
On Tue, Feb 07, 2017 at 09:47:09AM -0700, Jan Beulich wrote: > >>> On 07.02.17 at 17:38, wrote: > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -304,6 +304,11 @@ X: xen/arch/x86/acpi/lib.c > > F: xen/drivers/cpufreq/ > > F: xen/include/acpi/cpufreq/ > > > > +PUBLIC INTERFACES AND PV DRIVERS DESIGNS > > +M: Konrad Rzeszutek Wilk > > +S: Supported > > +F: xen/include/public/io/ > > I guess "PUBLIC INTERFACES" is no longer accurate now. How > about "PUBLIC I/O INTERFACES AND ..."? Changed it locally to be that. > > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
>>> On 07.02.17 at 18:02, wrote: > On Tue, Feb 07, 2017 at 09:47:09AM -0700, Jan Beulich wrote: >> >>> On 07.02.17 at 17:38, wrote: >> > --- a/MAINTAINERS >> > +++ b/MAINTAINERS >> > @@ -304,6 +304,11 @@ X:xen/arch/x86/acpi/lib.c >> > F:xen/drivers/cpufreq/ >> > F:xen/include/acpi/cpufreq/ >> > >> > +PUBLIC INTERFACES AND PV DRIVERS DESIGNS >> > +M: Konrad Rzeszutek Wilk >> > +S: Supported >> > +F: xen/include/public/io/ >> >> I guess "PUBLIC INTERFACES" is no longer accurate now. How >> about "PUBLIC I/O INTERFACES AND ..."? > > Changed it locally to be that. With that Acked-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 105610: regressions - FAIL
flight 105610 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/105610/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm5 xen-buildfail REGR. vs. 105279 build-amd64-xsm 5 xen-buildfail REGR. vs. 105279 build-amd64 5 xen-buildfail REGR. vs. 105279 build-i3865 xen-buildfail REGR. vs. 105279 build-armhf-xsm 5 xen-buildfail REGR. vs. 105279 build-armhf 5 xen-buildfail REGR. vs. 105279 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-winxpsp3 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 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-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-xsm 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(
[Xen-devel] PML causing race condition during guest bootstorm and host crash on Broadwell cpu.
Hi, Facing a issue where bootstorm of guests leads to host crash. I debugged and found that that enabling PML introduces a race condition during guest teardown stage while disabling PML on a vcpu and context switch happening for another vcpu. Crash happens only on Broadwell processors as PML got introduced in this generation. Here is my analysis: Race condition: vmcs.c vmx_vcpu_disable_pml (vcpu){ vmx_vmcs_enter() ; vm_write( disable_PML); vmx_vmcx_exit();) In between I have a callpath from another pcpu executing context switch-> vmx_fpu_leave() and crash on vmwrite.. if ( !(v->arch.hvm_vmx.host_cr0 & X86_CR0_TS) ) { v->arch.hvm_vmx.host_cr0 |= X86_CR0_TS; __vmwrite(HOST_CR0, v->arch.hvm_vmx.host_cr0); //crash } Debug logs XEN) [221256.749928] VMWRITE VMCS Invalid ! (XEN) [221256.754870] **[00] { now c93b4341df1d, hw 0035fffea000, op 0035fffea000 } vmclear (XEN) [221256.765052] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221256.773969] **[01] { now c93b4341e099, hw , op 0035fffea000 } vmptrld (XEN) [221256.784150] ** frames [ 82d0801f0765 vmx_vmcs_try_enter+0x95/0xb0 ] (XEN) [221256.792197] **[02] { now c93b4341e1f1, hw 0035fffea000, op 0035fffea000 } vmclear (XEN) [221256.802378] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221256.811298] **[03] { now c93b5784dd0a, hw , op 0039d7074000 } vmptrld (XEN) [221256.821478] ** frames [ 82d0801f4c31 vmx_do_resume+0x51/0x150 ] (XEN) [221256.829139] **[04] { now c93b59d67b5b, hw 0039d7074000, op 002b9a575000 } vmptrld (XEN) [221256.839320] ** frames [ 82d0801f4c31 vmx_do_resume+0x51/0x150 ] (XEN) [221256.882850] **[07] { now c93b59e71e48, hw 002b9a575000, op 0039d7074000 } vmptrld (XEN) [221256.893034] ** frames [ 82d0801f4d13 vmx_do_resume+0x133/0x150 ] (XEN) [221256.900790] **[08] { now c93b59e78675, hw 0039d7074000, op 0040077ae000 } vmptrld (XEN) [221256.910968] ** frames [ 82d0801f0765 vmx_vmcs_try_enter+0x95/0xb0 ] (XEN) [221256.919015] **[09] { now c93b59e78ac8, hw 0040077ae000, op 0040077ae000 } vmclear (XEN) [221256.929196] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221256.938117] **[10] { now c93b59e78d72, hw , op 0040077ae000 } vmptrld (XEN) [221256.948297] ** frames [ 82d0801f0765 vmx_vmcs_try_enter+0x95/0xb0 ] (XEN) [221256.956345] **[11] { now c93b59e78ff0, hw 0040077ae000, op 0040077ae000 } vmclear (XEN) [221256.966525] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221256.975445] **[12] { now c93b59e7deda, hw , op 0040077b3000 } vmptrld (XEN) [221256.985626] ** frames [ 82d0801f0765 vmx_vmcs_try_enter+0x95/0xb0 ] (XEN) [221256.993672] **[13] { now c93b59e9fe00, hw 0040077b3000, op 0040077b3000 } vmclear (XEN) [221257.003852] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221257.012772] **[14] { now c93b59ea007e, hw , op 0040077b3000 } vmptrld (XEN) [221257.022952] ** frames [ 82d0801f0765 vmx_vmcs_try_enter+0x95/0xb0 ] (XEN) [221257.031000] **[15] { now c93b59ea02ba, hw 0040077b3000, op 0040077b3000 } vmclear (XEN) [221257.041180] ** frames [ 82d080134652 smp_call_function_interrupt+0x92/0xa0 ] (XEN) [221257.050101] (XEN) [221257.053008] vmcs_ptr:0x, vcpu->vmcs:0x2b9a575000 vmcs is loaded and between the next call to vm_write, there is a clear of vmcs caused by vmx_vcpu_disable_pml. Above log highlights that IPI is clearing the vmcs in between vmptrld and vmwrite but I also verified that interrupts are disabled during context switch and execution of vm_write in vmx_fpu_leave.. This has got me confused. Also, I am not sure if I understand the handling of foreign_vmcs correctly, which can also be the cause of the race. Please if you can share some suggestions here. Thanks Anshul Makkar ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6] x86/iommu: add IOMMU entries for p2m_mmio_direct pages
There's nothing wrong with allowing the domain to perform DMA transfers to MMIO areas that it already can access from the CPU, and this allows us to remove the hack in set_identity_p2m_entry for PVH Dom0. Signed-off-by: Roger Pau Monné Reviewed-by: Andrew Cooper Reviewed-by: Kevin Tian --- Cc: Jun Nakajima Cc: Kevin Tian Cc: George Dunlap Cc: Jan Beulich Cc: Andrew Cooper --- Changes since v5: - Fix inverted check for mmio_ro_ranges. Changes since v4: - Check for mmio_ro_ranges, this requires passing the mfn to the function, and fixing the callers. --- xen/arch/x86/mm/p2m-ept.c | 5 +++-- xen/arch/x86/mm/p2m-pt.c | 17 ++--- xen/arch/x86/mm/p2m.c | 9 - xen/include/asm-x86/p2m.h | 6 +- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 04878f5..f47f323 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -672,7 +672,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, uint8_t ipat = 0; bool_t need_modify_vtd_table = 1; bool_t vtd_pte_present = 0; -unsigned int iommu_flags = p2m_get_iommu_flags(p2mt); +unsigned int iommu_flags = p2m_get_iommu_flags(p2mt, mfn); bool_t needs_sync = 1; ept_entry_t old_entry = { .epte = 0 }; ept_entry_t new_entry = { .epte = 0 }; @@ -798,7 +798,8 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, /* Safe to read-then-write because we hold the p2m lock */ if ( ept_entry->mfn == new_entry.mfn && - p2m_get_iommu_flags(ept_entry->sa_p2mt) == iommu_flags ) + p2m_get_iommu_flags(ept_entry->sa_p2mt, _mfn(ept_entry->mfn)) == + iommu_flags ) need_modify_vtd_table = 0; ept_p2m_type_to_flags(p2m, &new_entry, p2mt, p2ma); diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index 3b025d5..a23d0bd 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -499,7 +499,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, l2_pgentry_t l2e_content; l3_pgentry_t l3e_content; int rc; -unsigned int iommu_pte_flags = p2m_get_iommu_flags(p2mt); +unsigned int iommu_pte_flags = p2m_get_iommu_flags(p2mt, mfn); /* * old_mfn and iommu_old_flags control possible flush/update needs on the * IOMMU: We need to flush when MFN or flags (i.e. permissions) change. @@ -565,9 +565,10 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, { if ( flags & _PAGE_PSE ) { -iommu_old_flags = -p2m_get_iommu_flags(p2m_flags_to_type(flags)); old_mfn = l1e_get_pfn(*p2m_entry); +iommu_old_flags = +p2m_get_iommu_flags(p2m_flags_to_type(flags), +_mfn(old_mfn)); } else { @@ -609,9 +610,10 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn, 0, L1_PAGETABLE_ENTRIES); ASSERT(p2m_entry); -iommu_old_flags = -p2m_get_iommu_flags(p2m_flags_to_type(l1e_get_flags(*p2m_entry))); old_mfn = l1e_get_pfn(*p2m_entry); +iommu_old_flags = +p2m_get_iommu_flags(p2m_flags_to_type(l1e_get_flags(*p2m_entry)), +_mfn(old_mfn)); if ( mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt) ) entry_content = p2m_l1e_from_pfn(mfn_x(mfn), @@ -637,9 +639,10 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, { if ( flags & _PAGE_PSE ) { -iommu_old_flags = -p2m_get_iommu_flags(p2m_flags_to_type(flags)); old_mfn = l1e_get_pfn(*p2m_entry); +iommu_old_flags = +p2m_get_iommu_flags(p2m_flags_to_type(flags), +_mfn(old_mfn)); } else { diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 6548e9f..bd8ce35 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1053,16 +1053,7 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn, ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, p2m_mmio_direct, p2ma); else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2ma ) -{ ret = 0; -/* - * PVH fixme: during Dom0 PVH construction, p2m entries are being set - * but iomem regions are not mapped with IOMMU. This makes sure that - * RMRRs are correctly mapped with IOMMU. - */ -if ( is_hardware_domain(d) && !iommu_use_hap_pt(d) ) -ret = iommu_map_page(d, gfn, gfn, IOMM
[Xen-devel] [PATCH v6] x86/hvm: add vcpu parameter to guest memory copy function
Current __hvm_copy assumes that the destination memory belongs to the current vcpu, but this is not always the case since for PVHv2 Dom0 build hvm copy functions are used with current being the idle vcpu. Add a new vcpu parameter to hvm copy in order to solve that. Note that only hvm_copy_to_guest_phys is changed to take a vcpu parameter, because that's the only one at the moment that's required in order to build a PVHv2 Dom0. While there, also assert that the passed vcpu belongs to a HVM guest. Signed-off-by: Roger Pau Monné --- Changes since v5: - Name vcpu param 'v' in __hvm_copy and move it's position in the parameter list. - Do not call hvm_mmio_internal if v != current. - Remove hvm_copy_to_guest_phys_vcpu and instead modify hvm_copy_to_guest_phys to take a vcpu parameter. - Fix parameter passed to %pv printk modifier. Changes since v4: - New in the series. --- xen/arch/x86/hvm/emulate.c| 4 ++-- xen/arch/x86/hvm/hvm.c| 35 ++- xen/arch/x86/hvm/intercept.c | 2 +- xen/arch/x86/hvm/vmx/realmode.c | 2 +- xen/include/asm-x86/hvm/support.h | 2 +- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index 0d21fe1..fed8801 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -1294,7 +1294,7 @@ static int hvmemul_rep_movs( rc = hvm_copy_from_guest_phys(buf, sgpa, bytes); if ( rc == HVMCOPY_okay ) -rc = hvm_copy_to_guest_phys(dgpa, buf, bytes); +rc = hvm_copy_to_guest_phys(dgpa, buf, bytes, current); xfree(buf); @@ -1405,7 +1405,7 @@ static int hvmemul_rep_stos( if ( df ) gpa -= bytes - bytes_per_rep; -rc = hvm_copy_to_guest_phys(gpa, buf, bytes); +rc = hvm_copy_to_guest_phys(gpa, buf, bytes, current); if ( buf != p_data ) xfree(buf); diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 21a1649..42ff9ff 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3082,16 +3082,17 @@ void hvm_task_switch( #define HVMCOPY_phys (0u<<2) #define HVMCOPY_linear (1u<<2) static enum hvm_copy_result __hvm_copy( -void *buf, paddr_t addr, int size, unsigned int flags, uint32_t pfec, -pagefault_info_t *pfinfo) +void *buf, paddr_t addr, int size, struct vcpu *v, unsigned int flags, +uint32_t pfec, pagefault_info_t *pfinfo) { -struct vcpu *curr = current; unsigned long gfn; struct page_info *page; p2m_type_t p2mt; char *p; int count, todo = size; +ASSERT(has_hvm_container_vcpu(v)); + /* * XXX Disable for 4.1.0: PV-on-HVM drivers will do grant-table ops * such as query_size. Grant-table code currently does copy_to/from_guest @@ -3116,7 +3117,7 @@ static enum hvm_copy_result __hvm_copy( if ( flags & HVMCOPY_linear ) { -gfn = paging_gva_to_gfn(curr, addr, &pfec); +gfn = paging_gva_to_gfn(v, addr, &pfec); if ( gfn == gfn_x(INVALID_GFN) ) { if ( pfec & PFEC_page_paged ) @@ -3143,12 +3144,12 @@ static enum hvm_copy_result __hvm_copy( * - 32-bit WinXP (& older Windows) on AMD CPUs for LAPIC accesses, * - newer Windows (like Server 2012) for HPET accesses. */ -if ( !nestedhvm_vcpu_in_guestmode(curr) - && is_hvm_vcpu(curr) +if ( !nestedhvm_vcpu_in_guestmode(v) + && is_hvm_vcpu(v) && v == current && hvm_mmio_internal(gpa) ) return HVMCOPY_bad_gfn_to_mfn; -page = get_page_from_gfn(curr->domain, gfn, &p2mt, P2M_UNSHARE); +page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE); if ( !page ) return HVMCOPY_bad_gfn_to_mfn; @@ -3156,7 +3157,7 @@ static enum hvm_copy_result __hvm_copy( if ( p2m_is_paging(p2mt) ) { put_page(page); -p2m_mem_paging_populate(curr->domain, gfn); +p2m_mem_paging_populate(v->domain, gfn); return HVMCOPY_gfn_paged_out; } if ( p2m_is_shared(p2mt) ) @@ -3178,9 +3179,9 @@ static enum hvm_copy_result __hvm_copy( { static unsigned long lastpage; if ( xchg(&lastpage, gfn) != gfn ) -gdprintk(XENLOG_DEBUG, "guest attempted write to read-only" +printk(XENLOG_DEBUG, "%pv guest attempted write to read-only" " memory page. gfn=%#lx, mfn=%#lx\n", - gfn, page_to_mfn(page)); + v, gfn, page_to_mfn(page)); } else { @@ -3188,7 +3189,7 @@ static enum hvm_copy_result __hvm_copy( memcpy(p, buf, count); else memset(p, 0, count); -paging_mark_dirty(curr->domain, _mfn(
Re: [Xen-devel] [PATCH v2 1/2] xen/kbdif: update protocol documentation
On Thu, Jan 26, 2017 at 09:46:46AM +0200, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > Reviewed-by: Stefano Stabellini Reviewed-by: Konrad Rzeszutek Wilk Thank you for documenting this! ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [ovmf baseline-only test] 68532: all pass
This run is configured for baseline tests only. flight 68532 ovmf real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/68532/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf a316d7ac91d302085b5c35d76db703f2208ec026 baseline version: ovmf 7c609a144b6636577dd60fbaa5fc64efeecd7baf Last test of basis68531 2017-02-07 11:17:52 Z0 days Testing same since68532 2017-02-07 15:48:52 Z0 days1 attempts People who touched revisions under test: Laszlo Ersek jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. commit a316d7ac91d302085b5c35d76db703f2208ec026 Author: Laszlo Ersek Date: Wed Nov 30 20:06:34 2016 +0100 OvmfPkg/SmmControl2Dxe: select broadcast SMI if available When writing to IO port 0xB2 (ICH9_APM_CNT), QEMU by default injects an SMI only on the VCPU that is writing the port. This has exposed corner cases and strange behavior with edk2 code, which generally expects a software SMI to affect all CPUs at once. We've experienced instability despite the fact that OVMF sets PcdCpuSmmApSyncTimeout and PcdCpuSmmSyncMode differently from the UefiCpuPkg defaults, such that they match QEMU's unicast SMIs better. (Refer to edk2 commits 9b1e378811ff and bb0f18b0bce6.) Using the new fw_cfg-based SMI feature negotiation in QEMU (see commits 50de920b372b "hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg" and 5ce45c7a2b15 "hw/isa/lpc_ich9: add broadcast SMI feature"), we can ask QEMU to broadcast SMIs. Extensive testing from earlier proves that broadcast SMIs are only reliable if we use the UefiCpuPkg defaults for the above PCDs. With those settings however, the broadcast is very reliable -- the most reliable configuration encountered thus far. Therefore negotiate broadcast SMIs with QEMU, and if the negotiation is successful, dynamically revert the PCDs to the UefiCpuPkg defaults. Setting the PCDs in this module is safe: - only PiSmmCpuDxeSmm consumes them, - PiSmmCpuDxeSmm is a DXE_SMM_DRIVER, launched by the SMM_CORE (MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf), - the SMM_CORE is launched by the SMM IPL runtime DXE driver (MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf), - the SMM IPL has a DEPEX on EFI_SMM_CONTROL2_PROTOCOL, - OvmfPkg/SmmControl2Dxe produces that protocol. The end result is that PiSmmCpuDxeSmm cannot be dispatched before SmmControl2Dxe installs EFI_SMM_CONTROL2_PROTOCOL and returns from its entry point. Hence we can set the PCD's consumed by PiSmmCpuDxeSmm in SmmControl2Dxe. Cc: Jordan Justen Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen commit 22d7be69c45ecc7ce2b21252710c37be7122c1bd Author: Laszlo Ersek Date: Thu Nov 17 22:21:35 2016 +0100 OvmfPkg: dynamic defaults for PcdCpuSmmApSyncTimeout, PcdCpuSmmSyncMode Move the platform-specific default values for these PCDs from the [PcdsFixedAtBuild] / [PcdsFixedAtBuild.X64] sections to the [PcdsDynamicDefault] section. Cc: Jordan Justen Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 0/9] PVH v2 support (domU)
On 02/06/2017 12:00 PM, Boris Ostrovsky wrote: > PVHv2 support for unprivileged guests. > > Changes in v3: > * See patches 4 and 5 > Applied to for-linus-4.11 -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/2] XEN SWIOTLB dma operations extension
From: Andrii Anisov Some drivers need additional dma ops to be provided by xen swiotlb. Because common operations implementation came from x86 world and does not suite ARM needs we have to provide needed XEN SWIOTLB for ARM callbacks. dma_mmap patch is a port of an antique and lost patch discussed here: http://marc.info/?l=xen-devel&m=139695901430667&w=4 Changes in V4: - No functional changes - '#ifdef DEBUG' is replaced with '#if 0' for a debug code because Konrad insisted - Added more comments for the functions added and for the debug code - fixed code alignment. Changes in V3: - dma ops are moved from ARM specific to generic code - runtime operation verified for arm64 with LK 4.9 - compilation verified for arm, arm64, x86 with the current LK master HEAD 566cf877a1fcb6d6dc0126b076aad062054c2637 Changes in V2: - patches are rebased and checked for compilation for x86, arm, arm64 with the current LK master HEAD 83346fbc07d267de777e2597552f785174ad0373 Andrii Anisov (1): swiotlb-xen: implement xen_swiotlb_get_sgtable callback Stefano Stabellini (1): swiotlb-xen: implement xen_swiotlb_dma_mmap callback arch/arm/xen/mm.c | 2 ++ drivers/xen/swiotlb-xen.c | 47 +++ include/xen/swiotlb-xen.h | 11 +++ 3 files changed, 60 insertions(+) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 1/2] swiotlb-xen: implement xen_swiotlb_dma_mmap callback
From: Stefano Stabellini This function creates userspace mapping for the DMA-coherent memory. Signed-off-by: Stefano Stabellini Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Andrii Anisov --- arch/arm/xen/mm.c | 1 + drivers/xen/swiotlb-xen.c | 19 +++ include/xen/swiotlb-xen.h | 5 + 3 files changed, 25 insertions(+) diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index bd62d94..cd1684e 100644 --- a/arch/arm/xen/mm.c +++ b/arch/arm/xen/mm.c @@ -198,6 +198,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = { .unmap_page = xen_swiotlb_unmap_page, .dma_supported = xen_swiotlb_dma_supported, .set_dma_mask = xen_swiotlb_set_dma_mask, + .mmap = xen_swiotlb_dma_mmap, }; int __init xen_mm_init(void) diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index f8afc6d..728d4e0 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -681,3 +681,22 @@ xen_swiotlb_set_dma_mask(struct device *dev, u64 dma_mask) return 0; } EXPORT_SYMBOL_GPL(xen_swiotlb_set_dma_mask); + +/* + * Create userspace mapping for the DMA-coherent memory. + * This function should be called with the pages from the current domain only, + * passing pages mapped from other domains would lead to memory corruption. + */ +int +xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma, +void *cpu_addr, dma_addr_t dma_addr, size_t size, +unsigned long attrs) +{ +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) + if (__generic_dma_ops(dev)->mmap) + return __generic_dma_ops(dev)->mmap(dev, vma, cpu_addr, + dma_addr, size, attrs); +#endif + return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size); +} +EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap); diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h index a0083be..a315c87 100644 --- a/include/xen/swiotlb-xen.h +++ b/include/xen/swiotlb-xen.h @@ -55,4 +55,9 @@ xen_swiotlb_dma_supported(struct device *hwdev, u64 mask); extern int xen_swiotlb_set_dma_mask(struct device *dev, u64 dma_mask); + +extern int +xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma, +void *cpu_addr, dma_addr_t dma_addr, size_t size, +unsigned long attrs); #endif /* __LINUX_SWIOTLB_XEN_H */ -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback
From: Andrii Anisov Signed-off-by: Andrii Anisov Signed-off-by: Stefano Stabellini --- arch/arm/xen/mm.c | 1 + drivers/xen/swiotlb-xen.c | 28 include/xen/swiotlb-xen.h | 6 ++ 3 files changed, 35 insertions(+) diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index cd1684e..76ea48a 100644 --- a/arch/arm/xen/mm.c +++ b/arch/arm/xen/mm.c @@ -199,6 +199,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = { .dma_supported = xen_swiotlb_dma_supported, .set_dma_mask = xen_swiotlb_set_dma_mask, .mmap = xen_swiotlb_dma_mmap, + .get_sgtable = xen_swiotlb_get_sgtable, }; int __init xen_mm_init(void) diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 728d4e0..e8cef1a 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -700,3 +700,31 @@ xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma, return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size); } EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap); + +/* + * This function should be called with the pages from the current domain only, + * passing pages mapped from other domains would lead to memory corruption. + */ +int +xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt, + void *cpu_addr, dma_addr_t handle, size_t size, + unsigned long attrs) +{ +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) + if (__generic_dma_ops(dev)->get_sgtable) { +#if 0 + /* +* This check verifies that the page belongs to the current domain and +* is not one mapped from another domain. +* This check is for debug only, and should not go to production build +*/ + unsigned long bfn = PHYS_PFN(dma_to_phys(dev, handle)); + BUG_ON (!page_is_ram(bfn)); +#endif + return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, + handle, size, attrs); + } +#endif + return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size); +} +EXPORT_SYMBOL_GPL(xen_swiotlb_get_sgtable); diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h index a315c87..1f6d78f 100644 --- a/include/xen/swiotlb-xen.h +++ b/include/xen/swiotlb-xen.h @@ -2,6 +2,7 @@ #define __LINUX_SWIOTLB_XEN_H #include +#include #include extern int xen_swiotlb_init(int verbose, bool early); @@ -60,4 +61,9 @@ extern int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma, void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs); + +extern int +xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt, + void *cpu_addr, dma_addr_t handle, size_t size, + unsigned long attrs); #endif /* __LINUX_SWIOTLB_XEN_H */ -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 08/28] ARM: GICv3 ITS: introduce host LPI array
Hi Andre, On 30/01/2017 18:31, Andre Przywara wrote: The number of LPIs on a host can be potentially huge (millions), although in practise will be mostly reasonable. So prematurely allocating an array of struct irq_desc's for each LPI is not an option. However Xen itself does not care about LPIs, as every LPI will be injected into a guest (Dom0 for now). Create a dense data structure (8 Bytes) for each LPI which holds just enough information to determine the virtual IRQ number and the VCPU into which the LPI needs to be injected. Also to not artificially limit the number of LPIs, we create a 2-level table for holding those structures. This patch introduces functions to initialize these tables and to create, lookup and destroy entries for a given LPI. We allocate and access LPI information in a way that does not require a lock. Signed-off-by: Andre Przywara --- xen/arch/arm/gic-v3-its.c| 80 - xen/arch/arm/gic-v3-lpi.c| 187 ++- xen/include/asm-arm/atomic.h | 6 +- xen/include/asm-arm/gic.h| 5 ++ xen/include/asm-arm/gic_v3_its.h | 9 ++ 5 files changed, 282 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 4a3a394..f073ab5 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -83,6 +83,20 @@ static int its_send_cmd_sync(struct host_its *its, int cpu) return its_send_command(its, cmd); } +static int its_send_cmd_mapti(struct host_its *its, + uint32_t deviceid, uint32_t eventid, + uint32_t pintid, uint16_t icid) +{ +uint64_t cmd[4]; + +cmd[0] = GITS_CMD_MAPTI | ((uint64_t)deviceid << 32); +cmd[1] = eventid | ((uint64_t)pintid << 32); +cmd[2] = icid; +cmd[3] = 0x00; + +return its_send_command(its, cmd); +} + static int its_send_cmd_mapc(struct host_its *its, int collection_id, int cpu) { uint64_t cmd[4]; @@ -111,6 +125,19 @@ static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid, return its_send_command(its, cmd); } +static int its_send_cmd_inv(struct host_its *its, +uint32_t deviceid, uint32_t eventid) +{ +uint64_t cmd[4]; + +cmd[0] = GITS_CMD_INV | ((uint64_t)deviceid << 32); +cmd[1] = eventid; +cmd[2] = 0x00; +cmd[3] = 0x00; + +return its_send_command(its, cmd); +} + /* Set up the (1:1) collection mapping for the given host CPU. */ int gicv3_its_setup_collection(int cpu) { @@ -359,13 +386,47 @@ int gicv3_its_init(struct host_its *hw_its) static void remove_mapped_guest_device(struct its_devices *dev) { +int i; + if ( dev->hw_its ) its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false); +for ( i = 0; i < dev->eventids / 32; i++ ) Please use LPI_BLOCK rather than 32. +gicv3_free_host_lpi_block(dev->hw_its, dev->host_lpis[i]); Without looking at the implementation of gicv3_free_host_lpi_block, I think the usage of the function is very confusing. When I read host_lpis, I expect to see one LPI per event. But instead it be the first LPI of a block. The lack of documentation of the field in its_devices does not help to understand what's going on. So please add some documentation and probably renaming some fields. Also, the function can return an error but you don't check it. Please make sure to verify the return value. Lastly should not we discard the LPIs before removing the device? Or does MAPD take care for you? + xfree(dev->itt_addr); +xfree(dev->host_lpis); I forgot to mention in the previous patch. You free dev->itt_addr and dev->host_lpis without even waiting that the ITS handle the command. This is real bad idea as Xen could re-allocate the memory to someone else as soon as xfree has finished. xfree(dev); } +/* + * On the host ITS @its, map @nr_events consecutive LPIs. + * The mapping connects a device @devid and event @eventid pair to LPI @lpi, + * increasing both @eventid and @lpi to cover the number of requested LPIs. + */ +int gicv3_its_map_host_events(struct host_its *its, + int devid, int eventid, int lpi, + int nr_events) All those fields should at least be unsigned int. For devid, I would use uint32_t. In general anything that does not require to be signed should be unsigned int. Similarly if we deal with an hardware value the type should be uintXX_t. This makes easier to match the hardware and avoid potential issue later. +{ +int i, ret; i should be unsigned int. + +for ( i = 0; i < nr_events; i++ ) +{ +ret = its_send_cmd_mapti(its, devid, eventid + i, lpi + i, 0); A comment explain what 0 stands for would be really helpful. Something along those lines: "All interrupt are mapped to CPU0 (e.g collection 0) by default". +if ( ret ) +return ret; +ret = its_s
[Xen-devel] [distros-debian-snapshot test] 68530: tolerable trouble: blocked/broken/fail/pass
flight 68530 distros-debian-snapshot real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/68530/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-i386-i386-daily-netboot-pvgrub 10 guest-start fail blocked in 68502 test-amd64-i386-amd64-daily-netboot-pygrub 10 guest-start fail blocked in 68502 test-amd64-amd64-amd64-daily-netboot-pvgrub 10 guest-start fail blocked in 68502 test-amd64-amd64-amd64-weekly-netinst-pygrub 9 debian-di-install fail like 68502 test-armhf-armhf-armhf-daily-netboot-pygrub 9 debian-di-install fail like 68502 test-amd64-i386-i386-weekly-netinst-pygrub 9 debian-di-install fail like 68502 test-amd64-i386-amd64-weekly-netinst-pygrub 9 debian-di-install fail like 68502 test-amd64-amd64-i386-weekly-netinst-pygrub 9 debian-di-install fail like 68502 Tests which did not succeed, but are not blocking: test-arm64-arm64-armhf-daily-netboot-pygrub 1 build-check(1) blocked n/a build-arm64-pvops 2 hosts-allocate broken never pass build-arm64 2 hosts-allocate broken never pass build-arm64-pvops 3 capture-logs broken never pass build-arm64 3 capture-logs broken never pass baseline version: flight 68502 jobs: build-amd64 pass build-arm64 broken build-armhf pass build-i386 pass build-amd64-pvopspass build-arm64-pvopsbroken build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-daily-netboot-pvgrub fail test-amd64-i386-i386-daily-netboot-pvgrubfail test-amd64-i386-amd64-daily-netboot-pygrub fail test-arm64-arm64-armhf-daily-netboot-pygrub blocked test-armhf-armhf-armhf-daily-netboot-pygrub fail test-amd64-amd64-i386-daily-netboot-pygrub pass test-amd64-amd64-amd64-current-netinst-pygrubpass test-amd64-i386-amd64-current-netinst-pygrub pass test-amd64-amd64-i386-current-netinst-pygrub pass test-amd64-i386-i386-current-netinst-pygrub pass test-amd64-amd64-amd64-weekly-netinst-pygrub fail test-amd64-i386-amd64-weekly-netinst-pygrub fail test-amd64-amd64-i386-weekly-netinst-pygrub fail test-amd64-i386-i386-weekly-netinst-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] xen: optimize xenbus driver for multiple concurrent xenstore accesses
On 01/24/2017 11:23 AM, Juergen Gross wrote: > On 24/01/17 14:47, Boris Ostrovsky wrote: >> On 01/23/2017 01:59 PM, Boris Ostrovsky wrote: >>> On 01/23/2017 05:09 AM, Juergen Gross wrote: Handling of multiple concurrent Xenstore accesses through xenbus driver either from the kernel or user land is rather lame today: xenbus is capable to have one access active only at one point of time. >> >> This patch appears to break save/restore: > Hmm, tried multiple times, but I can't reproduce this issue. > > Anything special in the setup? I tried a 64 bit pv guest and did > "xl save". > > Do I have to run some load in parallel? Any luck reproducing this? I am still failing the test on dumpdata but I couldn't reproduce it on another system. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
On Tue, Feb 7, 2017 at 9:53 AM, Razvan Cojocaru wrote: > Hello, > > Setting, e.g. 16 VCPUs for a HVM guest, ends up blocking the guest > completely when subscribing to vm_events, apparently because of this > code in xen/common/vm_event.c: > > 315 /* Give this vCPU a black eye if necessary, on the way out. > 316 * See the comments above wake_blocked() for more information > 317 * on how this mechanism works to avoid waiting. */ > 318 avail_req = vm_event_ring_available(ved); > 319 if( current->domain == d && avail_req < d->max_vcpus ) > 320 vm_event_mark_and_pause(current, ved); > > It would appear that even if the guest only has 2 online VCPUs, the > "avail_req < d->max_vcpus" condition will pause current, and we > eventually end up with all the VCPUs paused. > > An ugly hack ("avail_req < 2") has allowed booting a guest with many > VCPUs (max_vcpus, the guest only brings 2 VCPUs online), however that's > just to prove that that was the culprit - a real solution to this needs > more in-depth understading of the issue and potential solution. That's > basically very old code (pre-2012 at least) that got moved around into > the current shape of Xen today - please CC anyone relevant to the > discussion that you're aware of. > > Thoughts? > I think is a side-effect of the growth of the vm_event structure and the fact that we have a single page ring. The check effectively sets a threshold of having enough space for each vCPU to place at least one more event on the ring, and if that's not the case it gets paused. OTOH I think this would only have an effect on asynchronous events, for all other events the vCPU is already paused. Is that the case you have? Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 0/2] XEN SWIOTLB dma operations extension
Both patches are fine by me. On Tue, 7 Feb 2017, Andrii Anisov wrote: > From: Andrii Anisov > > Some drivers need additional dma ops to be provided by xen swiotlb. Because > common operations implementation came from x86 world and does not suite ARM > needs we have to provide needed XEN SWIOTLB for ARM callbacks. > > dma_mmap patch is a port of an antique and lost patch discussed here: > http://marc.info/?l=xen-devel&m=139695901430667&w=4 > > Changes in V4: > - No functional changes > - '#ifdef DEBUG' is replaced with '#if 0' for a debug code because Konrad >insisted > - Added more comments for the functions added and for the debug code > - fixed code alignment. > > Changes in V3: > - dma ops are moved from ARM specific to generic code > - runtime operation verified for arm64 with LK 4.9 > - compilation verified for arm, arm64, x86 with the current LK master HEAD >566cf877a1fcb6d6dc0126b076aad062054c2637 > > Changes in V2: > - patches are rebased and checked for compilation for x86, arm, arm64 with >the current LK master HEAD 83346fbc07d267de777e2597552f785174ad0373 > > Andrii Anisov (1): > swiotlb-xen: implement xen_swiotlb_get_sgtable callback > > Stefano Stabellini (1): > swiotlb-xen: implement xen_swiotlb_dma_mmap callback > > arch/arm/xen/mm.c | 2 ++ > drivers/xen/swiotlb-xen.c | 47 > +++ > include/xen/swiotlb-xen.h | 11 +++ > 3 files changed, 60 insertions(+) > > -- > 2.7.4 > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] MAINTAINERS: Add myself as the public API "Czar"
Jan Beulich writes ("Re: [PATCH v2] MAINTAINERS: Add myself as the public API "Czar""): > On 07.02.17 at 18:02, wrote: > > Changed it locally to be that. > > With that > Acked-by: Jan Beulich Likewise. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
On 02/07/2017 08:15 PM, Tamas K Lengyel wrote: > > > On Tue, Feb 7, 2017 at 9:53 AM, Razvan Cojocaru > mailto:rcojoc...@bitdefender.com>> wrote: > > Hello, > > Setting, e.g. 16 VCPUs for a HVM guest, ends up blocking the guest > completely when subscribing to vm_events, apparently because of this > code in xen/common/vm_event.c: > > 315 /* Give this vCPU a black eye if necessary, on the way out. > 316 * See the comments above wake_blocked() for more information > 317 * on how this mechanism works to avoid waiting. */ > 318 avail_req = vm_event_ring_available(ved); > 319 if( current->domain == d && avail_req < d->max_vcpus ) > 320 vm_event_mark_and_pause(current, ved); > > It would appear that even if the guest only has 2 online VCPUs, the > "avail_req < d->max_vcpus" condition will pause current, and we > eventually end up with all the VCPUs paused. > > An ugly hack ("avail_req < 2") has allowed booting a guest with many > VCPUs (max_vcpus, the guest only brings 2 VCPUs online), however that's > just to prove that that was the culprit - a real solution to this needs > more in-depth understading of the issue and potential solution. That's > basically very old code (pre-2012 at least) that got moved around into > the current shape of Xen today - please CC anyone relevant to the > discussion that you're aware of. > > Thoughts? > > > I think is a side-effect of the growth of the vm_event structure and the > fact that we have a single page ring. The check effectively sets a > threshold of having enough space for each vCPU to place at least one > more event on the ring, and if that's not the case it gets paused. OTOH > I think this would only have an effect on asynchronous events, for all > other events the vCPU is already paused. Is that the case you have? No, on the contrary, all my events are synchronous (the VCPU is paused waiting for the vm_event reply). I've debugged this a bit, and the problem seems to be that vm_event_wake_blocked() breaks here: 150 /* We remember which vcpu last woke up to avoid scanning always linearly 151 * from zero and starving higher-numbered vcpus under high load */ 152 if ( d->vcpu ) 153 { 154 int i, j, k; 155 156 for (i = ved->last_vcpu_wake_up + 1, j = 0; j < d->max_vcpus; i++, j++) 157 { 158 k = i % d->max_vcpus; 159 v = d->vcpu[k]; 160 if ( !v ) 161 continue; 162 163 if ( !(ved->blocked) || online >= avail_req ) 164break; 165 166 if ( test_and_clear_bit(ved->pause_flag, &v->pause_flags) ) 167 { 168 vcpu_unpause(v); 169 online++; 170 ved->blocked--; 171 ved->last_vcpu_wake_up = k; 172 } 173 } 174 } at "if ( !(ved->blocked) || online >= avail_req )". At this point, nothing ever gets unblocked. It's hard to believe that this is desired behaviour, as I don't know what could possibly happen for that condition to become false once all the online VCPUs are stuck (especially when the guest has just started booting). Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: don't segfault when creating domain with invalid pvusb device
Juergen Gross writes ("[PATCH] libxl: don't segfault when creating domain with invalid pvusb device"): > Creating a domain with an invalid controller specification for a pvusb > device will currently segfault. > > Avoid this by bailing out early in case of a mandatory xenstore path > not existing. This should be done by adding a test of tmp for NULL. libxl__xs_read does no logging so is not really an improvement. If you felt like inventing libxl__xs_read_must or something (maybe Wei has a suggestion for a better name) that calls ENOENT a failure instead, I'd be happy with that. Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
On 07/02/17 18:31, Razvan Cojocaru wrote: > On 02/07/2017 08:15 PM, Tamas K Lengyel wrote: >> >> On Tue, Feb 7, 2017 at 9:53 AM, Razvan Cojocaru >> mailto:rcojoc...@bitdefender.com>> wrote: >> >> Hello, >> >> Setting, e.g. 16 VCPUs for a HVM guest, ends up blocking the guest >> completely when subscribing to vm_events, apparently because of this >> code in xen/common/vm_event.c: >> >> 315 /* Give this vCPU a black eye if necessary, on the way out. >> 316 * See the comments above wake_blocked() for more information >> 317 * on how this mechanism works to avoid waiting. */ >> 318 avail_req = vm_event_ring_available(ved); >> 319 if( current->domain == d && avail_req < d->max_vcpus ) >> 320 vm_event_mark_and_pause(current, ved); >> >> It would appear that even if the guest only has 2 online VCPUs, the >> "avail_req < d->max_vcpus" condition will pause current, and we >> eventually end up with all the VCPUs paused. >> >> An ugly hack ("avail_req < 2") has allowed booting a guest with many >> VCPUs (max_vcpus, the guest only brings 2 VCPUs online), however that's >> just to prove that that was the culprit - a real solution to this needs >> more in-depth understading of the issue and potential solution. That's >> basically very old code (pre-2012 at least) that got moved around into >> the current shape of Xen today - please CC anyone relevant to the >> discussion that you're aware of. >> >> Thoughts? >> >> >> I think is a side-effect of the growth of the vm_event structure and the >> fact that we have a single page ring. The check effectively sets a >> threshold of having enough space for each vCPU to place at least one >> more event on the ring, and if that's not the case it gets paused. OTOH >> I think this would only have an effect on asynchronous events, for all >> other events the vCPU is already paused. Is that the case you have? > No, on the contrary, all my events are synchronous (the VCPU is paused > waiting for the vm_event reply). > > I've debugged this a bit, and the problem seems to be that > vm_event_wake_blocked() breaks here: > > 150 /* We remember which vcpu last woke up to avoid scanning always > linearly > 151 * from zero and starving higher-numbered vcpus under high load */ > 152 if ( d->vcpu ) > 153 { > 154 int i, j, k; > 155 > 156 for (i = ved->last_vcpu_wake_up + 1, j = 0; j < > d->max_vcpus; i++, j++) > 157 { > 158 k = i % d->max_vcpus; > 159 v = d->vcpu[k]; > 160 if ( !v ) > 161 continue; > 162 > 163 if ( !(ved->blocked) || online >= avail_req ) > 164break; > 165 > 166 if ( test_and_clear_bit(ved->pause_flag, &v->pause_flags) ) > 167 { > 168 vcpu_unpause(v); > 169 online++; > 170 ved->blocked--; > 171 ved->last_vcpu_wake_up = k; > 172 } > 173 } > 174 } > > at "if ( !(ved->blocked) || online >= avail_req )". At this point, > nothing ever gets unblocked. It's hard to believe that this is desired > behaviour, as I don't know what could possibly happen for that condition > to become false once all the online VCPUs are stuck (especially when the > guest has just started booting). I wouldn't bet that this logic has ever been tested. If you recall, the addition of register state into the vm_event ring made each entry far larger, which in turns makes it more likely to hit this condition. However, simply fixing the logic to re-online the cpus isn't a good solution either, as having $N vcpus paused at any one time because of ring contention is not conducive good system performance. Realistically, the ring size needs to be max_cpus * sizeof(largest vm_event) at an absolute minimum, and I guess this is now beyond 1 page? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/p2m: Reposition p2m_teardown_nestedp2m() to avoid its forward declaration
While adjusting these functions, use unsigned int rather than uint8_t for the loop variable, and fix the whitespace style. No functional change. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Tim Deegan CC: George Dunlap --- xen/arch/x86/mm/p2m.c | 36 +--- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 6548e9f..7675e9c 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -147,15 +147,29 @@ static void p2m_teardown_hostp2m(struct domain *d) } } -static void p2m_teardown_nestedp2m(struct domain *d); +static void p2m_teardown_nestedp2m(struct domain *d) +{ +unsigned int i; +struct p2m_domain *p2m; + +for ( i = 0; i < MAX_NESTEDP2M; i++ ) +{ +if ( !d->arch.nested_p2m[i] ) +continue; +p2m = d->arch.nested_p2m[i]; +list_del(&p2m->np2m_list); +p2m_free_one(p2m); +d->arch.nested_p2m[i] = NULL; +} +} static int p2m_init_nestedp2m(struct domain *d) { -uint8_t i; +unsigned int i; struct p2m_domain *p2m; mm_lock_init(&d->arch.nested_p2m_lock); -for (i = 0; i < MAX_NESTEDP2M; i++) +for ( i = 0; i < MAX_NESTEDP2M; i++ ) { d->arch.nested_p2m[i] = p2m = p2m_init_one(d); if ( p2m == NULL ) @@ -171,22 +185,6 @@ static int p2m_init_nestedp2m(struct domain *d) return 0; } -static void p2m_teardown_nestedp2m(struct domain *d) -{ -uint8_t i; -struct p2m_domain *p2m; - -for (i = 0; i < MAX_NESTEDP2M; i++) -{ -if ( !d->arch.nested_p2m[i] ) -continue; -p2m = d->arch.nested_p2m[i]; -list_del(&p2m->np2m_list); -p2m_free_one(p2m); -d->arch.nested_p2m[i] = NULL; -} -} - static void p2m_teardown_altp2m(struct domain *d) { unsigned int i; -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/p2m: Stop other vcpus using a nested p2m before clearing it
Until the IPI has completed, other processors might be running on this nested p2m object. clear_domain_page() does not guarantee to make 8-byte atomic updates, which means that a pagewalk on a remote processor might encounter a partial update. This is currently safe as other issues prevents a nested p2m ever being shared between two cpus (although this is contrary to the original plan). Setting p2m->np2m_base to P2M_BASE_EADDR before the IPI ensures that the IPI'd processors won't continue to use the flushed mappings. While modifying this function, remove all the trailing whitespace and tweak style in the affected areas. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Tim Deegan CC: George Dunlap --- xen/arch/x86/mm/p2m.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 7675e9c..c5b1642 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1618,8 +1618,10 @@ p2m_flush_table(struct p2m_domain *p2m) p2m_lock(p2m); -/* "Host" p2m tables can have shared entries &c that need a bit more - * care when discarding them */ +/* + * "Host" p2m tables can have shared entries &c that need a bit more care + * when discarding them. + */ ASSERT(!p2m_is_hostp2m(p2m)); /* Nested p2m's do not do pod, hence the asserts (and no pod lock)*/ ASSERT(page_list_empty(&p2m->pod.super)); @@ -1633,19 +1635,21 @@ p2m_flush_table(struct p2m_domain *p2m) /* This is no longer a valid nested p2m for any address space */ p2m->np2m_base = P2M_BASE_EADDR; - -/* Zap the top level of the trie */ -mfn = pagetable_get_mfn(p2m_get_pagetable(p2m)); -clear_domain_page(mfn); /* Make sure nobody else is using this p2m table */ nestedhvm_vmcx_flushtlb(p2m); +/* Zap the top level of the trie */ +mfn = pagetable_get_mfn(p2m_get_pagetable(p2m)); +clear_domain_page(mfn); + /* Free the rest of the trie pages back to the paging pool */ top = mfn_to_page(mfn); while ( (pg = page_list_remove_head(&p2m->pages)) ) -if ( pg != top ) +{ +if ( pg != top ) d->arch.paging.free_page(d, pg); +} page_list_add(top, &p2m->pages); p2m_unlock(p2m); -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] xen/kbdif: add multi-touch support
On Thu, Jan 26, 2017 at 09:46:47AM +0200, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > Usually you say a bit in the description of what it does and what's its usage is. Like: "Multi-touch fields re-use the page that is used by the other features which means that you can interleave multi-touch, motion, and key events." Along with: --- v2: Added 'mt' in front of XenBus entries Dropped 'mtouch' subdirectory. .. etc. > Signed-off-by: Oleksandr Andrushchenko > --- > xen/include/public/io/kbdif.h | 210 > ++ > 1 file changed, 210 insertions(+) > > diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h > index 446aed2478b5..74883267d6e6 100644 > --- a/xen/include/public/io/kbdif.h > +++ b/xen/include/public/io/kbdif.h > @@ -57,6 +57,12 @@ > * Backends, which support reporting of absolute coordinates for pointer > * device should set this to 1. > * > + * feature-multi-touch > + * Values: > + * > + * Backends, which support reporting of multi-touch events > + * should set this to 1. > + * > *- Pointer Device Parameters > > * > * width > @@ -87,6 +93,11 @@ > * Request backend to report absolute pointer coordinates > * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). > * > + * request-multi-touch > + * Values: > + * > + * Request backend to report multi-touch events. > + * > *--- Request Transport Parameters > --- > * > * event-channel > @@ -106,6 +117,25 @@ > * > * OBSOLETE, not recommended for use. > * PFN of the shared page. > + * > + *--- Multi-touch Device Parameters > --- > + * > + * mt-num-contacts s/mt/multi-touch/ please for all of the entries. > + * Values: > + * > + * Number of simultaneous touches reported. > + * > + * mt-width > + * Values: > + * > + * Width of the touch area to be used by the frontend > + * while reporting input events, pixels, [0; UINT32_MAX]. > + * > + * mt-height > + * Values: > + * > + * Height of the touch area to be used by the frontend > + * while reporting input events, pixels, [0; UINT32_MAX]. > */ ... And I think that is it! ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] x86/time: Adjust init-time handling of pit0_ticks
There is no need for the volatile cast in the timer interrupt; the compiler may not elide the update. This reduces the generated assembly from a read, local modify, write to a single add instruction. Drop the memory barriers from timer_irq_works(), as they are not needed. pit0_ticks is only modified by timer_interrupt() running on the same CPU, so that is required is a volatile reference to prevent the compiler from eliding the second read. Signed-off-by: Andrew Cooper --- CC: Jan Beulich v2: * Update commit message --- xen/arch/x86/io_apic.c | 6 ++ xen/arch/x86/time.c| 2 +- xen/include/xen/lib.h | 2 ++ 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c index 33e5927..f989978 100644 --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -1485,8 +1485,7 @@ static int __init timer_irq_works(void) { unsigned long t1, flags; -t1 = pit0_ticks; -mb(); +t1 = ACCESS_ONCE(pit0_ticks); local_save_flags(flags); local_irq_enable(); @@ -1501,8 +1500,7 @@ static int __init timer_irq_works(void) * might have cached one ExtINT interrupt. Finally, at * least one tick may be lost due to delays. */ -mb(); -if (pit0_ticks - t1 > 4) +if ( (ACCESS_ONCE(pit0_ticks) - t1) > 4 ) return 1; return 0; diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index d3b0c69..699dfb6 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -197,7 +197,7 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) return; /* Only for start-of-day interruopt tests in io_apic.c. */ -(*(volatile unsigned long *)&pit0_ticks)++; +pit0_ticks++; /* Rough hack to allow accurate timers to sort-of-work with no APIC. */ if ( !cpu_has_apic ) diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index d1171b7..1976e4b 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -56,6 +56,8 @@ #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]) + __must_be_array(x)) +#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x)) + #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m))) #define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m)) -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel