[Xen-devel] [qemu-mainline test] 105593: regressions - FAIL

2017-02-07 Thread osstest service owner
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

2017-02-07 Thread Vijay Kilari
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

2017-02-07 Thread David Scott

> 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

2017-02-07 Thread Juergen Gross
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

2017-02-07 Thread osstest service owner
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

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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()

2017-02-07 Thread Jan Beulich
>>> 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()

2017-02-07 Thread Jan Beulich
>>> 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()

2017-02-07 Thread Wei Liu
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

2017-02-07 Thread Juergen Gross
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

2017-02-07 Thread Julien Grall

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

2017-02-07 Thread Platform Team regression test user
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

2017-02-07 Thread Juergen Gross
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

2017-02-07 Thread Jan Beulich
 >>> 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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread Wei Liu
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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread osstest service owner
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()

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread Vijay Kilari
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

2017-02-07 Thread Wei Liu
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

2017-02-07 Thread Julien Grall

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()

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Joao Martins
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

2017-02-07 Thread Julien Grall

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

2017-02-07 Thread Juergen Gross
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()

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Julien Grall

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

2017-02-07 Thread Roger Pau Monné
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

2017-02-07 Thread Vijay Kilari
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

2017-02-07 Thread Julien Grall



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

2017-02-07 Thread Julien Grall



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

2017-02-07 Thread Roger Pau Monné
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()

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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()

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread Vijay Kilari
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

2017-02-07 Thread Julien Grall



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()

2017-02-07 Thread Chao Gao
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()

2017-02-07 Thread Jan Beulich
>>> 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.

2017-02-07 Thread Konrad Rzeszutek Wilk
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.

2017-02-07 Thread Konrad Rzeszutek Wilk
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

2017-02-07 Thread Roger Pau Monne
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

2017-02-07 Thread Julien Grall

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

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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

2017-02-07 Thread osstest service owner
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()

2017-02-07 Thread Chao Gao
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

2017-02-07 Thread osstest service owner
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

2017-02-07 Thread Ross Lagerwall
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()

2017-02-07 Thread Sergey Dyasli
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

2017-02-07 Thread Platform Team regression test user
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"

2017-02-07 Thread Konrad Rzeszutek Wilk
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()

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Roger Pau Monne
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

2017-02-07 Thread G.R.
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

2017-02-07 Thread Sergey Dyasli
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

2017-02-07 Thread Vijay Kilari
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()

2017-02-07 Thread Jan Beulich
>>> 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()

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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

2017-02-07 Thread Jan Beulich
>>> 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"

2017-02-07 Thread Jan Beulich
>>> 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()

2017-02-07 Thread Andrew Cooper
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"

2017-02-07 Thread Konrad Rzeszutek Wilk
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

2017-02-07 Thread Jan Beulich
>>> 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"

2017-02-07 Thread Jan Beulich
>>> 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()

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread Razvan Cojocaru
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

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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"

2017-02-07 Thread Konrad Rzeszutek Wilk
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"

2017-02-07 Thread Jan Beulich
>>> 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

2017-02-07 Thread osstest service owner
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.

2017-02-07 Thread anshul makkar

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

2017-02-07 Thread Roger Pau Monne
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

2017-02-07 Thread Roger Pau Monne
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

2017-02-07 Thread Konrad Rzeszutek Wilk
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

2017-02-07 Thread Platform Team regression test user
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)

2017-02-07 Thread Boris Ostrovsky
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

2017-02-07 Thread Andrii Anisov
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

2017-02-07 Thread Andrii Anisov
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

2017-02-07 Thread Andrii Anisov
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

2017-02-07 Thread Julien Grall

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

2017-02-07 Thread Platform Team regression test user
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

2017-02-07 Thread Boris Ostrovsky
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

2017-02-07 Thread Tamas K Lengyel
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

2017-02-07 Thread Stefano Stabellini
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"

2017-02-07 Thread Ian Jackson
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

2017-02-07 Thread Razvan Cojocaru
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

2017-02-07 Thread Ian Jackson
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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Andrew Cooper
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

2017-02-07 Thread Konrad Rzeszutek Wilk
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

2017-02-07 Thread Andrew Cooper
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


  1   2   >