[Xen-devel] [xen-unstable test] 139169: regressions - FAIL

[osstest service owner]

flight 139169 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139169/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
139032

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139032
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139032
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139032
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139032
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139032
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 139032
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139032
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 139032
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  643d8e566de59f247556e62a27ed7a5ac2e8a8cf
baseline version:
 xen  38eeb3864de40aa568c48f9f26271c141c62b50b

Last test of basis   139032  2019-07-16 01:51:20 Z4 days
Failing since139069  2019-07-17 02:03:57 Z3 days4 attempts
T

[Xen-devel] [linux-next test] 139158: regressions - FAIL

[osstest service owner]

flight 139158 linux-next real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139158/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 139100

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-bootfail blocked in 139100
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop  fail blocked in 139100
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop  fail blocked in 139100
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop  fail blocked in 139100
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop  fail blocked in 139100
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot  fail like 139100
 test-amd64-i386-xl-raw7 xen-boot fail  like 139100
 test-amd64-i386-examine   8 reboot   fail  like 139100
 test-amd64-i386-xl7 xen-boot fail  like 139100
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot  fail like 139100
 test-amd64-i386-xl-xsm7 xen-boot fail  like 139100
 test-amd64-i386-xl-shadow 7 xen-boot fail  like 139100
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 
139100
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-bootfail like 139100
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 
139100
 test-amd64-i386-libvirt-xsm   7 xen-boot fail  like 139100
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  7 xen-boot   fail like 139100
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-bootfail like 139100
 test-amd64-i386-pair 10 xen-boot/src_hostfail  like 139100
 test-amd64-i386-pair 11 xen-boot/dst_hostfail  like 139100
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail  like 139100
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot   fail like 139100
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail  like 139100
 test-amd64-i386-freebsd10-i386  7 xen-bootfail like 139100
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  7 xen-boot fail like 139100
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot   fail like 139100
 test-amd64-i386-freebsd10-amd64  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot  fail like 139100
 test-amd64-i386-libvirt   7 xen-boot fail  like 139100
 test-amd64-i386-xl-pvshim 7 xen-boot fail  like 139100
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot   fail like 139100
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot   fail like 139100
 build-armhf-pvops 6 kernel-build fail  like 139100
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-check  

Re: [Xen-devel] preparations for 4.12.1

[Christopher Clark]

On Fri, Jul 19, 2019 at 7:25 AM Jan Beulich  wrote:
>
> All,
>
> the release is due in early August. Please point out backports you
> find missing from the respective staging branch, but which you
> consider relevant.

Please can the following be added to the branch:

480800c769
argo: warn sendv() caller when ring is full

8966a3e9ab
argo: correctly report pending message length

7abd7c21b9
argo: suppress select logging messages

Thanks,

Christopher

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-linus bisection] complete test-amd64-i386-examine

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-examine
testid reboot

Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
  Bug introduced:  3bfe1fc46794631366faa3ef075e1b0ff7ba120a
  Bug not present: 4aa9fc2a435abe95a1e8d7f8c7b3d6356514b37a
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/139190/


  (Revision log too long, omitted.)


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-i386-examine.reboot.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-linus/test-amd64-i386-examine.reboot
 --summary-out=tmp/139190.bisection-summary --basis-template=133580 
--blessings=real,real-bisect linux-linus test-amd64-i386-examine reboot
Searching for failure / basis pass:
 139134 fail [host=debina1] / 138849 [host=albana1] 138813 [host=baroque1] 
138780 [host=albana0] 138754 [host=fiano1] 138735 [host=italia0] 138710 
[host=elbling1] 138680 [host=fiano0] 138661 [host=rimava1] 138639 
[host=debina0] 138612 [host=chardonnay0] 138584 [host=albana1] 138488 
[host=pinot1] 138386 [host=baroque0] 138245 [host=albana0] 138073 
[host=italia0] 137986 [host=elbling0] 137896 [host=fiano1] 137739 [host=fiano0] 
137686 [host=pinot1] 137589 [host=chardonnay1] 137484 [host=pinot0] 137\
 388 [host=albana0] 137283 [host=debina0] 137191 [host=albana1] 137125 
[host=fiano0] 137098 [host=italia0] 137055 [host=chardonnay1] 137015 
[host=rimava1] 136981 [host=elbling1] 136911 [host=fiano1] 136823 
[host=baroque0] 136594 [host=albana0] 136433 [host=albana1] 136243 
[host=pinot0] 136116 [host=baroque1] 135988 [host=fiano0] 135873 
[host=chardonnay0] 135753 [host=baroque0] 135539 [host=italia0] 135443 
[host=albana0] 135426 [host=rimava1] 134885 [host=debina0] 134749 
[host=elbling1] 133995 [ho\
 st=albana0] 133973 [host=pinot1] 133934 [host=fiano0] 133902 [host=baroque1] 
133863 [host=italia1] 133829 [host=merlot1] 133778 [host=merlot0] 133738 
[host=chardonnay1] 133695 [host=baroque0] 133673 [host=italia0] 133631 
[host=albana1] 133605 [host=elbling1] 133580 [host=albana0] 133567 
[host=fiano0] 133555 [host=pinot1] 133510 [host=fiano1] 133474 [host=baroque1] 
133293 [host=baroque0] 133280 [host=merlot0] 132911 [host=joubertin1] 132804 
[host=fiano0] 132754 [host=elbling1] 132669 [host=debina\
 0] 132599 ok.
Failure / basis pass flights: 139134 / 132599
(tree with no url: minios)
Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 3bfe1fc46794631366faa3ef075e1b0ff7ba120a 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
cce01f538fb4d6ae8c13c88cfc0d3caf5baca833 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
9cca02d8ffc23e9688a971d858e4ffdff5389b11 
30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 
38eeb3864de40aa568c48f9f26271c141c62b50b
Basis pass 4aa9fc2a435abe95a1e8d7f8c7b3d6356514b37a 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
ef529e6ab7c31290a33045bb1f1837447cc0eb56 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
de5b678ca4dcdfa83e322491d478d66df56c1986 
a698c8995ffb2838296ec284fe3c4ad33dfca307 
08b908ba63dee8bc313983c5e412852cbcbcda85
Generating revisions with ./adhoc-revtuple-generator  
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#4aa9fc2a435abe95a1e8d7f8c7b3d6356514b37a-3bfe1fc46794631366faa3ef075e1b0ff7ba120a
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/osstest/ovmf.git#ef529e6ab7c31290a33045bb1f1837447cc0eb56-cce01f538fb4d6ae8c13c88cfc0d3caf5baca833
 git://xenbits.xen.org/qemu-xen-traditional.\
 
git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798
 
git://xenbits.xen.org/qemu-xen.git#de5b678ca4dcdfa83e322491d478d66df56c1986-9cca02d8ffc23e9688a971d858e4ffdff5389b11
 
git://xenbits.xen.org/osstest/seabios.git#a698c8995ffb2838296ec284fe3c4ad33dfca307-30f1e41f04fb4c715d27f987f003cfc31c9ff4f3
 
git://xenbits.xen.org/xen.git#08b908ba63dee8bc313983c5e412852cbcbcda85-38eeb3864de40aa568c48f9f26271c141c62b50b
adhoc-

[Xen-devel] [linux-4.19 test] 139152: regressions - FAIL

[osstest service owner]

flight 139152 linux-4.19 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139152/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-examine 11 examine-serial/bootloader  fail pass in 139121

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux3bd837bfe431839a378e9d421af05b2e22a6d329
baseline version:
 linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d

Last test of basis   129313  2018-11-02 05:39:08 Z  259 days
Failing since129412  2018-11-04 14:10:15 Z  257 days  164 attempts
Testing same since   139004  2019-07-14 23:35:54 Z4 days6 attempts


2271 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 bui

Re: [Xen-devel] [PATCH v3 0/9] x86: Concurrent TLB flushes

[Dave Hansen]

Thanks for doing this, it's something I've been hoping someone would do
for a long time.

While I kinda wish we had performance data for each individual patch (at
least the behavior-changing ones), this does look like a good
improvement.  That might, for instance, tell is a bit about how the
separating out "is_lazy" compares to the "check before setting"
optimization.  But, they're both sane enough on their own that I'm not
too worried.

I had some nits that I hope get covered in later revisions, if sent.
But, overall looks fine.  For the series:

Reviewed-by: Dave Hansen 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Roman Shaposhnik]

CCing relevant maintainers as well (sorry for not doing it first time around)

On Fri, Jul 19, 2019 at 12:31 PM Roman Shaposhnik  wrote:
>
> Hi!
>
> we're using Xen on Advantech ARK-2250 Embedded Box PC:
> 
> https://www.elmark.com.pl/web/uploaded/karty_produktow/advantech/ark-2250l/ark-2250l_instrukcja-uzytkownika.pdf
>
> After upgrading to Xen 4.12.0 from 4.11.0 we now have to utilize  
> iommu=no-igfx
> workaround as per:
>  https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html#iommu
>
> Without the workaround the screen appears to be garbled with colored
> static noise and the following message keeps showing up:
> (XEN) printk: 26235 messages suppressed.
> (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
> 8e43c000, iommu reg = 82c00021d000
> (XEN) printk: 26303 messages suppressed.
> (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
> 8e2c6000, iommu reg = 82c00021d000
>
> Once iommu=no-igfx is applied the box can boot fine.
>
> At the end of this email, you can see a full log of the box booting
> all the way into Dom0 with iommu=no-igfx applied. I am also attaching
> similar log without no-igfx
>
> Please let me know if you need any more information to help us diagnose this.
>
> Thanks,
> Roman.
>
> 0x:0x00:0x02.0x0: ROM: 0x1 bytes at 0x8968d018
> 0x:0x02:0x00.0x0: ROM: 0x1 bytes at 0x89640018
> 0x:0x00:0x1f.0x6: ROM: 0x10c00 bytes at 0x8962f018
>  Xen 4.12.0
> (XEN) Xen version 4.12.0 (@) (gcc (Alpine 6.4.0) 6.4.0) debug=n  Mon
> Jun 17 18:50:07 UTC 2019
> (XEN) Latest ChangeSet:
> (XEN) Bootloader: GRUB 2.03
> (XEN) Command line: iommu=no-igfx com1=115200,8n1 console=com1,vga
> dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false
> (XEN) Xen image load base address: 0x8860
> (XEN) Video information:
> (XEN)  VGA is graphics mode 1680x1050, 32 bpp
> (XEN) Disc information:
> (XEN)  Found 0 MBR signatures
> (XEN)  Found 1 EDD information structures
> (XEN) EFI RAM map:
> (XEN)   - 00058000 (usable)
> (XEN)  00058000 - 00059000 (reserved)
> (XEN)  00059000 - 0009f000 (usable)
> (XEN)  0009f000 - 000a (reserved)
> (XEN)  0010 - 8648a000 (usable)
> (XEN)  8648a000 - 8648b000 (ACPI NVS)
> (XEN)  8648b000 - 864b5000 (reserved)
> (XEN)  864b5000 - 8c224000 (usable)
> (XEN)  8c224000 - 8c528000 (reserved)
> (XEN)  8c528000 - 8c736000 (usable)
> (XEN)  8c736000 - 8cea7000 (ACPI NVS)
> (XEN)  8cea7000 - 8d2ff000 (reserved)
> (XEN)  8d2ff000 - 8d30 (usable)
> (XEN)  8d30 - 8d40 (reserved)
> (XEN)  e000 - f000 (reserved)
> (XEN)  fe00 - fe011000 (reserved)
> (XEN)  fec0 - fec01000 (reserved)
> (XEN)  fee0 - fee01000 (reserved)
> (XEN)  ff00 - 0001 (reserved)
> (XEN)  0001 - 00016e00 (usable)
> (XEN) ACPI: RSDP 8CE49000, 0024 (r2 ALASKA)
> (XEN) ACPI: XSDT 8CE490A8, 00CC (r1 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: FACP 8CE6C370, 010C (r5 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: DSDT 8CE49208, 23167 (r2 ALASKA   A M I   1072009 INTL 20120913)
> (XEN) ACPI: FACS 8CE8EF80, 0040
> (XEN) ACPI: APIC 8CE6C480, 0084 (r3 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: FPDT 8CE6C508, 0044 (r1 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: FIDT 8CE6C550, 009C (r1 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: MCFG 8CE6C5F0, 003C (r1 ALASKA   A M I   1072009 MSFT   97)
> (XEN) ACPI: HPET 8CE6C630, 0038 (r1 ALASKA   A M I   1072009 AMI.5000B)
> (XEN) ACPI: LPIT 8CE6C668, 0094 (r1 INTEL   SKL-ULT0 MSFT   5F)
> (XEN) ACPI: SSDT 8CE6C700, 0248 (r2 INTEL  sensrhub0 INTL 20120913)
> (XEN) ACPI: SSDT 8CE6C948, 2BAE (r2 INTEL  PtidDevc 1000 INTL 20120913)
> (XEN) ACPI: SSDT 8CE6F4F8, 0BE3 (r2 INTEL  Ther_Rvp 1000 INTL 20120913)
> (XEN) ACPI: SSDT 8CE700E0, 04A3 (r2 INTEL zpodd 1000 INTL 20120913)
> (XEN) ACPI: DBGP 8CE70588, 0034 (r1 INTEL  0 MSFT   5F)
> (XEN) ACPI: DBG2 8CE705C0, 0054 (r0 INTEL  0 MSFT   5F)
> (XEN) ACPI: SSDT 8CE70618, 06E9 (r2  INTEL xh_rvp070 INTL 20120913)
> (XEN) ACPI: SSDT 8CE70D08, 547E (r2 SaSsdt  SaSsdt  3000 INTL 20120913)
> (XEN) ACPI: UEFI 8CE76188, 0042 (r10 0)
> (XEN) ACPI: SSDT 8CE761D0, 0E73 (r2 CpuRef  CpuSsdt 3000 INTL 20120913)
> (XEN) ACPI: BGRT 8CE77048, 0038 (r1 ALASKA   A M I   1072009 AMI 10013)
> (XEN) ACPI: DMAR 8CE77080, 00A8 (r1 INTEL  SKL 1 INTL1)
> (XEN) ACPI: TPM2 8CE77128, 0034 (r3Tpm2Tabl1 AMI 0)
> (XEN) ACPI: ASF! 8CE77160, 00A5 (r32 INTEL   HCG1 TFSMF4240)
> (XE

Re: [Xen-devel] PCI Passthrough bug with x86 HVM

[Stefano Stabellini]

On Wed, 26 Jun 2019, Stefano Stabellini wrote:
> On Wed, 26 Jun 2019, Juergen Gross wrote:
> > On 26.06.19 14:21, Chao Gao wrote:
> > > On Wed, Jun 26, 2019 at 08:17:50AM +0200, Juergen Gross wrote:
> > > > On 24.06.19 20:47, Stefano Stabellini wrote:
> > > > > + xen-devel
> > > > > 
> > > > > On Mon, 24 Jun 2019, Stefano Stabellini wrote:
> > > > > > Hi all,
> > > > > > 
> > > > > > I might have found a bug with PCI passthrough to a Linux HVM guest 
> > > > > > on
> > > > > > x86 with Xen 4.12. It is not easy for me to get access, and 
> > > > > > especially
> > > > > > change components, on this particular system, and I don't have 
> > > > > > access
> > > > > > to
> > > > > > other x86 boxes at the moment, so apologies for the partial
> > > > > > information
> > > > > > report. The setup is as follow:
> > > > > > 
> > > > > > - two PCI devices have been assigned to a HVM guest, everything is
> > > > > > fine
> > > > > > - reboot the guest from inside, i.e. `reboot' in Linux
> > > > > > - after the reboot completes, only one device is assigned
> > > > > > 
> > > > > > Before the reboot, I see all the appropriate xenstore entries for 
> > > > > > both
> > > > > > devices. Everything is fine. After the reboot, I can only see the
> > > > > > xenstore entries of one device. It is as if the other device
> > > > > > "disappeared" without throwing any errors.
> > > > > > 
> > > > > > Have you seen this before? Do you know if it has been fixed in
> > > > > > staging?
> > > > > > I noticed this fix which seems to be very relevant:
> > > > > > 
> > > > > > https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg01616.html
> > > > > > 
> > > > > > but it is already included in 4.12.
> > > > 
> > > > Stefano, could you please try the attached patch? It is only compile
> > > > tested for now.
> > > > 
> > > > 
> > > > Juergen
> > > 
> > > > From ea95dcdfc60a895cc43baf34c8e0fb088e10008d Mon Sep 17 00:00:00 2001
> > > > From: Juergen Gross 
> > > > To: xen-devel@lists.xenproject.org
> > > > Cc: Ian Jackson 
> > > > Cc: Wei Liu 
> > > > Date: Wed, 26 Jun 2019 08:15:28 +0200
> > > > Subject: [PATCH] libxl: fix pci device re-assigning after domain reboot
> > > > 
> > > > After a reboot of a guest only the first pci device configuration will
> > > > be retrieved from Xenstore resulting in loss of any further assigned
> > > > passed through pci devices.
> > > > 
> > > > The main reason is that all passed through pci devices reside under a
> > > > common root device "0" in Xenstore. So when the device list is rebuilt
> > > > from Xenstore after a reboot the sub-devices below that root device
> > > > need to be selected instead of using the root device number as a
> > > > selector.
> > > > 
> > > > Fix that by adding a new member to struct libxl_device_type which when
> > > > set is used to get the number of devices. Add such a member for pci to
> > > > get the correct number of pci devices instead of implying it from the
> > > > number of pci root devices (which will always be 1).
> > > > 
> > > > While at it fix the type of libxl__device_pci_from_xs_be() to match
> > > > the one of the .from_xenstore member of struct libxl_device_type. This
> > > > fixes a latent bug checking the return value of a function returning
> > > > void.
> > > > 
> > > > Signed-off-by: Juergen Gross 
> > > 
> > > Tested-by: Chao Gao 
> > 
> > Thanks!
> 
> Thank you very much both of you! I'll let you know if it works.

Tested-by: Stefano Stabellini 

Let's get it in the tree, thanks!

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [GIT PULL] xen: fixes and features for 5.3-rc1

[pr-tracker-bot]

The pull request you sent on Fri, 19 Jul 2019 08:09:25 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
> for-linus-5.3a-rc1-tag

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b5d72dda8976e878be47415b94bca8465d1fa22d

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

[Roman Shaposhnik]

Hi!

we're using Xen on Advantech ARK-2250 Embedded Box PC:

https://www.elmark.com.pl/web/uploaded/karty_produktow/advantech/ark-2250l/ark-2250l_instrukcja-uzytkownika.pdf

After upgrading to Xen 4.12.0 from 4.11.0 we now have to utilize  iommu=no-igfx
workaround as per:
 https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html#iommu

Without the workaround the screen appears to be garbled with colored
static noise and the following message keeps showing up:
(XEN) printk: 26235 messages suppressed.
(XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
8e43c000, iommu reg = 82c00021d000
(XEN) printk: 26303 messages suppressed.
(XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
8e2c6000, iommu reg = 82c00021d000

Once iommu=no-igfx is applied the box can boot fine.

At the end of this email, you can see a full log of the box booting
all the way into Dom0 with iommu=no-igfx applied. I am also attaching
similar log without no-igfx

Please let me know if you need any more information to help us diagnose this.

Thanks,
Roman.

0x:0x00:0x02.0x0: ROM: 0x1 bytes at 0x8968d018
0x:0x02:0x00.0x0: ROM: 0x1 bytes at 0x89640018
0x:0x00:0x1f.0x6: ROM: 0x10c00 bytes at 0x8962f018
 Xen 4.12.0
(XEN) Xen version 4.12.0 (@) (gcc (Alpine 6.4.0) 6.4.0) debug=n  Mon
Jun 17 18:50:07 UTC 2019
(XEN) Latest ChangeSet:
(XEN) Bootloader: GRUB 2.03
(XEN) Command line: iommu=no-igfx com1=115200,8n1 console=com1,vga
dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false
(XEN) Xen image load base address: 0x8860
(XEN) Video information:
(XEN)  VGA is graphics mode 1680x1050, 32 bpp
(XEN) Disc information:
(XEN)  Found 0 MBR signatures
(XEN)  Found 1 EDD information structures
(XEN) EFI RAM map:
(XEN)   - 00058000 (usable)
(XEN)  00058000 - 00059000 (reserved)
(XEN)  00059000 - 0009f000 (usable)
(XEN)  0009f000 - 000a (reserved)
(XEN)  0010 - 8648a000 (usable)
(XEN)  8648a000 - 8648b000 (ACPI NVS)
(XEN)  8648b000 - 864b5000 (reserved)
(XEN)  864b5000 - 8c224000 (usable)
(XEN)  8c224000 - 8c528000 (reserved)
(XEN)  8c528000 - 8c736000 (usable)
(XEN)  8c736000 - 8cea7000 (ACPI NVS)
(XEN)  8cea7000 - 8d2ff000 (reserved)
(XEN)  8d2ff000 - 8d30 (usable)
(XEN)  8d30 - 8d40 (reserved)
(XEN)  e000 - f000 (reserved)
(XEN)  fe00 - fe011000 (reserved)
(XEN)  fec0 - fec01000 (reserved)
(XEN)  fee0 - fee01000 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN)  0001 - 00016e00 (usable)
(XEN) ACPI: RSDP 8CE49000, 0024 (r2 ALASKA)
(XEN) ACPI: XSDT 8CE490A8, 00CC (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FACP 8CE6C370, 010C (r5 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DSDT 8CE49208, 23167 (r2 ALASKA   A M I   1072009 INTL 20120913)
(XEN) ACPI: FACS 8CE8EF80, 0040
(XEN) ACPI: APIC 8CE6C480, 0084 (r3 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FPDT 8CE6C508, 0044 (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FIDT 8CE6C550, 009C (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: MCFG 8CE6C5F0, 003C (r1 ALASKA   A M I   1072009 MSFT   97)
(XEN) ACPI: HPET 8CE6C630, 0038 (r1 ALASKA   A M I   1072009 AMI.5000B)
(XEN) ACPI: LPIT 8CE6C668, 0094 (r1 INTEL   SKL-ULT0 MSFT   5F)
(XEN) ACPI: SSDT 8CE6C700, 0248 (r2 INTEL  sensrhub0 INTL 20120913)
(XEN) ACPI: SSDT 8CE6C948, 2BAE (r2 INTEL  PtidDevc 1000 INTL 20120913)
(XEN) ACPI: SSDT 8CE6F4F8, 0BE3 (r2 INTEL  Ther_Rvp 1000 INTL 20120913)
(XEN) ACPI: SSDT 8CE700E0, 04A3 (r2 INTEL zpodd 1000 INTL 20120913)
(XEN) ACPI: DBGP 8CE70588, 0034 (r1 INTEL  0 MSFT   5F)
(XEN) ACPI: DBG2 8CE705C0, 0054 (r0 INTEL  0 MSFT   5F)
(XEN) ACPI: SSDT 8CE70618, 06E9 (r2  INTEL xh_rvp070 INTL 20120913)
(XEN) ACPI: SSDT 8CE70D08, 547E (r2 SaSsdt  SaSsdt  3000 INTL 20120913)
(XEN) ACPI: UEFI 8CE76188, 0042 (r10 0)
(XEN) ACPI: SSDT 8CE761D0, 0E73 (r2 CpuRef  CpuSsdt 3000 INTL 20120913)
(XEN) ACPI: BGRT 8CE77048, 0038 (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DMAR 8CE77080, 00A8 (r1 INTEL  SKL 1 INTL1)
(XEN) ACPI: TPM2 8CE77128, 0034 (r3Tpm2Tabl1 AMI 0)
(XEN) ACPI: ASF! 8CE77160, 00A5 (r32 INTEL   HCG1 TFSMF4240)
(XEN) System RAM: 4003MB (4099736kB)
(XEN) Domain heap initialised
(XEN) ACPI: 32/64X FACS address mismatch in FADT -
8ce8ef80/, using 32
(XEN) IOAPIC[0]: apic_id 2, version 32, address 0xfec0, GSI 0-119
(XEN) Enabling APIC mode:  Flat.  Using 1 I/O APICs
(XEN) [VT-D]  RMRR address range 8d80..8fff

Re: [Xen-devel] [PATCH v3 04/14] AMD/IOMMU: use bit field for IRTE

[Andrew Cooper]

On 19/07/2019 17:16, Jan Beulich wrote:
> On 19.07.2019 17:56, Andrew Cooper wrote:
>> On 16/07/2019 17:36, Jan Beulich wrote:
>>> At the same time restrict its scope to just the single source file
>>> actually using it, and abstract accesses by introducing a union of
>>> pointers. (A union of the actual table entries is not used to make it
>>> impossible to [wrongly, once the 128-bit form gets added] perform
>>> pointer arithmetic / array accesses on derived types.)
>>>
>>> Also move away from updating the entries piecemeal: Construct a full new
>>> entry, and write it out.
>>>
>>> Signed-off-by: Jan Beulich 
>> I'm still not entirely convinced by extra union and containerof(), but
>> the result looks correct.
> And I'm still open to going the other way, if you're convinced that
> in update_intremap_entry() this
>
>  struct irte_basic basic = {
>  .flds = {
>  .remap_en = true,
>  .int_type = int_type,
>  .dm = dest_mode,
>  .dest = dest,
>  .vector = vector,
>  }
>  };
>
> (and similarly then for the 128-bit form, and of course .flds
> inserted at other use sites) is overall better than the current
> variant.

I've just experimented with the attached delta and it does compile in
CentOS 6, which is the usual culprit for problems in this area.

I do think the result is easier-to-read code, which I am definitely in
favour of.

My ack still stands in all affected patches, but ideally with this kind
of change folded in appropriately.

~Andrew
>From e028cb655e889ca392023925280f30729be29be7 Mon Sep 17 00:00:00 2001
From: Andrew Cooper 
Date: Fri, 19 Jul 2019 19:20:27 +0100
Subject: Experiment without containerof


diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index d8d0f71d0d..dc781bb9f1 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -24,41 +24,37 @@
 #include 
 #include 
 
-struct irte_basic {
-bool remap_en:1;
-bool sup_io_pf:1;
-unsigned int int_type:3;
-bool rq_eoi:1;
-bool dm:1;
-bool guest_mode:1; /* MBZ */
-unsigned int dest:8;
-unsigned int vector:8;
-unsigned int :8;
-};
-
 union irte32 {
-uint32_t raw[1];
-struct irte_basic basic;
-};
-
-struct irte_full {
-bool remap_en:1;
-bool sup_io_pf:1;
-unsigned int int_type:3;
-bool rq_eoi:1;
-bool dm:1;
-bool guest_mode:1; /* MBZ */
-unsigned int dest_lo:24;
-unsigned int :32;
-unsigned int vector:8;
-unsigned int :24;
-unsigned int :24;
-unsigned int dest_hi:8;
+uint32_t raw;
+struct {
+bool remap_en:1;
+bool sup_io_pf:1;
+unsigned int int_type:3;
+bool rq_eoi:1;
+bool dm:1;
+bool guest_mode:1; /* MBZ */
+unsigned int dest:8;
+unsigned int vector:8;
+unsigned int :8;
+} flds;
 };
 
 union irte128 {
 uint64_t raw[2];
-struct irte_full full;
+struct {
+bool remap_en:1;
+bool sup_io_pf:1;
+unsigned int int_type:3;
+bool rq_eoi:1;
+bool dm:1;
+bool guest_mode:1; /* MBZ */
+unsigned int dest_lo:24;
+unsigned int :32;
+unsigned int vector:8;
+unsigned int :24;
+unsigned int :24;
+unsigned int dest_hi:8;
+} flds;
 };
 
 union irte_ptr {
@@ -187,7 +183,7 @@ static void free_intremap_entry(const struct amd_iommu *iommu,
 entry.ptr128->raw[1] = 0;
 }
 else
-ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
+ACCESS_ONCE(entry.ptr32->raw) = 0;
 
 __clear_bit(index, get_ivrs_mappings(iommu->seg)[bdf].intremap_inuse);
 }
@@ -199,35 +195,36 @@ static void update_intremap_entry(const struct amd_iommu *iommu,
 {
 if ( iommu->ctrl.ga_en )
 {
-struct irte_full full = {
-.remap_en = true,
-.int_type = int_type,
-.dm = dest_mode,
-.dest_lo = dest,
-.dest_hi = dest >> 24,
-.vector = vector,
+union irte128 irte = {
+.flds = {
+.remap_en = true,
+.int_type = int_type,
+.dm = dest_mode,
+.dest_lo = dest,
+.dest_hi = dest >> 24,
+.vector = vector,
+},
 };
 
-ASSERT(!entry.ptr128->full.remap_en);
-entry.ptr128->raw[1] =
-container_of(&full, union irte128, full)->raw[1];
+ASSERT(!entry.ptr128->flds.remap_en);
+entry.ptr128->raw[1] = irte.raw[1];
 /* High half needs to be set before low one (containing RemapEn). */
 smp_wmb();
-ACCESS_ONCE(entry.ptr128->raw[0]) =
-container_of(&full, union irte128, full)->raw[0];
+ACCESS_ONCE(entry.ptr128->raw[0]) = irte.raw[0];
 }
 else
 {
-struct irte_basic basic = {
-.remap_en = true,
-.int_type = int_type,

Re: [Xen-devel] [PATCH v3 14/14] AMD/IOMMU: process softirqs while dumping IRTs

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:41:21PM +, Jan Beulich wrote:
> When there are sufficiently many devices listed in the ACPI tables (no
> matter if they actually exist), output may take way longer than the
> watchdog would like.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: New.
> ---
> TBD: Seeing the volume of output I wonder whether we should further
>   suppress logging headers of devices which have no active entry
>   (i.e. emit the header only upon finding the first IRTE worth
>   logging). And while minor for the total volume of output I'm
>   also unconvinced logging both a "per device" header line and a
>   "shared" one makes sense, when only one of the two can actually
>   be followed by actual contents.
> 
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -22,6 +22,7 @@
>   #include 
>   #include 
>   #include 
> +#include 
>   
>   struct irte_basic {
>   bool remap_en:1;
> @@ -917,6 +918,8 @@ static int dump_intremap_mapping(const s
>   dump_intremap_table(iommu, ivrs_mapping->intremap_table);
>   spin_unlock_irqrestore(&(ivrs_mapping->intremap_lock), flags);
>   
> +process_pending_softirqs();
> +
>   return 0;
>   }
>   
> 

-- 
Brian Woods

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 12/14] AMD/IOMMU: enable x2APIC mode when available

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:40:33PM +, Jan Beulich wrote:
> In order for the CPUs to use x2APIC mode, the IOMMU(s) first need to be
> switched into suitable state.
> 
> The post-AP-bringup IRQ affinity adjustment is done also for the non-
> x2APIC case, matching what VT-d does.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: Set GAEn (and other control register bits) earlier. Also clear the
>  bits enabled here in amd_iommu_init_cleanup(). Re-base. Pass NULL
>  CPU mask to set_{x2apic,msi}_affinity().
> v2: Drop cpu_has_cx16 check. Add comment.
> ---
> TBD: Instead of the system_state check in iov_enable_xt() the function
>   could also zap its own hook pointer, at which point it could also
>   become __init. This would, however, require that either
>   resume_x2apic() be bound to ignore iommu_enable_x2apic() errors
>   forever, or that iommu_enable_x2apic() be slightly re-arranged to
>   not return -EOPNOTSUPP when finding a NULL hook during resume.
> 
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -834,6 +834,30 @@ static bool_t __init set_iommu_interrupt
>   return 1;
>   }
>   
> +int iov_adjust_irq_affinities(void)
> +{
> +const struct amd_iommu *iommu;
> +
> +if ( !iommu_enabled )
> +return 0;
> +
> +for_each_amd_iommu ( iommu )
> +{
> +struct irq_desc *desc = irq_to_desc(iommu->msi.irq);
> +unsigned long flags;
> +
> +spin_lock_irqsave(&desc->lock, flags);
> +if ( iommu->ctrl.int_cap_xt_en )
> +set_x2apic_affinity(desc, NULL);
> +else
> +set_msi_affinity(desc, NULL);
> +spin_unlock_irqrestore(&desc->lock, flags);
> +}
> +
> +return 0;
> +}
> +__initcall(iov_adjust_irq_affinities);
> +
>   /*
>* Family15h Model 10h-1fh erratum 746 (IOMMU Logging May Stall 
> Translations)
>* Workaround:
> @@ -1047,7 +1071,7 @@ static void * __init allocate_ppr_log(st
>   IOMMU_PPR_LOG_DEFAULT_ENTRIES, "PPR Log");
>   }
>   
> -static int __init amd_iommu_init_one(struct amd_iommu *iommu)
> +static int __init amd_iommu_init_one(struct amd_iommu *iommu, bool intr)
>   {
>   if ( allocate_cmd_buffer(iommu) == NULL )
>   goto error_out;
> @@ -1058,7 +1082,7 @@ static int __init amd_iommu_init_one(str
>   if ( iommu->features.flds.ppr_sup && !allocate_ppr_log(iommu) )
>   goto error_out;
>   
> -if ( !set_iommu_interrupt_handler(iommu) )
> +if ( intr && !set_iommu_interrupt_handler(iommu) )
>   goto error_out;
>   
>   /* To make sure that device_table.buffer has been successfully 
> allocated */
> @@ -1087,8 +,16 @@ static void __init amd_iommu_init_cleanu
>   list_for_each_entry_safe ( iommu, next, &amd_iommu_head, list )
>   {
>   list_del(&iommu->list);
> +
> +iommu->ctrl.ga_en = 0;
> +iommu->ctrl.xt_en = 0;
> +iommu->ctrl.int_cap_xt_en = 0;
> +
>   if ( iommu->enabled )
>   disable_iommu(iommu);
> +else if ( iommu->mmio_base )
> +writeq(iommu->ctrl.raw,
> +   iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
>   
>   deallocate_ring_buffer(&iommu->cmd_buffer);
>   deallocate_ring_buffer(&iommu->event_log);
> @@ -1290,7 +1322,7 @@ static int __init amd_iommu_prepare_one(
>   return 0;
>   }
>   
> -int __init amd_iommu_init(void)
> +int __init amd_iommu_prepare(bool xt)
>   {
>   struct amd_iommu *iommu;
>   int rc = -ENODEV;
> @@ -1305,9 +1337,14 @@ int __init amd_iommu_init(void)
>   if ( unlikely(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_MSI) )
>   goto error_out;
>   
> +/* Have we been here before? */
> +if ( ivhd_type )
> +return 0;
> +
>   rc = amd_iommu_get_supported_ivhd_type();
>   if ( rc < 0 )
>   goto error_out;
> +BUG_ON(!rc);
>   ivhd_type = rc;
>   
>   rc = amd_iommu_get_ivrs_dev_entries();
> @@ -1323,9 +1360,37 @@ int __init amd_iommu_init(void)
>   rc = amd_iommu_prepare_one(iommu);
>   if ( rc )
>   goto error_out;
> +
> +rc = -ENODEV;
> +if ( xt && (!iommu->features.flds.ga_sup || 
> !iommu->features.flds.xt_sup) )
> +goto error_out;
> +}
> +
> +for_each_amd_iommu ( iommu )
> +{
> +/* NB: There's no need to actually write these out right here. */
> +iommu->ctrl.ga_en |= xt;
> +iommu->ctrl.xt_en = xt;
> +iommu->ctrl.int_cap_xt_en = xt;
>   }
>   
>   rc = amd_iommu_update_ivrs_mapping_acpi();
> +
> + error_out:
> +if ( rc )
> +{
> +amd_iommu_init_cleanup();
> +ivhd_type = 0;
> +}
> +
> +return rc;
> +}
> +
> +int __init amd_iommu_init(bool xt)
> +{
> +struct amd_iommu *iommu;
> +int rc = amd_iommu_prepare(xt);
> +
>   if ( rc )
>   goto error_out;
>   
> @@ -1351,7 +1416

Re: [Xen-devel] [PATCH v3 11/14] AMD/IOMMU: adjust setup of internal interrupt for x2APIC mode

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:39:58PM +, Jan Beulich wrote:
> In order to be able to express all possible destinations we need to make
> use of this non-MSI-capability based mechanism. The new IRQ controller
> structure can re-use certain MSI functions, though.
> 
> For now general and PPR interrupts still share a single vector, IRQ, and
> hence handler.
> 
> Signed-off-by: Jan Beulich 
> Reviewed-by: Andrew Cooper 

Acked-by: Brian Woods 

> ---
> v3: Re-base.
> 
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -472,6 +472,44 @@ static hw_irq_controller iommu_maskable_
>   .set_affinity = set_msi_affinity,
>   };
>   
> +static void set_x2apic_affinity(struct irq_desc *desc, const cpumask_t *mask)
> +{
> +struct amd_iommu *iommu = desc->action->dev_id;
> +unsigned int dest = set_desc_affinity(desc, mask);
> +union amd_iommu_x2apic_control ctrl = {};
> +unsigned long flags;
> +
> +if ( dest == BAD_APICID )
> +return;
> +
> +msi_compose_msg(desc->arch.vector, NULL, &iommu->msi.msg);
> +iommu->msi.msg.dest32 = dest;
> +
> +ctrl.dest_mode = MASK_EXTR(iommu->msi.msg.address_lo,
> +   MSI_ADDR_DESTMODE_MASK);
> +ctrl.int_type = MASK_EXTR(iommu->msi.msg.data,
> +  MSI_DATA_DELIVERY_MODE_MASK);
> +ctrl.vector = desc->arch.vector;
> +ctrl.dest_lo = dest;
> +ctrl.dest_hi = dest >> 24;
> +
> +spin_lock_irqsave(&iommu->lock, flags);
> +writeq(ctrl.raw, iommu->mmio_base + IOMMU_XT_INT_CTRL_MMIO_OFFSET);
> +writeq(ctrl.raw, iommu->mmio_base + IOMMU_XT_PPR_INT_CTRL_MMIO_OFFSET);
> +spin_unlock_irqrestore(&iommu->lock, flags);
> +}
> +
> +static hw_irq_controller iommu_x2apic_type = {
> +.typename = "IOMMU-x2APIC",
> +.startup  = irq_startup_none,
> +.shutdown = irq_shutdown_none,
> +.enable   = irq_enable_none,
> +.disable  = irq_disable_none,
> +.ack  = ack_nonmaskable_msi_irq,
> +.end  = end_nonmaskable_msi_irq,
> +.set_affinity = set_x2apic_affinity,
> +};
> +
>   static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[])
>   {
>   u16 domain_id, device_id, flags;
> @@ -726,8 +764,6 @@ static void iommu_interrupt_handler(int
>   static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
>   {
>   int irq, ret;
> -hw_irq_controller *handler;
> -u16 control;
>   
>   irq = create_irq(NUMA_NO_NODE);
>   if ( irq <= 0 )
> @@ -747,20 +783,43 @@ static bool_t __init set_iommu_interrupt
>   PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf));
>   return 0;
>   }
> -control = pci_conf_read16(iommu->seg, PCI_BUS(iommu->bdf),
> -  PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf),
> -  iommu->msi.msi_attrib.pos + PCI_MSI_FLAGS);
> -iommu->msi.msi.nvec = 1;
> -if ( is_mask_bit_support(control) )
> -{
> -iommu->msi.msi_attrib.maskbit = 1;
> -iommu->msi.msi.mpos = msi_mask_bits_reg(iommu->msi.msi_attrib.pos,
> -is_64bit_address(control));
> -handler = &iommu_maskable_msi_type;
> +
> +if ( iommu->ctrl.int_cap_xt_en )
> +{
> +struct irq_desc *desc = irq_to_desc(irq);
> +
> +iommu->msi.msi_attrib.pos = MSI_TYPE_IOMMU;
> +iommu->msi.msi_attrib.maskbit = 0;
> +iommu->msi.msi_attrib.is_64 = 1;
> +
> +desc->msi_desc = &iommu->msi;
> +desc->handler = &iommu_x2apic_type;
> +
> +ret = 0;
>   }
>   else
> -handler = &iommu_msi_type;
> -ret = __setup_msi_irq(irq_to_desc(irq), &iommu->msi, handler);
> +{
> +hw_irq_controller *handler;
> +u16 control;
> +
> +control = pci_conf_read16(iommu->seg, PCI_BUS(iommu->bdf),
> +  PCI_SLOT(iommu->bdf), PCI_FUNC(iommu->bdf),
> +  iommu->msi.msi_attrib.pos + PCI_MSI_FLAGS);
> +
> +iommu->msi.msi.nvec = 1;
> +if ( is_mask_bit_support(control) )
> +{
> +iommu->msi.msi_attrib.maskbit = 1;
> +iommu->msi.msi.mpos = 
> msi_mask_bits_reg(iommu->msi.msi_attrib.pos,
> +
> is_64bit_address(control));
> +handler = &iommu_maskable_msi_type;
> +}
> +else
> +handler = &iommu_msi_type;
> +
> +ret = __setup_msi_irq(irq_to_desc(irq), &iommu->msi, handler);
> +}
> +
>   if ( !ret )
>   ret = request_irq(irq, 0, iommu_interrupt_handler, "amd_iommu", 
> iommu);
>   if ( ret )
> @@ -838,8 +897,19 @@ static void enable_iommu(struct amd_iomm
>   struct irq_desc *desc = irq_to_desc(iommu->msi.irq);
>   
>   spin_lock(&desc->lock);
> -set_msi_affinity(desc, NULL);
> -spin_unlock(&desc->lock);
> +
>

Re: [Xen-devel] [PATCH v3 10/14] AMD/IOMMU: allow enabling with IRQ not yet set up

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:39:34PM +, Jan Beulich wrote:
> Early enabling (to enter x2APIC mode) requires deferring of the IRQ
> setup. Code to actually do that setup in the x2APIC case will get added
> subsequently.
> 
> Signed-off-by: Jan Beulich 
> Acked-by: Andrew Cooper 

Acked-by: Brian Woods 

> ---
> v3: Re-base.
> 
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -814,7 +814,6 @@ static void amd_iommu_erratum_746_workar
>   static void enable_iommu(struct amd_iommu *iommu)
>   {
>   unsigned long flags;
> -struct irq_desc *desc;
>   
>   spin_lock_irqsave(&iommu->lock, flags);
>   
> @@ -834,19 +833,27 @@ static void enable_iommu(struct amd_iomm
>   if ( iommu->features.flds.ppr_sup )
>   register_iommu_ppr_log_in_mmio_space(iommu);
>   
> -desc = irq_to_desc(iommu->msi.irq);
> -spin_lock(&desc->lock);
> -set_msi_affinity(desc, NULL);
> -spin_unlock(&desc->lock);
> +if ( iommu->msi.irq > 0 )
> +{
> +struct irq_desc *desc = irq_to_desc(iommu->msi.irq);
> +
> +spin_lock(&desc->lock);
> +set_msi_affinity(desc, NULL);
> +spin_unlock(&desc->lock);
> +}
>   
>   amd_iommu_msi_enable(iommu, IOMMU_CONTROL_ENABLED);
>   
>   set_iommu_ht_flags(iommu);
>   set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_ENABLED);
> -set_iommu_event_log_control(iommu, IOMMU_CONTROL_ENABLED);
>   
> -if ( iommu->features.flds.ppr_sup )
> -set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_ENABLED);
> +if ( iommu->msi.irq > 0 )
> +{
> +set_iommu_event_log_control(iommu, IOMMU_CONTROL_ENABLED);
> +
> +if ( iommu->features.flds.ppr_sup )
> +set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_ENABLED);
> +}
>   
>   if ( iommu->features.flds.gt_sup )
>   set_iommu_guest_translation_control(iommu, IOMMU_CONTROL_ENABLED);
> 

-- 
Brian Woods

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 09/14] AMD/IOMMU: split amd_iommu_init_one()

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:39:10PM +, Jan Beulich wrote:
> Mapping the MMIO space and obtaining feature information needs to happen
> slightly earlier, such that for x2APIC support we can set XTEn prior to
> calling amd_iommu_update_ivrs_mapping_acpi() and
> amd_iommu_setup_ioapic_remapping().
> 
> Signed-off-by: Jan Beulich 
> Reviewed-by: Andrew Cooper 

Acked-by: Brian Woods 

> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -970,14 +970,6 @@ static void * __init allocate_ppr_log(st
>   
>   static int __init amd_iommu_init_one(struct amd_iommu *iommu)
>   {
> -if ( map_iommu_mmio_region(iommu) != 0 )
> -goto error_out;
> -
> -get_iommu_features(iommu);
> -
> -if ( iommu->features.raw )
> -iommuv2_enabled = 1;
> -
>   if ( allocate_cmd_buffer(iommu) == NULL )
>   goto error_out;
>   
> @@ -1202,6 +1194,23 @@ static bool_t __init amd_sp5100_erratum2
>   return 0;
>   }
>   
> +static int __init amd_iommu_prepare_one(struct amd_iommu *iommu)
> +{
> +int rc = alloc_ivrs_mappings(iommu->seg);
> +
> +if ( !rc )
> +rc = map_iommu_mmio_region(iommu);
> +if ( rc )
> +return rc;
> +
> +get_iommu_features(iommu);
> +
> +if ( iommu->features.raw )
> +iommuv2_enabled = true;
> +
> +return 0;
> +}
> +
>   int __init amd_iommu_init(void)
>   {
>   struct amd_iommu *iommu;
> @@ -1232,7 +1241,7 @@ int __init amd_iommu_init(void)
>   radix_tree_init(&ivrs_maps);
>   for_each_amd_iommu ( iommu )
>   {
> -rc = alloc_ivrs_mappings(iommu->seg);
> +rc = amd_iommu_prepare_one(iommu);
>   if ( rc )
>   goto error_out;
>   }
> 

-- 
Brian Woods

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/14] AMD/IOMMU: pass IOMMU to {get, free, update}_intremap_entry()

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:37:51PM +, Jan Beulich wrote:
> The functions will want to know IOMMU properties (specifically the IRTE
> size) subsequently.
> 
> Rather than introducing a second error path bogusly returning -E... from
> amd_iommu_read_ioapic_from_ire(), also change the existing one to follow
> VT-d in returning the raw (untranslated) IO-APIC RTE.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: New.
> 
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -123,11 +123,11 @@ static unsigned int alloc_intremap_entry
>   return slot;
>   }
>   
> -static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf,
> - unsigned int index)
> +static union irte_ptr get_intremap_entry(const struct amd_iommu *iommu,
> + unsigned int bdf, unsigned int 
> index)
>   {
>   union irte_ptr table = {
> -.ptr = get_ivrs_mappings(seg)[bdf].intremap_table
> +.ptr = get_ivrs_mappings(iommu->seg)[bdf].intremap_table
>   };
>   
>   ASSERT(table.ptr && (index < INTREMAP_ENTRIES));
> @@ -137,18 +137,19 @@ static union irte_ptr get_intremap_entry
>   return table;
>   }
>   
> -static void free_intremap_entry(unsigned int seg, unsigned int bdf,
> -unsigned int index)
> +static void free_intremap_entry(const struct amd_iommu *iommu,
> +unsigned int bdf, unsigned int index)
>   {
> -union irte_ptr entry = get_intremap_entry(seg, bdf, index);
> +union irte_ptr entry = get_intremap_entry(iommu, bdf, index);
>   
>   ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
>   
> -__clear_bit(index, get_ivrs_mappings(seg)[bdf].intremap_inuse);
> +__clear_bit(index, get_ivrs_mappings(iommu->seg)[bdf].intremap_inuse);
>   }
>   
> -static void update_intremap_entry(union irte_ptr entry, unsigned int vector,
> -  unsigned int int_type,
> +static void update_intremap_entry(const struct amd_iommu *iommu,
> +  union irte_ptr entry,
> +  unsigned int vector, unsigned int int_type,
> unsigned int dest_mode, unsigned int dest)
>   {
>   struct irte_basic basic = {
> @@ -212,7 +213,7 @@ static int update_intremap_entry_from_io
>   lo_update = 1;
>   }
>   
> -entry = get_intremap_entry(iommu->seg, req_id, offset);
> +entry = get_intremap_entry(iommu, req_id, offset);
>   if ( !lo_update )
>   {
>   /*
> @@ -223,7 +224,7 @@ static int update_intremap_entry_from_io
>   vector = entry.ptr32->basic.vector;
>   delivery_mode = entry.ptr32->basic.int_type;
>   }
> -update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
> +update_intremap_entry(iommu, entry, vector, delivery_mode, dest_mode, 
> dest);
>   
>   spin_unlock_irqrestore(lock, flags);
>   
> @@ -288,8 +289,8 @@ int __init amd_iommu_setup_ioapic_remapp
>   spin_lock_irqsave(lock, flags);
>   offset = alloc_intremap_entry(seg, req_id, 1);
>   BUG_ON(offset >= INTREMAP_ENTRIES);
> -entry = get_intremap_entry(iommu->seg, req_id, offset);
> -update_intremap_entry(entry, vector,
> +entry = get_intremap_entry(iommu, req_id, offset);
> +update_intremap_entry(iommu, entry, vector,
> delivery_mode, dest_mode, dest);
>   spin_unlock_irqrestore(lock, flags);
>   
> @@ -413,7 +414,7 @@ unsigned int amd_iommu_read_ioapic_from_
>   
>   idx = ioapic_id_to_index(IO_APIC_ID(apic));
>   if ( idx == MAX_IO_APICS )
> -return -EINVAL;
> +return val;
>   
>   offset = ioapic_sbdf[idx].pin_2_idx[pin];
>   
> @@ -422,9 +423,13 @@ unsigned int amd_iommu_read_ioapic_from_
>   u16 bdf = ioapic_sbdf[idx].bdf;
>   u16 seg = ioapic_sbdf[idx].seg;
>   u16 req_id = get_intremap_requestor_id(seg, bdf);
> -union irte_ptr entry = get_intremap_entry(seg, req_id, offset);
> +const struct amd_iommu *iommu = find_iommu_for_device(seg, bdf);
> +union irte_ptr entry;
>   
> +if ( !iommu )
> +return val;
>   ASSERT(offset == (val & (INTREMAP_ENTRIES - 1)));
> +entry = get_intremap_entry(iommu, req_id, offset);
>   val &= ~(INTREMAP_ENTRIES - 1);
>   val |= MASK_INSR(entry.ptr32->basic.int_type,
>IO_APIC_REDIR_DELIV_MODE_MASK);
> @@ -454,7 +459,7 @@ static int update_intremap_entry_from_ms
>   lock = get_intremap_lock(iommu->seg, req_id);
>   spin_lock_irqsave(lock, flags);
>   for ( i = 0; i < nr; ++i )
> -free_intremap_entry(iommu->seg, req_id, *remap_index + i);
> +free_intremap_entry(iommu, req_id, *remap_inde

Re: [Xen-devel] [PATCH 60/60] xen/sched: add scheduling granularity enum

[Dario Faggioli]

On Tue, 2019-05-28 at 12:33 +0200, Juergen Gross wrote:
> Add a scheduling granularity enum ("cpu", "core", "socket") for
> specification of the scheduling granularity. Initially it is set to
> "cpu", this can be modified by the new boot parameter (x86 only)
> "sched-gran".
> 
> According to the selected granularity sched_granularity is set after
> all cpus are online.
> 
> A test is added for all sched resources holding the same number of
> cpus. Fall back to core- or cpu-scheduling in that case.
>
Might be me (non-native speaker), but this reads weird.

Perhaps the second sentence should have ended with "is that _is_not_
the case" ?

As in, "if it is not the case that all sched resources hold the same
number of cpus, we fallback". While right now it seems to me to say
that we fall back in the case that the resources hold the same number
of cpus...

> Signed-off-by: Juergen Gross 
>
With the above clarified:

Reviewed-by: Dario Faggioli 

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf test] 139157: all pass - PUSHED

[osstest service owner]

flight 139157 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139157/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 296c908c6968910ea7c4496b94cfba1e52212de2
baseline version:
 ovmf 3dafa0382286f00d5969b9471d4a0ab362beab11

Last test of basis   139118  2019-07-18 12:57:46 Z1 days
Testing same since   139157  2019-07-19 09:08:59 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  David Woodhouse 
  Hao A Wu 
  Julien Grall 
  Laszlo Ersek 
  Leif Lindholm 
  Philippe Mathieu-Daude 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   3dafa03822..296c908c69  296c908c6968910ea7c4496b94cfba1e52212de2 -> 
xen-tested-master

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 06/14] AMD/IOMMU: pass IOMMU to amd_iommu_alloc_intremap_table()

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:37:26PM +, Jan Beulich wrote:
> The function will want to know IOMMU properties (specifically the IRTE
> size) subsequently.
> 
> Correct indentation of one of the call sites at this occasion.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: New.
> 
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -74,12 +74,14 @@ static void __init add_ivrs_mapping_entr
>/* allocate per-device interrupt remapping table */
>if ( amd_iommu_perdev_intremap )
>ivrs_mappings[alias_id].intremap_table =
> -amd_iommu_alloc_intremap_table(
> -&ivrs_mappings[alias_id].intremap_inuse);
> + amd_iommu_alloc_intremap_table(
> + iommu,
> + &ivrs_mappings[alias_id].intremap_inuse);
>else
>{
>if ( shared_intremap_table == NULL  )
>shared_intremap_table = amd_iommu_alloc_intremap_table(
> + iommu,
>&shared_intremap_inuse);
>ivrs_mappings[alias_id].intremap_table = shared_intremap_table;
>ivrs_mappings[alias_id].intremap_inuse = shared_intremap_inuse;
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -632,7 +632,8 @@ int __init amd_iommu_free_intremap_table
>   return 0;
>   }
>   
> -void* __init amd_iommu_alloc_intremap_table(unsigned long **inuse_map)
> +void *__init amd_iommu_alloc_intremap_table(
> +const struct amd_iommu *iommu, unsigned long **inuse_map)
>   {
>   void *tb;
>   tb = __alloc_amd_iommu_tables(INTREMAP_TABLE_ORDER);
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -97,7 +97,8 @@ struct amd_iommu *find_iommu_for_device(
>   
>   /* interrupt remapping */
>   int amd_iommu_setup_ioapic_remapping(void);
> -void *amd_iommu_alloc_intremap_table(unsigned long **);
> +void *amd_iommu_alloc_intremap_table(
> +const struct amd_iommu *, unsigned long **);
>   int amd_iommu_free_intremap_table(
>   const struct amd_iommu *, struct ivrs_mappings *);
>   void amd_iommu_ioapic_update_ire(
> 

-- 
Brian Woods

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 05/14] AMD/IOMMU: pass IOMMU to iterate_ivrs_entries() callback

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:37:04PM +, Jan Beulich wrote:
> Both users will want to know IOMMU properties (specifically the IRTE
> size) subsequently. Leverage this to avoid pointless calls to the
> callback when IVRS mapping table entries are unpopulated. To avoid
> leaking interrupt remapping tables (bogusly) allocated for IOMMUs
> themselves, this requires suppressing their allocation in the first
> place, taking a step further what commit 757122c0cf ('AMD/IOMMU: don't
> "add" IOMMUs') had done.
> 
> Additionally suppress the call for alias entries, as again both users
> don't care about these anyway. In fact this eliminates a fair bit of
> redundancy from dump output.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: New.
> ---
> TBD: Along the lines of avoiding the IRT allocation for the IOMMUs, is
>   there a way to recognize the many CPU-provided devices many of
>   which can't generate interrupts anyway, and avoid allocations for
>   them as well? It's 32k per device, after all. Another option might
>   be on-demand allocation of the tables, but quite possibly we'd get
>   into trouble with error handling there.
> 
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -65,7 +65,11 @@ static void __init add_ivrs_mapping_entr
>   /* override flags for range of devices */
>   ivrs_mappings[bdf].device_flags = flags;
>   
> -if (ivrs_mappings[alias_id].intremap_table == NULL )
> +/* Don't map an IOMMU by itself. */
> +if ( iommu->bdf == bdf )
> +return;
> +
> +if ( !ivrs_mappings[alias_id].intremap_table )
>   {
>/* allocate per-device interrupt remapping table */
>if ( amd_iommu_perdev_intremap )
> @@ -81,8 +85,9 @@ static void __init add_ivrs_mapping_entr
>ivrs_mappings[alias_id].intremap_inuse = shared_intremap_inuse;
>}
>   }
> -/* Assign IOMMU hardware, but don't map an IOMMU by itself. */
> -ivrs_mappings[bdf].iommu = iommu->bdf != bdf ? iommu : NULL;
> +
> +/* Assign IOMMU hardware. */
> +ivrs_mappings[bdf].iommu = iommu;
>   }
>   
>   static struct amd_iommu * __init find_iommu_from_bdf_cap(
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1069,7 +1069,8 @@ int iterate_ivrs_mappings(int (*handler)
>   return rc;
>   }
>   
> -int iterate_ivrs_entries(int (*handler)(u16 seg, struct ivrs_mappings *))
> +int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *,
> +struct ivrs_mappings *))
>   {
>   u16 seg = 0;
>   int rc = 0;
> @@ -1082,7 +1083,12 @@ int iterate_ivrs_entries(int (*handler)(
>   break;
>   seg = IVRS_MAPPINGS_SEG(map);
>   for ( bdf = 0; !rc && bdf < ivrs_bdf_entries; ++bdf )
> -rc = handler(seg, map + bdf);
> +{
> +const struct amd_iommu *iommu = map[bdf].iommu;
> +
> +if ( iommu && map[bdf].dte_requestor_id == bdf )
> +rc = handler(iommu, &map[bdf]);
> +}
>   } while ( !rc && ++seg );
>   
>   return rc;
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -617,7 +617,7 @@ void amd_iommu_read_msi_from_ire(
>   }
>   
>   int __init amd_iommu_free_intremap_table(
> -u16 seg, struct ivrs_mappings *ivrs_mapping)
> +const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping)
>   {
>   void *tb = ivrs_mapping->intremap_table;
>   
> @@ -693,14 +693,15 @@ static void dump_intremap_table(const u3
>   }
>   }
>   
> -static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)
> +static int dump_intremap_mapping(const struct amd_iommu *iommu,
> + struct ivrs_mappings *ivrs_mapping)
>   {
>   unsigned long flags;
>   
>   if ( !ivrs_mapping )
>   return 0;
>   
> -printk("  %04x:%02x:%02x:%u:\n", seg,
> +printk("  %04x:%02x:%02x:%u:\n", iommu->seg,
>  PCI_BUS(ivrs_mapping->dte_requestor_id),
>  PCI_SLOT(ivrs_mapping->dte_requestor_id),
>  PCI_FUNC(ivrs_mapping->dte_requestor_id));
> --- a/xen/include/asm-x86/amd-iommu.h
> +++ b/xen/include/asm-x86/amd-iommu.h
> @@ -129,7 +129,8 @@ extern u8 ivhd_type;
>   
>   struct ivrs_mappings *get_ivrs_mappings(u16 seg);
>   int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
> -int iterate_ivrs_entries(int (*)(u16 seg, struct ivrs_mappings *));
> +int iterate_ivrs_entries(int (*)(const struct amd_iommu *,
> + struct ivrs_mappings *));
>   
>   /* iommu tables in guest space */
>   struct mmio_reg {
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -98,7 +98,8 @@ struct amd_iommu *find_iommu_for_device(
>   /* interrupt remapping */
>   int amd_iomm

Re: [Xen-devel] [PATCH v3 04/14] AMD/IOMMU: use bit field for IRTE

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:36:34PM +, Jan Beulich wrote:
> At the same time restrict its scope to just the single source file
> actually using it, and abstract accesses by introducing a union of
> pointers. (A union of the actual table entries is not used to make it
> impossible to [wrongly, once the 128-bit form gets added] perform
> pointer arithmetic / array accesses on derived types.)
> 
> Also move away from updating the entries piecemeal: Construct a full new
> entry, and write it out.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: Switch boolean bitfields to bool.
> v2: name {get,free}_intremap_entry()'s last parameter "index" instead of
>  "offset". Introduce union irte32.
> 
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -23,6 +23,28 @@
>   #include 
>   #include 
>   
> +struct irte_basic {
> +bool remap_en:1;
> +bool sup_io_pf:1;
> +unsigned int int_type:3;
> +bool rq_eoi:1;
> +bool dm:1;
> +bool guest_mode:1; /* MBZ */
> +unsigned int dest:8;
> +unsigned int vector:8;
> +unsigned int :8;
> +};
> +
> +union irte32 {
> +uint32_t raw[1];
> +struct irte_basic basic;
> +};
> +
> +union irte_ptr {
> +void *ptr;
> +union irte32 *ptr32;
> +};
> +
>   #define INTREMAP_TABLE_ORDER1
>   #define INTREMAP_LENGTH 0xB
>   #define INTREMAP_ENTRIES (1 << INTREMAP_LENGTH)
> @@ -101,47 +123,44 @@ static unsigned int alloc_intremap_entry
>   return slot;
>   }
>   
> -static u32 *get_intremap_entry(int seg, int bdf, int offset)
> +static union irte_ptr get_intremap_entry(unsigned int seg, unsigned int bdf,
> + unsigned int index)
>   {
> -u32 *table = get_ivrs_mappings(seg)[bdf].intremap_table;
> +union irte_ptr table = {
> +.ptr = get_ivrs_mappings(seg)[bdf].intremap_table
> +};
> +
> +ASSERT(table.ptr && (index < INTREMAP_ENTRIES));
>   
> -ASSERT( (table != NULL) && (offset < INTREMAP_ENTRIES) );
> +table.ptr32 += index;
>   
> -return table + offset;
> +return table;
>   }
>   
> -static void free_intremap_entry(int seg, int bdf, int offset)
> -{
> -u32 *entry = get_intremap_entry(seg, bdf, offset);
> -
> -memset(entry, 0, sizeof(u32));
> -__clear_bit(offset, get_ivrs_mappings(seg)[bdf].intremap_inuse);
> -}
> -
> -static void update_intremap_entry(u32* entry, u8 vector, u8 int_type,
> -u8 dest_mode, u8 dest)
> -{
> -set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, 0,
> -INT_REMAP_ENTRY_REMAPEN_MASK,
> -INT_REMAP_ENTRY_REMAPEN_SHIFT, entry);
> -set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry,
> -INT_REMAP_ENTRY_SUPIOPF_MASK,
> -INT_REMAP_ENTRY_SUPIOPF_SHIFT, entry);
> -set_field_in_reg_u32(int_type, *entry,
> -INT_REMAP_ENTRY_INTTYPE_MASK,
> -INT_REMAP_ENTRY_INTTYPE_SHIFT, entry);
> -set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *entry,
> -INT_REMAP_ENTRY_REQEOI_MASK,
> -INT_REMAP_ENTRY_REQEOI_SHIFT, entry);
> -set_field_in_reg_u32((u32)dest_mode, *entry,
> -INT_REMAP_ENTRY_DM_MASK,
> -INT_REMAP_ENTRY_DM_SHIFT, entry);
> -set_field_in_reg_u32((u32)dest, *entry,
> -INT_REMAP_ENTRY_DEST_MAST,
> -INT_REMAP_ENTRY_DEST_SHIFT, entry);
> -set_field_in_reg_u32((u32)vector, *entry,
> -INT_REMAP_ENTRY_VECTOR_MASK,
> -INT_REMAP_ENTRY_VECTOR_SHIFT, entry);
> +static void free_intremap_entry(unsigned int seg, unsigned int bdf,
> +unsigned int index)
> +{
> +union irte_ptr entry = get_intremap_entry(seg, bdf, index);
> +
> +ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
> +
> +__clear_bit(index, get_ivrs_mappings(seg)[bdf].intremap_inuse);
> +}
> +
> +static void update_intremap_entry(union irte_ptr entry, unsigned int vector,
> +  unsigned int int_type,
> +  unsigned int dest_mode, unsigned int dest)
> +{
> +struct irte_basic basic = {
> +.remap_en = true,
> +.int_type = int_type,
> +.dm = dest_mode,
> +.dest = dest,
> +.vector = vector,
> +};
> +
> +ACCESS_ONCE(entry.ptr32->raw[0]) =
> +container_of(&basic, union irte32, basic)->raw[0];
>   }
>   
>   static inline int get_rte_index(const struct IO_APIC_route_entry *rte)
> @@ -163,7 +182,7 @@ static int update_intremap_entry_from_io
>   u16 *index)
>   {
>   unsigned long flags;
> -u32* entry;
> +union irte_ptr entry;
>   u8 delivery_mode, dest, vector, dest_mode;
>   int req_id;
>   spinlock_t *lock;
> @@ -201,12 +220,8 @@ static int updat

Re: [Xen-devel] [PATCH v3 03/14] AMD/IOMMU: use bit field for control register

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:36:06PM +, Jan Beulich wrote:
> Also introduce a field in struct amd_iommu caching the most recently
> written control register. All writes should now happen exclusively from
> that cached value, such that it is guaranteed to be up to date.
> 
> Take the opportunity and add further fields. Also convert a few boolean
> function parameters to bool, such that use of !! can be avoided.
> 
> Because of there now being definitions beyond bit 31, writel() also gets
> replaced by writeq() when updating hardware.
> 
> Signed-off-by: Jan Beulich 
> Acked-by: Andrew Cooper 

Acked-by: Brian Woods 

> ---
> v3: Switch boolean bitfields to bool.
> v2: Add domain_id_pne field. Mention writel() -> writeq() change.
> 
> --- a/xen/drivers/passthrough/amd/iommu_guest.c
> +++ b/xen/drivers/passthrough/amd/iommu_guest.c
> @@ -317,7 +317,7 @@ static int do_invalidate_iotlb_pages(str
>   
>   static int do_completion_wait(struct domain *d, cmd_entry_t *cmd)
>   {
> -bool_t com_wait_int_en, com_wait_int, i, s;
> +bool com_wait_int, i, s;
>   struct guest_iommu *iommu;
>   unsigned long gfn;
>   p2m_type_t p2mt;
> @@ -354,12 +354,10 @@ static int do_completion_wait(struct dom
>   unmap_domain_page(vaddr);
>   }
>   
> -com_wait_int_en = iommu_get_bit(iommu->reg_ctrl.lo,
> -IOMMU_CONTROL_COMP_WAIT_INT_SHIFT);
>   com_wait_int = iommu_get_bit(iommu->reg_status.lo,
>IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
>   
> -if ( com_wait_int_en && com_wait_int )
> +if ( iommu->reg_ctrl.com_wait_int_en && com_wait_int )
>   guest_iommu_deliver_msi(d);
>   
>   return 0;
> @@ -521,40 +519,17 @@ static void guest_iommu_process_command(
>   return;
>   }
>   
> -static int guest_iommu_write_ctrl(struct guest_iommu *iommu, uint64_t 
> newctrl)
> +static int guest_iommu_write_ctrl(struct guest_iommu *iommu, uint64_t val)
>   {
> -bool_t cmd_en, event_en, iommu_en, ppr_en, ppr_log_en;
> -bool_t cmd_en_old, event_en_old, iommu_en_old;
> -bool_t cmd_run;
> -
> -iommu_en = iommu_get_bit(newctrl,
> - IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT);
> -iommu_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
> - IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT);
> -
> -cmd_en = iommu_get_bit(newctrl,
> -   IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT);
> -cmd_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
> -   IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT);
> -cmd_run = iommu_get_bit(iommu->reg_status.lo,
> -IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT);
> -event_en = iommu_get_bit(newctrl,
> - IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT);
> -event_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
> - IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT);
> -
> -ppr_en = iommu_get_bit(newctrl,
> -   IOMMU_CONTROL_PPR_ENABLE_SHIFT);
> -ppr_log_en = iommu_get_bit(newctrl,
> -   IOMMU_CONTROL_PPR_LOG_ENABLE_SHIFT);
> +union amd_iommu_control newctrl = { .raw = val };
>   
> -if ( iommu_en )
> +if ( newctrl.iommu_en )
>   {
>   guest_iommu_enable(iommu);
>   guest_iommu_enable_dev_table(iommu);
>   }
>   
> -if ( iommu_en && cmd_en )
> +if ( newctrl.iommu_en && newctrl.cmd_buf_en )
>   {
>   guest_iommu_enable_ring_buffer(iommu, &iommu->cmd_buffer,
>  sizeof(cmd_entry_t));
> @@ -562,7 +537,7 @@ static int guest_iommu_write_ctrl(struct
>   tasklet_schedule(&iommu->cmd_buffer_tasklet);
>   }
>   
> -if ( iommu_en && event_en )
> +if ( newctrl.iommu_en && newctrl.event_log_en )
>   {
>   guest_iommu_enable_ring_buffer(iommu, &iommu->event_log,
>  sizeof(event_entry_t));
> @@ -570,7 +545,7 @@ static int guest_iommu_write_ctrl(struct
>   guest_iommu_clear_status(iommu, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT);
>   }
>   
> -if ( iommu_en && ppr_en && ppr_log_en )
> +if ( newctrl.iommu_en && newctrl.ppr_en && newctrl.ppr_log_en )
>   {
>   guest_iommu_enable_ring_buffer(iommu, &iommu->ppr_log,
>  sizeof(ppr_entry_t));
> @@ -578,19 +553,21 @@ static int guest_iommu_write_ctrl(struct
>   guest_iommu_clear_status(iommu, 
> IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT);
>   }
>   
> -if ( iommu_en && cmd_en_old && !cmd_en )
> +if ( newctrl.iommu_en && iommu->reg_ctrl.cmd_buf_en &&
> + !newctrl.cmd_buf_en )
>   {
>   /* Disable iommu command processing */
>   tasklet_kill(&iommu->cmd_buffer_tasklet);
>   }
>   
> -if ( event_en_old && !event_en )
> +if ( iommu->reg_ctrl.event_log_en && !newctrl.event_log_en )
>  

Re: [Xen-devel] [PATCH v3 01/14] AMD/IOMMU: free more memory when cleaning up after error

[Woods, Brian]

On Tue, Jul 16, 2019 at 04:35:08PM +, Jan Beulich wrote:
> The interrupt remapping in-use bitmaps were leaked in all cases. The
> ring buffers and the mapping of the MMIO space were leaked for any IOMMU
> that hadn't been enabled yet.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Brian Woods 

> ---
> v3: New.
> 
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1070,13 +1070,12 @@ static void __init amd_iommu_init_cleanu
>   {
>   list_del(&iommu->list);
>   if ( iommu->enabled )
> -{
>   disable_iommu(iommu);
> -deallocate_ring_buffer(&iommu->cmd_buffer);
> -deallocate_ring_buffer(&iommu->event_log);
> -deallocate_ring_buffer(&iommu->ppr_log);
> -unmap_iommu_mmio_region(iommu);
> -}
> +
> +deallocate_ring_buffer(&iommu->cmd_buffer);
> +deallocate_ring_buffer(&iommu->event_log);
> +deallocate_ring_buffer(&iommu->ppr_log);
> +unmap_iommu_mmio_region(iommu);
>   xfree(iommu);
>   }
>   
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -610,6 +610,8 @@ int __init amd_iommu_free_intremap_table
>   {
>   void *tb = ivrs_mapping->intremap_table;
>   
> +XFREE(ivrs_mapping->intremap_inuse);
> +
>   if ( tb )
>   {
>   __free_amd_iommu_tables(tb, INTREMAP_TABLE_ORDER);
> 

-- 
Brian Woods

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 09/60] xen/sched: let pick_cpu return a scheduler resource

[Dario Faggioli]

On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> Instead of returning a physical cpu number let pick_cpu() return a
> scheduler resource instead. Rename pick_cpu() to pick_resource() to
> reflect that change.
> 
> Signed-off-by: Juergen Gross 
>
Reviewed-by: Dario Faggioli 

with:

> --- a/xen/include/xen/sched-if.h
> +++ b/xen/include/xen/sched-if.h
> @@ -351,14 +351,14 @@ static inline void sched_migrate(const struct
> scheduler *s,
>  else
>  {
>  unit->vcpu->processor = cpu;
> -unit->res = per_cpu(sched_res, cpu);
> +unit->res = get_sched_res(cpu);
>  }
>  }
>  
this hunk moving in patch 8, as noted while reviewing it already. :-)

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [freebsd-master test] 139159: all pass - PUSHED

[osstest service owner]

flight 139159 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139159/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 freebsd  37af220308d220ce946683e1a2e80b352fb9e856
baseline version:
 freebsd  6418500c9f9c91a698564bbc264c513461611472

Last test of basis   139084  2019-07-17 09:19:38 Z2 days
Testing same since   139159  2019-07-19 09:19:38 Z0 days1 attempts


People who touched revisions under test:
  andrew 
  asomers 
  bdrewery 
  brooks 
  cy 
  delphij 
  ian 
  imp 
  kib 
  kp 
  markj 
  mckusick 
  sjg 

jobs:
 build-amd64-freebsd-againpass
 build-amd64-freebsd  pass
 build-amd64-xen-freebsd  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/freebsd.git
   6418500c9f9..37af220308d  37af220308d220ce946683e1a2e80b352fb9e856 -> 
tested/master

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 14/14] AMD/IOMMU: process softirqs while dumping IRTs

[Andrew Cooper]

On 16/07/2019 17:41, Jan Beulich wrote:
> When there are sufficiently many devices listed in the ACPI tables (no
> matter if they actually exist), output may take way longer than the
> watchdog would like.
>
> Signed-off-by: Jan Beulich 
> ---
> v3: New.
> ---
> TBD: Seeing the volume of output I wonder whether we should further
>   suppress logging headers of devices which have no active entry
>   (i.e. emit the header only upon finding the first IRTE worth
>   logging). And while minor for the total volume of output I'm
>   also unconvinced logging both a "per device" header line and a
>   "shared" one makes sense, when only one of the two can actually
>   be followed by actual contents.

I don't have a system I can access at the moment, so can't judge how bad
it is right now.  However, I would advocate the removal of irrelevant
information.

Either way, this is debugging so Acked-by: Andrew Cooper


As an observation, I wonder whether continually sprinkling
process_pending_softirqs() is the best thing to do for keyhandlers. 
We've got a number of other which incur the wrath of the watchdog (grant
table in particular), which in practice means they are typically broken
when they are actually used for debugging production.

As these are for debugging only, might it be a better idea to stop the
watchdog while keyhandlers are running?  The only useful thing we
actually manage here is to stop the watchdog killing us.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-linus test] 139134: regressions - FAIL

[osstest service owner]

flight 139134 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139134/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 133580
 test-amd64-amd64-i386-pvgrub  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-libvirt  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-pvhv2-intel  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-qemut-win7-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-qemuu-nested-intel  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemut-debianhvm-amd64  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-qemut-win10-i386  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-pygrub   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-credit2   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-amd64-libvirt-xsm  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-ovmf-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-amd64-xl-pvhv2-amd  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-qemuu-nested-amd  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-win10-i386  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-xl-qemut-ws16-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-xl-credit1   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 133580
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-win7-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-libvirt-vhd  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64

Re: [Xen-devel] [PATCH 08/60] xen/sched: introduce struct sched_resource

[Dario Faggioli]

On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> Add a scheduling abstraction layer between physical processors and
> the
> schedulers by introducing a struct sched_resource. Each scheduler
> unit
> running is active on such a scheduler resource. For the time being
> there is one struct sched_resource per cpu, but in future there might
> be one for each core or socket only.
> 
Ah, one more thing.

> +++ b/xen/common/schedule.c
> @@ -63,6 +63,7 @@ static void poll_timer_fn(void *data);
>  /* This is global for now so that private implementations can reach
> it */
>  DEFINE_PER_CPU(struct schedule_data, schedule_data);
>  DEFINE_PER_CPU(struct scheduler *, scheduler);
> +DEFINE_PER_CPU(struct sched_resource *, sched_res);
>  
I wouldn't expect this to change too much.

Therefore, does DEFINE_PER_CPU_READ_MOSTLY() make sense?

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC v3 13/14] AMD/IOMMU: correct IRTE updating

[Andrew Cooper]

On 16/07/2019 17:40, Jan Beulich wrote:
> Flushing didn't get done along the lines of what the specification says.
> Mark entries to be updated as not remapped (which will result in
> interrupt requests to get target aborted, but the interrupts should be
> masked anyway at that point in time), issue the flush, and only then
> write the new entry.
>
> In update_intremap_entry_from_msi_msg() also fold the duplicate initial
> lock determination and acquire into just a single instance.
>
> Signed-off-by: Jan Beulich 
> ---
> RFC: Putting the flush invocations in loops isn't overly nice, but I
>   don't think this can really be abused, since callers up the stack
>   hold further locks. Nevertheless I'd like to ask for better
>   suggestions.

Looking again, and at v2, I think this is a consequence of our insane
!x2apic interrupt set up, where we wrap an already-established system
with interrupt remapping.

Longer term, when we undo that, we should have far more clear code
structure.  Therefore, I think it is fine for now.

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 08/60] xen/sched: introduce struct sched_resource

[Dario Faggioli]

On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> Add a scheduling abstraction layer between physical processors and
> the
> schedulers by introducing a struct sched_resource. Each scheduler
> unit
> running is active on such a scheduler resource. For the time being
> there is one struct sched_resource per cpu, but in future there might
> be one for each core or socket only.
> 
I only have two questions:

> @@ -1618,6 +1623,13 @@ static int cpu_schedule_up(unsigned int cpu)
>  {
>  struct schedule_data *sd = &per_cpu(schedule_data, cpu);
>  void *sched_priv;
> +struct sched_resource *res;
> +
> +res = xzalloc(struct sched_resource);
> +if ( res == NULL )
> +return -ENOMEM;
> +res->processor = cpu;
> +set_sched_res(cpu, res);
>  
>  per_cpu(scheduler, cpu) = &ops;
>  spin_lock_init(&sd->_lock);
> @@ -1682,6 +1694,9 @@ static void cpu_schedule_down(unsigned int cpu)
>  sd->sched_priv = NULL;
>  
>  kill_timer(&sd->s_timer);
> +
> +set_sched_res(cpu, NULL);
> +xfree(sd);
>  }
> 
What's that xfree(sd) about?
 

> @@ -334,7 +349,10 @@ static inline void sched_migrate(const struct
> scheduler *s,
>  if ( s->migrate )
>  s->migrate(s, unit, cpu);
>  else
> +{
>  unit->vcpu->processor = cpu;
> +unit->res = per_cpu(sched_res, cpu);
> +}
>
  unit->res = get_sched_res(cpu)

?

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface

[Petre Ovidiu PIRCALABU]

On Fri, 2019-07-19 at 12:59 +, Jan Beulich wrote:
> On 19.07.2019 14:37, Paul Durrant wrote:
> > > From: Jan Beulich 
> > > Sent: 19 July 2019 13:32
> > > 
> > > On 19.07.2019 14:11, Paul Durrant wrote:
> > > > > -Original Message-
> > > > > From: Petre Ovidiu PIRCALABU 
> > > > > Sent: 19 July 2019 12:24
> > > > > 
> > > > > Sorry, my mistake. I meant to say it's shared with MD.
> > > > > 
> > > > > Many thanks for your support,
> > > > 
> > > > Ok, in that case please share with the ID instead.
> > > 
> > > But that's exactly what we want to avoid: If sharing at all, then
> > > please with the more privileged entity.
> > 
> > Why? We're talking HVM guests only here IIUC so this is equivalent
> > to IOREQ server...
> 
> Not sure: The main vm_event.c files live in common/ and arch/x86/
> respectively, so I thought at least architecturally VM events were
> possible for PV as well. If it's indeed HVM-only, then following
> the IOREQ server model in its entirety would of course be fine.
> 
> Jan

In one of the previous version of the patchset there was a suggestion
to implement the new vm_event transport using IOREQ, but it was dropped
.

https://lists.xenproject.org/archives/html/xen-devel/2019-04/msg00173.html

Also, unless there isn't a proper way allocate the necessary pages, I
wouldn't introduce a HVM-only limitation because, other than the HVM
param used to keep track of the ring pfn, the vm_event mechanism is
quite generic.

Many thanks for your support,
Petre

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 12/14] AMD/IOMMU: enable x2APIC mode when available

[Andrew Cooper]

On 16/07/2019 17:40, Jan Beulich wrote:
> In order for the CPUs to use x2APIC mode, the IOMMU(s) first need to be
> switched into suitable state.
>
> The post-AP-bringup IRQ affinity adjustment is done also for the non-
> x2APIC case, matching what VT-d does.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 11/14] AMD/IOMMU: adjust setup of internal interrupt for x2APIC mode

[Andrew Cooper]

On 16/07/2019 17:39, Jan Beulich wrote:
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
> @@ -416,6 +416,25 @@ union amd_iommu_ext_features {
>   } flds;
>   };
>   
> +/* x2APIC Control Registers */
> +#define IOMMU_XT_INT_CTRL_MMIO_OFFSET0x0170
> +#define IOMMU_XT_PPR_INT_CTRL_MMIO_OFFSET0x0178
> +#define IOMMU_XT_GA_INT_CTRL_MMIO_OFFSET 0x0180
> +
> +union amd_iommu_x2apic_control {
> +uint64_t raw;
> +struct {
> +unsigned int :2;
> +unsigned int dest_mode:1;
> +unsigned int :5;
> +unsigned int dest_lo:24;
> +unsigned int vector:8;
> +unsigned int int_type:1; /* DM in IOMMU spec 3.04 */
> +unsigned int :15;
> +unsigned int dest_hi:8;

Bool bitfields like you've done elsewhere in v3?

My pre-existing R-by stands, but ideally with this chagned.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 08/14] AMD/IOMMU: introduce 128-bit IRTE non-guest-APIC IRTE format

[Andrew Cooper]

On 16/07/2019 17:38, Jan Beulich wrote:
> This is in preparation of actually enabling x2APIC mode, which requires
> this wider IRTE format to be used.
>
> A specific remark regarding the first hunk changing
> amd_iommu_ioapic_update_ire(): This bypass was introduced for XSA-36,
> i.e. by 94d4a1119d ("AMD,IOMMU: Clean up old entries in remapping
> tables when creating new one"). Other code introduced by that change has
> meanwhile disappeared or further changed, and I wonder if - rather than
> adding an x2apic_enabled check to the conditional - the bypass couldn't
> be deleted altogether. For now the goal is to affect the non-x2APIC
> paths as little as possible.

There are plenty of mistakes with XSA-36.  Reading the XSA back, the
MITIGATION section gets the sense of the iommu=amd-iommu-perdev-intremap
boolean the wrong way around.  Oh well...

SP5100 erratum 28 only requires that the IDE and SATA devices share
tables, not that every device on the whole system shares tables.

With the proposed work to perform IOMMU assignment by group rather than
individually, this will naturally fall out as a quirk requiring the two
devices to be grouped, at which point we can drop all remnants of global
remapping tables.

In this case, I'm not sure it is worth caring about shared-table mode on
x2apic-capable systems.  0 people will be using that mode.  That said,
if its easier to wait until the IOMMU changes to make this adjustment,
then fine.

> @@ -142,7 +178,15 @@ static void free_intremap_entry(const st
>   {
>   union irte_ptr entry = get_intremap_entry(iommu, bdf, index);
>   
> -ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
> +if ( iommu->ctrl.ga_en )
> +{
> +ACCESS_ONCE(entry.ptr128->raw[0]) = 0;
> +/* Low half (containing RemapEn) needs to be cleared first. */
> +barrier();

While this will function on x86, I still consider this buggy.  From a
conceptual point of view, barrier() is not the correct construction to
use, whereas smp_wmb() is.

As this is the only remaining issue, with it fixed in each location,
Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 07/60] xen/sched: build a linked list of struct sched_unit

[Dario Faggioli]

On Fri, 2019-07-19 at 07:07 +0200, Juergen Gross wrote:
> On 19.07.19 02:01, Dario Faggioli wrote:
> > On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> > > 
> > How about a ',' between domain and build?
> 
> Okay.

> > "over all vcpus of a sched_unit" ?
> 
> Oh, of course!
> 
Thanks.

> > One question:
> > 
> > > @@ -279,8 +279,16 @@ struct vcpu
> > >   struct sched_unit {
> > >   struct vcpu   *vcpu;
> > >   void  *priv;  /* scheduler private data
> > > */
> > > +struct sched_unit *next_in_list;
> > >   };
> > >   
> > > +#define for_each_sched_unit(d,
> > > e) \
> > > +for ( (e) = (d)->sched_unit_list; (e) != NULL; (e) = (e)-
> > > > next_in_list )
> > > +
> > > +#define for_each_sched_unit_vcpu(i,
> > > v)\
> > > +for ( (v) = (i)->vcpu; (v) != NULL && (v)->sched_unit ==
> > > (i); \
> > > +  (v) = (v)->next_in_list )
> > > +
> > > 
> > So, here... sorry if it's me not seeing it, but why the
> > (v)->sched_unit == (i) check is necessary?
> > 
> > Do we expect to put in the list of vcpus of a particular unit,
> > vcpus
> > that are in another unit?
> 
> Yes. 
>
Ah!

> I'm making use of the fact that all vcpus in a unit are consecutive
> as I'm re-using the already existing list of vcpus in a domain:
> 
> dom->vcpu0->vcpu1->vcpu2->vcpu3
>^ ^
>! !
> unit0-+ !
>  !
> unit2---+
> 
Ok, I see. Can you add a short comment, above the for_each_xxx
construct, about that?

"All vcpus from all sched units are kept in the same. Only iterate over
the ones from a particular unit"

Or something like this.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 06/60] xen/sched: move per-vcpu scheduler private data pointer to sched_unit

[Dario Faggioli]

On Fri, 2019-07-19 at 07:03 +0200, Juergen Gross wrote:
> On 19.07.19 00:52, Dario Faggioli wrote:
> > On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> > > This prepares making the different schedulers vcpu agnostic.
>
> > But shouldn't then the struct be called csched2_unit, and cited
> > functions be called csched2_alloc_udata() and csched2_unit()?
> > Now, I know that these transformation happen later in the series.
> > And, this time, I'm not asking to consolidate the patches.
> > 
> > However:
> > - can the changelog of this patch be a little more explicit, not
> > only
> >informing that this is a preparatory patch, but also explaining
> >briefly the temporary inconcistency
> 
> Sure.
> 
Ok, thanks.

> > - could the patches that deal with this be grouped together, so
> > that
> >they are close to each other in the series [...]
> 
> There are some patches which could be reordered, but I'm not sure the
> needed work is worth it. By moving the patches you named closer to
> each
> other there will be other closely related patches ripped apart from
> each other.
>
Yes, I appreciate that.
> 
> In the end it will make no sense to apply only some patches of the
> main
> series. The huge amount of patches is meant only to make review
> easier.
>
And yet, this happens some times, especially for big series, and was
exactly what I was thinking to when making this comment.

Anyway, let's go with an updated changelog for now... And I'll build a
better and more solid opinion after having looked at the rest of the
series.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 05/60] xen/sched: alloc struct sched_unit for each vcpu

[Dario Faggioli]

On Fri, 2019-07-19 at 06:56 +0200, Juergen Gross wrote:
> On 18.07.19 19:57, Dario Faggioli wrote:
> > On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> > > 
> > However, I don't see much value in not doing what's done here in
> > patch
> > 4 already (it's pretty much only line changed by patch 4 that are
> > being
> > changed again here).
> > 
> > Is there a particular reason you think it's important to keep these
> > two
> > patches separated?
> 
> Not important, but I thought it would make it more clear.
> 
> If you like it better I can merge the two patches.
> 
Yes, I'd prefer them merged.

> > Ah, my comment about 'unit' --> 'su' --in case you think it's
> > feasible-
> > - applies to struct members as well, of course, e.g., here:
> > 
> I have to disagree here: this is no scheduler private structure, so I
> believe the struct member names should be descriptive. I guess this
> is
> the reason why there are many more struct members named "vcpu" than
> "v".
> 
Well, ditto in my last reply to patch 4. And in fact, in scheduling
code, the preference was for 'v'.

But again, let's settle on 'unit', on the basis that the amount of work
necessary to change this is, again, not worth it.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 04/60] xen/sched: use new sched_unit instead of vcpu in scheduler interfaces

[Dario Faggioli]

On Fri, 2019-07-19 at 06:49 +0200, Juergen Gross wrote:
> On 18.07.19 19:44, Dario Faggioli wrote:
> > On Tue, 2019-05-28 at 12:32 +0200, Juergen Gross wrote:
> > One thing that came to mind, is that the various function
> > parameters
> > and local variables called 'unit', could be called 'su'.
> > 
> > It's a contraction of 'sched_unit', like, e.g., 'v' or 'vc' were
> > contractions of 'vcpu', it's still quite descriptive, it's short,
> > which
> > is always good, IMO, and might mean less line wrap reformatting
> > (considering that it's replacing 'v' or 'vc').
> > 
> > Of course, this will likely mean changing all the other ~60
> > patches, so
> > I'll understand if you say that it would be too much.
> 
> I prefer "unit", as it is more readable and the effort to do the
> change
> would be quite large (replacing "item" by "unit" was doable via sed,
> while this change would require more manual intervention).
> 
I guess what'd be more readable it's to some big extent a matter of
personal taste. However, I trust you about the amount of work needed
for doing such a rework, so I won't insist on it.

Reviewed-by: Dario Faggioli 

And this stands even with this:

> > > index 2201faca6b..72a17758a1 100644
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -275,6 +275,10 @@ struct vcpu
> > >   struct arch_vcpu arch;
> > >   };
> > >   
> > > +struct sched_unit {
> > > +struct vcpu   *vcpu;
> > > +};
> > > +
> > > 
> > Is my understanding correct that this field is going to be renamed
> > vcpu_list, right from this patch?
> 
> Yes.
> 
Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [qemu-mainline test] 139140: FAIL

[osstest service owner]

flight 139140 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139140/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-arndale  broken  in 139110

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale  4 host-install(4) broken in 139110 pass in 139140
 test-amd64-i386-xl-qemuu-ovmf-amd64 18 guest-start/debianhvm.repeat fail in 
139110 pass in 139140
 test-armhf-armhf-libvirt 19 leak-check/check fail in 139110 pass in 139140
 test-armhf-armhf-xl-vhd  18 leak-check/check fail in 139110 pass in 139140
 test-amd64-amd64-pygrub   7 xen-boot   fail pass in 139110

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 138977
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 138977
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 138977
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 138977
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 138977
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu0b18cfb8f1828c905139b54c8644b0d8f4aad879
baseline version:
 qemuu1316b1ddc8a05e418c8134243f8bff8cccbbccb1

Last test of basis   138977  2019-07-14 03:43:52 Z5 da

Re: [Xen-devel] [PATCH 2/2] CODING_STYLE: list further brace placement exceptions

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich writes:

> For easy spotting of struct/union/enum definitions we already commonly
> place the opening braces on the initial line of such a definition.
>
> We also often don't place the opening brace of an initializer on a
> separate line.
>
> And finally for compound literals placing the braces on separate lines
> often makes the code more difficult to read, so it should (and in
> practice does) typically go on the same line as well.  The placement of
> the closing brace often depends on how large such a compound literal is.
>
> Signed-off-by: Jan Beulich 
> ---
> TBD: We may want to make explicit that for initializers both forms are
>   fine.
>
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -64,8 +64,13 @@ Bracing
>   ---
>
>   Braces ('{' and '}') are usually placed on a line of their own, except
> -for the do/while loop.  This is unlike the Linux coding style and
> -unlike K&R.  do/while loops are an exception. e.g.:
> +for
> +- the do/while loop
> +- the opening brace in definitions of enum, struct, and union
> +- the opening brace in initializers
> +- compound literals
Looks like this leaves us only with "if/else", "for", "switch" and
various forms of "for_each_*". So maybe it is worth to rewrite this
in the opposite manner? Like this:

Braces ('{' and '}') are usually placed on the same line, except the
following cases:

 - if, else, for, switch statements
 - for_each_* iterators like for_each_vcpu

> +This is unlike the Linux coding style and unlike K&R.  do/while loops
There is extra space before "do/while".

> +are one exception. e.g.:
>
>   if ( condition )
>   {
>


--
Volodymyr Babchuk at EPAM
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/14] AMD/IOMMU: pass IOMMU to {get, free, update}_intremap_entry()

[Andrew Cooper]

On 16/07/2019 17:37, Jan Beulich wrote:
> The functions will want to know IOMMU properties (specifically the IRTE
> size) subsequently.
>
> Rather than introducing a second error path bogusly returning -E... from
> amd_iommu_read_ioapic_from_ire(), also change the existing one to follow
> VT-d in returning the raw (untranslated) IO-APIC RTE.

I'm not convinced that this is any less bogus.  The caller still can't
figure out if an error occurred.

Still, consistency with VT-d is less bad overall.

>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [edk2-devel] [PATCH v3 00/35] Specific platform to run OVMF in Xen PVH and HVM guests

[Anthony PERARD]

On Fri, Jul 05, 2019 at 02:21:13PM +0200, Laszlo Ersek wrote:
> The patches on the list are malformed. They have
> 
> Content-Transfer-Encoding: quoted-printable
> 
> which is fine, in itself; however, they have CR-CR-LF line terminators.
> 
> For example, from the first patch:
> 
> diff --git a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf b/OvmfPkg/L=
> ibrary/ResetSystemLib/ResetSystemLib.inf
> index 7c44f99a5c..2f24dac87f 100644
> --- a/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> +++ b/OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf
> @@ -30,4 +30,5 @@ [Packages]
>  [LibraryClasses]=0D=0D
>DebugLib=0D=0D
>IoLib=0D=0D
> +  PciLib=0D=0D
>TimerLib=0D=0D
> 
> Note "=0D=0D".
> 
> Now, if I try to apply this full set with git-am like that, the first
> patch in the series applies, but the second still fails:
> 
> > error: corrupt patch at line 23
> > Patch failed at 0002 OvmfPkg: Create platform OvmfXen
> 
> Based on the email headers, the "iphmx.com" references suggest (via a
> google search) "Cisco's Ironport Cloud email service".
> 
> I think that email service (MTA) is broken.
> 
> If you could use a different MTA (or get the current one fixed), that
> would be helpful. (Yes, yes: if the edk2 project didn't use CRLF line
> terminators, that would be *even more* helpful.)

I'm not sure that using a different MTA is going to help. I don't think
I can find a patch on the list that I can apply (without using unix2dos).
I did send a patch to my gmail address, and it looks fine (=0D in the
expected places and nowhere else). So maybe when a patch is sent through
a mailing list, some more formating is done?

Anyway, can I try sending patch encoded in base64 instead of
quoted-printable? That would probably work better.

I found <20190704040731.5303-1-g...@suse.com> on the list that is base64
encoded, that I can easily apply and patchew too.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 06/14] AMD/IOMMU: pass IOMMU to amd_iommu_alloc_intremap_table()

[Andrew Cooper]

On 16/07/2019 17:37, Jan Beulich wrote:
> The function will want to know IOMMU properties (specifically the IRTE
> size) subsequently.
>
> Correct indentation of one of the call sites at this occasion.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 05/14] AMD/IOMMU: pass IOMMU to iterate_ivrs_entries() callback

[Andrew Cooper]

On 16/07/2019 17:37, Jan Beulich wrote:
> Both users will want to know IOMMU properties (specifically the IRTE
> size) subsequently. Leverage this to avoid pointless calls to the
> callback when IVRS mapping table entries are unpopulated. To avoid
> leaking interrupt remapping tables (bogusly) allocated for IOMMUs
> themselves, this requires suppressing their allocation in the first
> place, taking a step further what commit 757122c0cf ('AMD/IOMMU: don't
> "add" IOMMUs') had done.
>
> Additionally suppress the call for alias entries, as again both users
> don't care about these anyway. In fact this eliminates a fair bit of
> redundancy from dump output.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 02/14] AMD/IOMMU: use bit field for extended feature register

[Jan Beulich]

On 19.07.2019 18:23, Andrew Cooper wrote:
> On 16/07/2019 17:35, Jan Beulich wrote:
>> This also takes care of several of the shift values wrongly having been
>> specified as hex rather than dec.
>>
>> Take the opportunity and
>> - replace a readl() pair by a single readq(),
>> - add further fields.
>>
>> Signed-off-by: Jan Beulich 
> 
> CI is happy this time around.
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/pipelines/71942193

Hurray.

> Acked-by: Andrew Cooper 

Thanks.

> but as a warning, I'm still certain that FEAT() is a fragile, and will
> be liable to break on future compilers, seeing as that seem to be the
> trend for diagnostics.

There's a certain risk, yes.

> I'm also unsure whether it works correctly on signed fields.

I'm sure it wouldn't, but I don't see any signed fields appearing
there.

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 02/14] AMD/IOMMU: use bit field for extended feature register

[Andrew Cooper]

On 16/07/2019 17:35, Jan Beulich wrote:
> This also takes care of several of the shift values wrongly having been
> specified as hex rather than dec.
>
> Take the opportunity and
> - replace a readl() pair by a single readq(),
> - add further fields.
>
> Signed-off-by: Jan Beulich 

CI is happy this time around.

https://gitlab.com/xen-project/people/andyhhp/xen/pipelines/71942193

Acked-by: Andrew Cooper 

but as a warning, I'm still certain that FEAT() is a fragile, and will
be liable to break on future compilers, seeing as that seem to be the
trend for diagnostics.

I'm also unsure whether it works correctly on signed fields.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1 5/5] xen: sched: null: refactor the ASSERTs around vcpu_deassing()

[George Dunlap]

On 8/25/18 1:22 AM, Dario Faggioli wrote:
> It is all the time that we call vcpu_deassing() that the vcpu _must_ be
> assigned to a pCPU, and hence that such pCPU can't be free.
> 
> Therefore, move the ASSERT-s which check for these properties in that
> function, where they belong better.
> ---
> Cc: George Dunlap 

This seems to missing an SoB.

With that fixed:

Reviewed-by: George Dunlap 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 04/14] AMD/IOMMU: use bit field for IRTE

[Jan Beulich]

On 19.07.2019 17:56, Andrew Cooper wrote:
> On 16/07/2019 17:36, Jan Beulich wrote:
>> At the same time restrict its scope to just the single source file
>> actually using it, and abstract accesses by introducing a union of
>> pointers. (A union of the actual table entries is not used to make it
>> impossible to [wrongly, once the 128-bit form gets added] perform
>> pointer arithmetic / array accesses on derived types.)
>>
>> Also move away from updating the entries piecemeal: Construct a full new
>> entry, and write it out.
>>
>> Signed-off-by: Jan Beulich 
> 
> I'm still not entirely convinced by extra union and containerof(), but
> the result looks correct.

And I'm still open to going the other way, if you're convinced that
in update_intremap_entry() this

 struct irte_basic basic = {
 .flds = {
 .remap_en = true,
 .int_type = int_type,
 .dm = dest_mode,
 .dest = dest,
 .vector = vector,
 }
 };

(and similarly then for the 128-bit form, and of course .flds
inserted at other use sites) is overall better than the current
variant.

> Acked-by: Andrew Cooper 

Thanks, Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [xen-unstable-smoke test] 139173: tolerable all pass - PUSHED

[osstest service owner]

flight 139173 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139173/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  66d11b9c1281ac133b92b3e6dd5b5e1c2abea7bf
baseline version:
 xen  af4acbc7a5f705417729e413f7678ae090688a1e

Last test of basis   139163  2019-07-19 10:01:05 Z0 days
Testing same since   139173  2019-07-19 13:04:22 Z0 days1 attempts


People who touched revisions under test:
  Daniel De Graaf 
  Jan Beulich 
  Razvan Cojocaru 
  Ricardo Neri 
  Tamas K Lengyel 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   af4acbc7a5..66d11b9c12  66d11b9c1281ac133b92b3e6dd5b5e1c2abea7bf -> smoke

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1 4/5] xen: sched: null: reassign vcpus to pcpus when they come back online

[George Dunlap]

On 8/25/18 1:22 AM, Dario Faggioli wrote:
> When a vcpu that was offline, comes back online, we do want it to either
> be assigned to a pCPU, or go into the wait list.
> 
> So let's do exactly that. Detecting that a vcpu is coming back online is
> a bit tricky. Basically, if the vcpu is waking up, and is neither
> assigned to a pCPU, nor in the wait list, it must be coming back from
> offline.
> 
> When this happens, we put it in the waitqueue, and we "tickle" an idle
> pCPU (if any), to go pick it up.
> 
> Looking at the patch, it seems that the vcpu wakeup code is getting
> complex, and hence that it could potentially introduce latencies.
> However, all this new logic is triggered only by the case of a vcpu
> coming online, so, basically, the overhead during normal operations is
> just an additional 'if()'.
> 
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 04/14] AMD/IOMMU: use bit field for IRTE

[Andrew Cooper]

On 16/07/2019 17:36, Jan Beulich wrote:
> At the same time restrict its scope to just the single source file
> actually using it, and abstract accesses by introducing a union of
> pointers. (A union of the actual table entries is not used to make it
> impossible to [wrongly, once the 128-bit form gets added] perform
> pointer arithmetic / array accesses on derived types.)
>
> Also move away from updating the entries piecemeal: Construct a full new
> entry, and write it out.
>
> Signed-off-by: Jan Beulich 

I'm still not entirely convinced by extra union and containerof(), but
the result looks correct.

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

[Anthony PERARD]

On Fri, Jul 19, 2019 at 03:41:52PM +0100, Andrew Cooper wrote:
> On 19/07/2019 15:33, Laszlo Ersek wrote:
> > On 07/19/19 12:20, Anthony PERARD wrote:
> >> On Fri, Jul 05, 2019 at 02:57:06PM +0100, Andrew Cooper wrote:
> >>> On 04/07/2019 15:42, Anthony PERARD wrote:
>  diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm 
>  b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>  new file mode 100644
>  index 00..958195bc5e
>  --- /dev/null
>  +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>  +vtfSignature:
>  +DB  'V', 'T', 'F', 0
>  +
>  +ALIGN   16
>  +
>  +resetVector:
>  +;
>  +; Reset Vector
>  +;
>  +; This is where the processor will begin execution
>  +;
>  +nop
>  +nop
> >>> Why two nops?
> >> I don't know, this is existing code that I duplicated to allow adding a
> >> new entry point. (I wanted to use --find-copies-harder when sending the
> >> patch, but forgot this time.
> > Not a big problem; while reviewing v3, I did such comparisons myself, in
> > my local clone. Feel free to skip "--find-copies-harder" when posting v4
> > too; I think there isn't much churn going on in parallel right now.
> >
> > However, a new request for v4: please make sure that the new modules /
> > paths introduced by this patch set are covered in Maintainers.txt.

Will do.

> >> This part of the chunk would not be there.)
> > Regarding the NOPs: all I can tell you is that they originate from
> > commit 8332983e2e33 ("UefiCpuPkg: Replace the un-necessary WBINVD
> > instruction at the reset vector with two NOPs in VTF0.", 2011-08-04).
> >
> > Whether that change made sense back then, let alone if it makes sense
> > now: no clue.
> 
> Dropping wbinvd makes sense, because when virtualised, the caches (and
> paging for that matter) are always up and running correctly.  Its an
> unnecessary vmexit for something which the hypervisor will nop out anyway.
> 
> Leaving two nops behind makes no sense at all.

I'll remove the nops.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1 3/5] xen: sched: null: deassign offline vcpus from pcpus

[Dario Faggioli]

On Thu, 2019-07-18 at 14:09 +0100, George Dunlap wrote:
> On 7/17/19 7:39 PM, Dario Faggioli wrote:
> > Point is the work of removing such vCPU from any CPU and from the
> > wait
> > list has been done already, in null_vcpu_sleep(), while the vCPU
> > was
> > going offline. So, here, we only need to make sure that we don't do
> > anything, i.e., that we don't call _vcpu_remove().
> 
> Right; I'm mainly saying, if the commit message had said what I wrote
> above, then I would  have immediately been able to see what this hunk
> was doing and understand why it was needed.
> 
Ok then, I'll improve the commit message and...

> > But I appreciate you seeing it differently, while I don't have a
> > too
> > strong opinion, so I'd be fine merging the patches (or doing other
> > series rearrangements, if you feel strongly that they're
> > necessary).
> > 
> Merging the patches would be one way to avoid the regression, yes.
> 
... I'll merge the patches.

> Sorry to be picky, but I've recently spent a lot of time doing
> archaeology, and wishing people in the distant past had been more
> careful / informative in their commit hygiene.
>
No problem at all, I see and agree on the fact that changelogs are
really important. :-)

I guess I'll wait a little, to see if you have any comments on patch 4,
and then resend.

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [libvirt test] 139147: regressions - FAIL

[osstest service owner]

flight 139147 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139147/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-libvirt-qcow2 15 guest-start/debian.repeat fail REGR. vs. 
139037

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139037
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139037
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  d524c9a893a2d8c40f6237ccb8af40c5b6cdce2f
baseline version:
 libvirt  f58bcb80b2ca1791acd5ec0255297a44aa9d4dbe

Last test of basis   139037  2019-07-16 04:18:53 Z3 days
Failing since139076  2019-07-17 04:18:47 Z2 days3 attempts
Testing same since   139147  2019-07-19 04:21:45 Z0 days1 attempts


People who touched revisions under test:
  Cole Robinson 
  Daniel Henrique Barboza 
  Julio Faracco 
  Ján Tomko 
  Michal Privoznik 
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd 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


Not pushing.

(No revision log; it would 

Re: [Xen-devel] [PATCH v3 01/14] AMD/IOMMU: free more memory when cleaning up after error

[Andrew Cooper]

On 16/07/2019 17:35, Jan Beulich wrote:
> The interrupt remapping in-use bitmaps were leaked in all cases. The
> ring buffers and the mapping of the MMIO space were leaked for any IOMMU
> that hadn't been enabled yet.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 5/5] tools/libxc: allow controlling the max C-state sub-state

[Andrew Cooper]

On 03/07/2019 14:04, Jan Beulich wrote:
> From: Ross Lagerwall 
>
> Signed-off-by: Ross Lagerwall 
>
> Make handling in do_pm_op() more homogeneous: Before interpreting
> op->cpuid as such, handle all operations not acting on a particular
> CPU. Also expose the setting via xenpm.
>
> Signed-off-by: Jan Beulich 
> Acked-by: Wei Liu 

Acked-by: Andrew Cooper , subject to fixing
the issue Roger spotted.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 4/5] x86: allow limiting the max C-state sub-state

[Andrew Cooper]

On 03/07/2019 14:03, Jan Beulich wrote:
> From: Ross Lagerwall 
>
> Allow limiting the max C-state sub-state by appending to the max_cstate
> command-line parameter. E.g. max_cstate=1,0
> The limit only applies to the highest legal C-state. For example:
>   max_cstate = 1, max_csubstate = 0 ==> C0, C1 okay, but not C1E
>   max_cstate = 1, max_csubstate = 1 ==> C0, C1 and C1E okay, but not C2
>   max_cstate = 2, max_csubstate = 0 ==> C0, C1, C1E, C2 okay, but not C3
>   max_cstate = 2, max_csubstate = 1 ==> C0, C1, C1E, C2 okay, but not C3
>
> Signed-off-by: Ross Lagerwall 
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper , subject to the
correction Roger noticed.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 3/5] x86/AMD: make C-state handling independent of Dom0

[Andrew Cooper]

On 03/07/2019 14:01, Jan Beulich wrote:
> At least for more recent CPUs, following what BKDG / PPR suggest for the
> BIOS to surface via ACPI we can make ourselves independent of Dom0
> uploading respective data.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/5] x86/cpuidle: switch to uniform meaning of "max_cstate="

[Andrew Cooper]

On 03/07/2019 13:59, Jan Beulich wrote:
> While the MWAIT idle driver already takes it to mean an actual C state,
> the ACPI idle driver so far used it as a list index. The list index,
> however, is an implementation detail of Xen and affected by firmware
> settings (i.e. not necessarily uniform for a particular system).
>
> While touching this code also avoid invoking menu_get_trace_data()
> when tracing is not active. For consistency do this also for the
> MWAIT driver.
>
> Note that I'm intentionally not adding any sorting logic to set_cx():
> Before and after this patch we assume entries to arrive in order, so
> this would be an orthogonal change.
>
> Take the opportunity and add minimal documentation for the command line
> option.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Julien Grall]

Hi Rich,

On 19/07/2019 14:50, Rich Persaud wrote:

On Jul 19, 2019, at 09:31, Julien Grall  wrote:

On 19/07/2019 14:24, Julien Grall wrote:
Hi Tamas,

On 19/07/2019 14:14, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:

Hi Tamas,


On 19/07/2019 14:00, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  wrote:

Hi Tamas,

On 18/07/2019 18:48, Tamas K Lengyel wrote:

   - Line 1025: The tools needs to be able to deal for_each_vcpu(...) & co.


These can be made OK by adding braces. Other than that the only way I
found to make it not change the indentation is to add the comment "/*
*INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.


None of them looks really appealing because it means astyle will not correctly
indent if the user does not add braces or comments.

Could astyle be easily modified to recognize foreach macros?


Not that I'm aware of. If you don't want to manually annotate files
with unsupported macros then just exclude those files from astyle. I
wouldn't recommend adding this to the CI for all files, only for those
that their respective maintainers have confirmed to conform to the
style and want to enforce it going forward.


So a couple use of an unsupported macros would make impossible to enforce the
coding style. This is not a very ideal position to be in.

_if_ we are going to adopt astyle then we need to be able to enforce it on every
Xen files long-term. If it is not possible to do it with astyle, then maybe this
is not the right tool to use.

For instance, I know that tools such as clang-format is able to deal with
foreach macros.


If there are better tools then sure, I don't really mind using
something else. I just don't have time to do the manual style check
back-and-forth anymore, so the sooner we have something in place the
better.

I totally agree we need a tool so the reviewer can free-up some time to focus 
on more important things. However, I think we should be careful on what we 
adopt here.
Similar to Andrew, I am open with modifying the coding style to help the 
automatic style check. But I am not happy to disable automatic style on part 
(or entire) of files forever.
At the moment, clang-format feels more powerful and there are people working on 
it.


FYI, below a link to the clang-format changes:

https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch


Were these clang-format changes done for FuSa work?  Are they intended to be 
run within OSStest and/or Xen's Gitlab CI, which do not currently support 
OpenEmbedded/Yocto and xen-troops?


It was originally started by EPAM to address review burden. IIRC, the goal is to 
have a robot checking coding style before any review is actually done. So 
probably part of Xen's Gitlab.


I am not sure why you mention OpenEmbedded/Yocto or even EPAM's Xen. The patch 
is against clang-format (from LLVM project) and does not have any dependencies 
on all the rest.




It would be helpful to have a xen-devel thread on the motivation for the 
clang-format work, the specific style being enforced (including the nuances 
discussed in this thread) and additional work needed before clang-format can 
perform automated style checking to address (a) existing Xen/Linux style 
requirements, (b) FuSa requirements.


I will leave that to Lars and Artem. They have been following the work more 
closely.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] x86/vLAPIC: avoid speculative out of bounds accesses

[Andrew Cooper]

On 17/07/2019 17:02, Jan Beulich wrote:
> Array indexes used in the MSR read/write emulation functions as well as
> the direct VMX / APIC-V hook are derived from guest controlled values.
> Restrict their ranges to limit the side effects of speculative
> execution.
>
> Along these lines also constrain the vlapic_lvt_mask[] access.
>
> Remove the unused vlapic_lvt_{vector,dm}() instead of adjusting them.
>
> This is part of the speculative hardening effort.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

[Andrew Cooper]

On 19/07/2019 15:33, Laszlo Ersek wrote:
> On 07/19/19 12:20, Anthony PERARD wrote:
>> On Fri, Jul 05, 2019 at 02:57:06PM +0100, Andrew Cooper wrote:
>>> On 04/07/2019 15:42, Anthony PERARD wrote:
 diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm 
 b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
 new file mode 100644
 index 00..958195bc5e
 --- /dev/null
 +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
 +vtfSignature:
 +DB  'V', 'T', 'F', 0
 +
 +ALIGN   16
 +
 +resetVector:
 +;
 +; Reset Vector
 +;
 +; This is where the processor will begin execution
 +;
 +nop
 +nop
>>> Why two nops?
>> I don't know, this is existing code that I duplicated to allow adding a
>> new entry point. (I wanted to use --find-copies-harder when sending the
>> patch, but forgot this time.
> Not a big problem; while reviewing v3, I did such comparisons myself, in
> my local clone. Feel free to skip "--find-copies-harder" when posting v4
> too; I think there isn't much churn going on in parallel right now.
>
> However, a new request for v4: please make sure that the new modules /
> paths introduced by this patch set are covered in Maintainers.txt.
>
>> This part of the chunk would not be there.)
> Regarding the NOPs: all I can tell you is that they originate from
> commit 8332983e2e33 ("UefiCpuPkg: Replace the un-necessary WBINVD
> instruction at the reset vector with two NOPs in VTF0.", 2011-08-04).
>
> Whether that change made sense back then, let alone if it makes sense
> now: no clue.

Dropping wbinvd makes sense, because when virtualised, the caches (and
paging for that matter) are always up and running correctly.  Its an
unnecessary vmexit for something which the hypervisor will nop out anyway.

Leaving two nops behind makes no sense at all.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

[Laszlo Ersek]

On 07/19/19 12:20, Anthony PERARD wrote:
> On Fri, Jul 05, 2019 at 02:57:06PM +0100, Andrew Cooper wrote:
>> On 04/07/2019 15:42, Anthony PERARD wrote:
>>> diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm 
>>> b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>>> new file mode 100644
>>> index 00..958195bc5e
>>> --- /dev/null
>>> +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>>> +vtfSignature:
>>> +DB  'V', 'T', 'F', 0
>>> +
>>> +ALIGN   16
>>> +
>>> +resetVector:
>>> +;
>>> +; Reset Vector
>>> +;
>>> +; This is where the processor will begin execution
>>> +;
>>> +nop
>>> +nop
>>
>> Why two nops?
> 
> I don't know, this is existing code that I duplicated to allow adding a
> new entry point. (I wanted to use --find-copies-harder when sending the
> patch, but forgot this time.

Not a big problem; while reviewing v3, I did such comparisons myself, in
my local clone. Feel free to skip "--find-copies-harder" when posting v4
too; I think there isn't much churn going on in parallel right now.

However, a new request for v4: please make sure that the new modules /
paths introduced by this patch set are covered in Maintainers.txt.

> This part of the chunk would not be there.)

Regarding the NOPs: all I can tell you is that they originate from
commit 8332983e2e33 ("UefiCpuPkg: Replace the un-necessary WBINVD
instruction at the reset vector with two NOPs in VTF0.", 2011-08-04).

Whether that change made sense back then, let alone if it makes sense
now: no clue.

Thanks
Laszlo

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] preparations for 4.12.1

[Jan Beulich]

All,

the release is due in early August. Please point out backports you
find missing from the respective staging branch, but which you
consider relevant. The one commit I've queued already on top of
what was just pushed is

ec2ab491b5  x86/ept: pass correct level to p2m_entry_modify

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

[Razvan Cojocaru]

On 7/19/19 4:38 PM, Jan Beulich wrote:

On 19.07.2019 15:30, Razvan Cojocaru wrote:

On 7/19/19 4:18 PM, Jan Beulich wrote:

On 19.07.2019 14:34, Alexandru Stefan ISAILA wrote:

On 18.07.2019 15:58, Jan Beulich wrote:

On 03.07.2019 12:56, Alexandru Stefan ISAILA wrote:

A/D bit writes (on page walks) can be considered benign by an introspection
agent, so receiving vm_events for them is a pessimization. We try here to
optimize by fitering these events out.


But you add the sending of more events - how does "filter out" match
the actual implementation?


The events are send only if there is a mem access violation therefore we
are filtering and only sending the events that are interesting to
introspection.


Where is it that you prevent any event from being sent? As said,
reading the patch I only see new sending sites to get added.


If we don't emulate, we would receive the page-walk-generated events
_and_ the touching-the-page-the-instruction-is-touching events.


Since the patch here alters emulation paths only, how do you know
whether to emulate? In order to not receive undue events it would
seem to me that you'd first have to intercept the guest on insns
of interest ... Overall I think that the patch description, while
it has improved, is still lacking sufficient information for a
person like me (not knowing much about your monitor tools) to be
able to sensibly review this (which includes understanding the
precise scenario you want to improve).


If the hardware exits because of an EPT fault caused by a page walk, we 
end up in p2m_mem_access_check(), at which point we need to decide if we 
want to send out a vm_event or not.


If we were to send out this vm_event, and it would then be magically 
treated so that we get to actually run the instruction at RIP, said 
instruction might also hit a protected page and provoke a vm_event.


Now, if npfec.kind != npfec_kind_with_gla, then we're in the page walk 
case, and so in this case only, and only if 
d->arch.monitor.inguest_pagefault_disabled is true, we would choose to 
do this emulation trick: emulate _the_page_walk_ while ignoring the EPT, 
but don't ignore the EPT for the emulation of the actual instruction.


So where in the first case we would have 2 EPT events, in the second we 
only have one (or if the instruction at RIP does not trigger an EPT 
event, we would have 1 event in the first case, and none in the second).

Hence the filtering mentioned.

So to answer your question: "how do you know whether to emulate", we do 
so only if npfec.kind != npfec_kind_with_gla && 
d->arch.monitor.inguest_pagefault_disabled.


I hope this clears it up somewhat.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen backports

[Andrew Cooper]

On 19/07/2019 14:53, Jan Beulich wrote:
> On 16.07.2019 14:27, Andrew Cooper wrote:
>> Bugfixes:
>>
>> 65c165d6595f - x86/xstate: Don't special case feature collection
>> 013896cb8b2f - x86/msr: Fix handling of
>> MSR_AMD_PATCHLEVEL/MSR_IA32_UCODE_REV
>> 7d161f653755 - x86/svm: Fix svm_vmcb_dump() when used in current context
>> 9b757bdc1794 - x86/boot: Don't leak the module_map allocation in
>> __start_xen()
>>
>> The backport for the microcode MSR is rather tricky to rebase over the
>> x86_vendor bitmap changes.  I've already got it locally.
> Not overly tricky I would say, but requiring attention. While doing
> this I've run into two questions:
>
> 1) Was it really a good idea to remove the write to the MSR and the
> CPUID invocation from the read path? The comment says 'A guest might
> itself perform the "write 0, CPUID, read" sequence', but that won't
> help at all if the specific vCPU gets re-scheduled in the middle. And
> there may not have been any ucode load we've done, which we could
> then guarantee was followed by a CPUID invocation.

You asked the same on the original patch.  The "write 0, CPUID" is a
bodge for the P5 which had to introduce the UCODE_REV MSR in microcode,
in a compatible fashion.  The write 0 is unnecessary on all subsequent
processors.

All that matters, for any CPU Xen will boot on, is that a CPUID
instruction has executed previously on the current CPU, which is
guaranteed by our AP boot logic, even if we haven't explicitly loaded
microcode.

> 2) We still haven't got confirmation that Hygon behaves the same ucode-
> wise, have we?

Until we get an answer to the question, I'm trust treating Hygon as "no
microcode facilities".

>
>> Enhancements:
>>
>> 787619a0640e - tools: re-sync CPUID leaf 7 tables (perhaps only the
>> xen-cpuid bits.)
>> 260acc521db4 - x86/clear_page: Update clear_page_sse2() after dropping
>> 32bit Xen
>> 564d261687c0 - x86/ctxt-switch: Document and improve GDT handling
> The last one doesn't even come close to applying, due to its dependency
> on 12dce7ea5a. While I think that's a little too much, I've still
> decided to pull in both (but for now I'll perhaps do this only for 4.12)
> in anticipation of us wanting to at least consider a backport of the
> core scheduling series (assuming it won't take too long to get fully
> ready).
>
>> Altp2m: While altp2m isn't supported yet, it would be very helpful to
>> two downstreams to take these fixes while we work on getting it fully
>> supported.
>>
>> 44f3c3cdd315 - x86/altp2m: treat view 0 as the hostp2m in
>> p2m_get_mem_access()
>> 8228577ad1ba - x86/hvm: Fix altp2m_op hypercall continuations
>> 9abcac7ff145 - x86/altp2m: cleanup p2m_altp2m_lazy_copy
>> df4e4cafd28d - x86/altp2m: Fix style errors introduced with c/s 9abcac7ff
> I'll apply all of these soon.

Thanks,

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 3/6] pci: switch pci_conf_read32 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

While there convert {IGD/IOH}_DEV to be a pci_sbdf_t itself instead of
a device number.

Signed-off-by: Roger Pau Monné 
Acked-by: Brian Woods 
Reviewed-by: Kevin Tian 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
---
Changes since v3:
 - Fix two usages of u32 and u64.
 - Switch {IGD/IOH}_DEV to pci_sbdf_t.
---
 xen/arch/x86/cpu/amd.c |  7 ++--
 xen/arch/x86/mm.c  |  2 +-
 xen/arch/x86/msi.c | 30 ++
 xen/arch/x86/oprofile/op_model_athlon.c|  6 ++-
 xen/arch/x86/x86_64/mmconf-fam10h.c|  8 ++--
 xen/arch/x86/x86_64/mmconfig-shared.c  | 12 +++---
 xen/arch/x86/x86_64/pci.c  | 27 ++---
 xen/drivers/char/ehci-dbgp.c   | 20 ++
 xen/drivers/char/ns16550.c | 18 +
 xen/drivers/passthrough/amd/iommu_detect.c |  2 +-
 xen/drivers/passthrough/amd/iommu_init.c   |  4 +-
 xen/drivers/passthrough/pci.c  | 15 +++
 xen/drivers/passthrough/vtd/quirks.c   | 46 +++---
 xen/drivers/pci/pci.c  |  4 +-
 xen/drivers/vpci/header.c  |  6 +--
 xen/drivers/vpci/msix.c|  6 +--
 xen/drivers/vpci/vpci.c|  5 +--
 xen/include/xen/pci.h  |  4 +-
 18 files changed, 106 insertions(+), 116 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c6c74e75f5..8a40373ddf 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -417,7 +417,8 @@ static void disable_c1_ramping(void)
int node, nr_nodes;
 
/* Read the number of nodes from the first Northbridge. */
-   nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1;
+   nr_nodes = ((pci_conf_read32(PCI_SBDF(0, 0, 0x18, 0), 0x60) >> 4) &
+   0x07) + 1;
for (node = 0; node < nr_nodes; node++) {
/* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */
pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87);
@@ -703,8 +704,8 @@ static void init_amd(struct cpuinfo_x86 *c)
 
if (c->x86 == 0x16 && c->x86_model <= 0xf) {
if (c == &boot_cpu_data) {
-   l = pci_conf_read32(0, 0, 0x18, 0x3, 0x58);
-   h = pci_conf_read32(0, 0, 0x18, 0x3, 0x5c);
+   l = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x58);
+   h = pci_conf_read32(PCI_SBDF(0, 0, 0x18, 3), 0x5c);
if ((l & 0x1f) | (h & 0x1))
printk(KERN_WARNING
   "Applying workaround for erratum 792: 
%s%s%s\n",
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 334571d445..56b95e15d1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5950,7 +5950,7 @@ const struct platform_bad_page *__init 
get_platform_badpages(unsigned int *array
 }
 
 *array_size = ARRAY_SIZE(snb_bad_pages);
-igd_id = pci_conf_read32(0, 0, 2, 0, 0);
+igd_id = pci_conf_read32(PCI_SBDF(0, 0, 2, 0), 0);
 if ( IS_SNB_GFX(igd_id) )
 return snb_bad_pages;
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 7df9ddacff..52617cd843 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -191,16 +191,13 @@ static bool read_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
 {
 struct pci_dev *dev = entry->dev;
 int pos = entry->msi_attrib.pos;
-u16 data, seg = dev->seg;
-u8 bus = dev->bus;
-u8 slot = PCI_SLOT(dev->devfn);
-u8 func = PCI_FUNC(dev->devfn);
+uint16_t data;
 
-msg->address_lo = pci_conf_read32(seg, bus, slot, func,
+msg->address_lo = pci_conf_read32(dev->sbdf,
   msi_lower_address_reg(pos));
 if ( entry->msi_attrib.is_64 )
 {
-msg->address_hi = pci_conf_read32(seg, bus, slot, func,
+msg->address_hi = pci_conf_read32(dev->sbdf,
   msi_upper_address_reg(pos));
 data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 1));
 }
@@ -396,7 +393,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool 
host, bool guest)
 {
 u32 mask_bits;
 
-mask_bits = pci_conf_read32(seg, bus, slot, func, entry->msi.mpos);
+mask_bits = pci_conf_read32(pdev->sbdf, entry->msi.mpos);
 mask_bits &= ~((u32)1 << entry->msi_attrib.entry_nr);
 mask_bits |= (u32)flag << entry->msi_attrib.entry_nr;
 pci_conf_write32(seg, bus, slot, func, entry->msi.mpos, mask_bits);
@@

[Xen-devel] [PATCH v4 2/6] pci: switch pci_conf_read16 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
Acked-by: Brian Woods 
Reviewed-by: Kevin Tian 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
---
Changes since v3:
 - Convert one missing u16 to uint16_t.
---
 xen/arch/x86/dmi_scan.c  |  6 +-
 xen/arch/x86/msi.c   | 73 ++--
 xen/arch/x86/x86_64/mmconfig-shared.c|  2 +-
 xen/arch/x86/x86_64/pci.c| 27 -
 xen/drivers/char/ehci-dbgp.c |  5 +-
 xen/drivers/char/ns16550.c   | 16 --
 xen/drivers/passthrough/amd/iommu_init.c |  3 +-
 xen/drivers/passthrough/ats.h|  4 +-
 xen/drivers/passthrough/pci.c| 40 +
 xen/drivers/passthrough/vtd/quirks.c |  9 ++-
 xen/drivers/passthrough/x86/ats.c|  9 +--
 xen/drivers/pci/pci.c|  4 +-
 xen/drivers/video/vga.c  |  8 +--
 xen/drivers/vpci/header.c| 11 ++--
 xen/drivers/vpci/msi.c   |  3 +-
 xen/drivers/vpci/msix.c  |  3 +-
 xen/drivers/vpci/vpci.c  | 11 ++--
 xen/include/xen/pci.h|  4 +-
 18 files changed, 99 insertions(+), 139 deletions(-)

diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
index fcdf2d3952..31caad133e 100644
--- a/xen/arch/x86/dmi_scan.c
+++ b/xen/arch/x86/dmi_scan.c
@@ -469,15 +469,15 @@ static int __init ich10_bios_quirk(struct dmi_system_id 
*d)
 {
 u32 port, smictl;
 
-if ( pci_conf_read16(0, 0, 0x1f, 0, PCI_VENDOR_ID) != 0x8086 )
+if ( pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), PCI_VENDOR_ID) != 0x8086 )
 return 0;
 
-switch ( pci_conf_read16(0, 0, 0x1f, 0, PCI_DEVICE_ID) ) {
+switch ( pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), PCI_DEVICE_ID) ) {
 case 0x3a14:
 case 0x3a16:
 case 0x3a18:
 case 0x3a1a:
-port = (pci_conf_read16(0, 0, 0x1f, 0, 0x40) & 0xff80) + 0x30;
+port = (pci_conf_read16(PCI_SBDF(0, 0, 0x1f, 0), 0x40) & 0xff80) + 
0x30;
 smictl = inl(port);
 /* turn off LEGACY_USB{,2}_EN if enabled */
 if ( smictl & 0x20008 )
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index ed8b89cc0f..7df9ddacff 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -124,29 +124,20 @@ static void msix_put_fixmap(struct arch_msix *msix, int 
idx)
 
 static bool memory_decoded(const struct pci_dev *dev)
 {
-u8 bus, slot, func;
+pci_sbdf_t sbdf = dev->sbdf;
 
-if ( !dev->info.is_virtfn )
-{
-bus = dev->bus;
-slot = PCI_SLOT(dev->devfn);
-func = PCI_FUNC(dev->devfn);
-}
-else
+if ( dev->info.is_virtfn )
 {
-bus = dev->info.physfn.bus;
-slot = PCI_SLOT(dev->info.physfn.devfn);
-func = PCI_FUNC(dev->info.physfn.devfn);
+sbdf.bus = dev->info.physfn.bus;
+sbdf.devfn = dev->info.physfn.devfn;
 }
 
-return !!(pci_conf_read16(dev->seg, bus, slot, func, PCI_COMMAND) &
-  PCI_COMMAND_MEMORY);
+return pci_conf_read16(sbdf, PCI_COMMAND) & PCI_COMMAND_MEMORY;
 }
 
 static bool msix_memory_decoded(const struct pci_dev *dev, unsigned int pos)
 {
-u16 control = pci_conf_read16(dev->seg, dev->bus, PCI_SLOT(dev->devfn),
-  PCI_FUNC(dev->devfn), msix_control_reg(pos));
+uint16_t control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
 
 if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
 return false;
@@ -211,14 +202,12 @@ static bool read_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
 {
 msg->address_hi = pci_conf_read32(seg, bus, slot, func,
   msi_upper_address_reg(pos));
-data = pci_conf_read16(seg, bus, slot, func,
-   msi_data_reg(pos, 1));
+data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 1));
 }
 else
 {
 msg->address_hi = 0;
-data = pci_conf_read16(seg, bus, slot, func,
-   msi_data_reg(pos, 0));
+data = pci_conf_read16(dev->sbdf, msi_data_reg(pos, 0));
 }
 msg->data = data;
 break;
@@ -337,7 +326,8 @@ void set_msi_affinity(struct irq_desc *desc, const 
cpumask_t *mask)
 
 void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable)
 {
-u16 control = pci_conf_read16(seg, bus, slot, func, pos + PCI_MSI_FLAGS);
+uint16_t control = pci_conf_read16(PCI_SBDF(seg, bus, slot, func),
+   pos + PCI_MSI_FLAGS);
 
 control &= ~PCI_MSI_FLAGS_ENABLE;
 if ( enable )
@@ -369,7 +359,7 @@ static v

[Xen-devel] [PATCH v4 4/6] pci: switch pci_conf_write8 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
---
Changes since v3:
 - Drop pointless AND-ing by constants.
 - Remove insertion of stray newline.
---
 xen/arch/x86/cpu/amd.c   |  2 +-
 xen/arch/x86/x86_64/pci.c| 21 -
 xen/drivers/acpi/reboot.c|  5 ++---
 xen/drivers/char/ehci-dbgp.c |  8 +---
 xen/drivers/vpci/vpci.c  |  8 +++-
 xen/include/xen/pci.h|  4 +---
 6 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 8a40373ddf..f66de362cb 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -426,7 +426,7 @@ static void disable_c1_ramping(void)
if (pmm7 == 0xFF)
break;
pmm7 &= 0xFC; /* clear pmm7[1:0] */
-   pci_conf_write8(0, 0, 0x18+node, 0x3, 0x87, pmm7);
+   pci_conf_write8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87, pmm7);
printk ("AMD: Disabling C1 Clock Ramping Node #%x\n", node);
}
 }
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index b8b82a6fe7..eaa67b04f2 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -50,23 +50,18 @@ uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
 return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-(0x8000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-void pci_conf_write8(
-unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-unsigned int reg, uint8_t data)
+void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data)
 {
-if ( seg || reg > 255 )
-pci_mmcfg_write(seg, bus, PCI_DEVFN(dev, func), reg, 1, data);
+if ( sbdf.seg || reg > 255 )
+pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, data);
 else
-{
-BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1, 
data);
-}
+pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, data);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+(0x8000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 void pci_conf_write16(
 unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
 unsigned int reg, uint16_t data)
diff --git a/xen/drivers/acpi/reboot.c b/xen/drivers/acpi/reboot.c
index 72d06fd8e5..f6345be874 100644
--- a/xen/drivers/acpi/reboot.c
+++ b/xen/drivers/acpi/reboot.c
@@ -23,9 +23,8 @@ void acpi_reboot(void)
case ACPI_ADR_SPACE_PCI_CONFIG:
printk("Resetting with ACPI PCI RESET_REG.\n");
/* Write the value that resets us. */
-   pci_conf_write8(0, 0,
-   (rr->address >> 32) & 31,
-   (rr->address >> 16) & 7,
+   pci_conf_write8(PCI_SBDF(0, 0, rr->address >> 32,
+rr->address >> 16),
(rr->address & 255),
reset_value);
break;
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 9b9025fb33..010fc3c5bc 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1048,7 +1048,8 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp 
*dbgp, u32 hcc_params)
 if ( (cap & 0xff) == 1 && (cap & EHCI_USBLEGSUP_BIOS) )
 {
 dbgp_printk("dbgp: BIOS handoff\n");
-pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 3, 1);
+pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+offset + 3, 1);
 }
 
 /* if boot firmware now owns EHCI, spin till it hands it over. */
@@ -1066,11 +1067,12 @@ static void ehci_dbgp_bios_handoff(struct ehci_dbgp 
*dbgp, u32 hcc_params)
 /* well, possibly buggy BIOS... try to shut it down,
  * and hope nothing goes too wrong */
 dbgp_printk("dbgp: BIOS handoff failed: %08x\n", cap);
-pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 2, 0);
+pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+offset + 2, 0);
 }
 
 /* just in case, always disable EHCI SMIs */
-pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func,
+pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
 offset + EHCI_USBLEGCTLSTS, 0);
 }
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 2106255863..b141e57883 100644
--- a/xen/drivers/vpci/vpc

[Xen-devel] [PATCH v4 0/6] pci: expand usage of pci_sbdf_t

[Roger Pau Monne]

Hello,

The following are the remaining patches from my 'expand usage of
pci_sbdf_t' previous series except for the one that introduces a custom
printf formatter for pci_sbdf_t.

I've also pushed them to my git tree at:

git://xenbits.xen.org/people/royger/xen.git pci_sbdf_t

Thanks, Roger.

Roger Pau Monne (6):
  pci: switch pci_conf_read8 to use pci_sbdf_t
  pci: switch pci_conf_read16 to use pci_sbdf_t
  pci: switch pci_conf_read32 to use pci_sbdf_t
  pci: switch pci_conf_write8 to use pci_sbdf_t
  pci: switch pci_conf_write16 to use pci_sbdf_t
  pci: switch pci_conf_write32 to use pci_sbdf_t

 xen/arch/x86/cpu/amd.c |  15 +-
 xen/arch/x86/dmi_scan.c|   6 +-
 xen/arch/x86/mm.c  |   2 +-
 xen/arch/x86/msi.c | 170 +
 xen/arch/x86/oprofile/op_model_athlon.c|  10 +-
 xen/arch/x86/x86_64/mmconf-fam10h.c|   8 +-
 xen/arch/x86/x86_64/mmconfig-shared.c  |  14 +-
 xen/arch/x86/x86_64/pci.c  |  98 +---
 xen/drivers/acpi/reboot.c  |   5 +-
 xen/drivers/char/ehci-dbgp.c   |  49 +++---
 xen/drivers/char/ns16550.c |  71 +
 xen/drivers/passthrough/amd/iommu_detect.c |   2 +-
 xen/drivers/passthrough/amd/iommu_init.c   |  17 +--
 xen/drivers/passthrough/ats.h  |   4 +-
 xen/drivers/passthrough/pci.c  | 102 +
 xen/drivers/passthrough/vtd/dmar.c |   6 +-
 xen/drivers/passthrough/vtd/quirks.c   |  61 
 xen/drivers/passthrough/x86/ats.c  |  15 +-
 xen/drivers/pci/pci.c  |  23 ++-
 xen/drivers/video/vga.c|  11 +-
 xen/drivers/vpci/header.c  |  47 +++---
 xen/drivers/vpci/msi.c |   7 +-
 xen/drivers/vpci/msix.c|  11 +-
 xen/drivers/vpci/vpci.c|  42 ++---
 xen/include/xen/pci.h  |  24 +--
 25 files changed, 353 insertions(+), 467 deletions(-)

-- 
2.20.1 (Apple Git-117)


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 6/6] pci: switch pci_conf_write32 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
Acked-by: Brian Woods 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
---
 xen/arch/x86/cpu/amd.c   |  4 ++--
 xen/arch/x86/msi.c   | 12 
 xen/arch/x86/oprofile/op_model_athlon.c  |  4 +++-
 xen/arch/x86/x86_64/pci.c| 17 -
 xen/drivers/char/ehci-dbgp.c |  5 +++--
 xen/drivers/char/ns16550.c   | 22 --
 xen/drivers/passthrough/amd/iommu_init.c |  8 
 xen/drivers/passthrough/pci.c|  8 
 xen/drivers/passthrough/vtd/quirks.c |  8 
 xen/drivers/vpci/header.c|  7 +++
 xen/drivers/vpci/vpci.c  |  2 +-
 xen/include/xen/pci.h|  4 +---
 12 files changed, 45 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index f66de362cb..a2f83c79a5 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -714,11 +714,11 @@ static void init_amd(struct cpuinfo_x86 *c)
   (h & 0x1) ? "clearing D18F3x5C[0]" : "");
 
if (l & 0x1f)
-   pci_conf_write32(0, 0, 0x18, 0x3, 0x58,
+   pci_conf_write32(PCI_SBDF(0, 0, 0x18, 3), 0x58,
 l & ~0x1f);
 
if (h & 0x1)
-   pci_conf_write32(0, 0, 0x18, 0x3, 0x5c,
+   pci_conf_write32(PCI_SBDF(0, 0, 0x18, 3), 0x5c,
 h & ~0x1);
}
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index f3e5c5cb03..d6306005a9 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -251,21 +251,17 @@ static int write_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
 {
 struct pci_dev *dev = entry->dev;
 int pos = entry->msi_attrib.pos;
-u16 seg = dev->seg;
-u8 bus = dev->bus;
-u8 slot = PCI_SLOT(dev->devfn);
-u8 func = PCI_FUNC(dev->devfn);
 int nr = entry->msi_attrib.entry_nr;
 
 ASSERT((msg->data & (entry[-nr].msi.nvec - 1)) == nr);
 if ( nr )
 return 0;
 
-pci_conf_write32(seg, bus, slot, func, msi_lower_address_reg(pos),
+pci_conf_write32(dev->sbdf, msi_lower_address_reg(pos),
  msg->address_lo);
 if ( entry->msi_attrib.is_64 )
 {
-pci_conf_write32(seg, bus, slot, func, msi_upper_address_reg(pos),
+pci_conf_write32(dev->sbdf, msi_upper_address_reg(pos),
  msg->address_hi);
 pci_conf_write16(dev->sbdf, msi_data_reg(pos, 1), msg->data);
 }
@@ -395,7 +391,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool 
host, bool guest)
 mask_bits = pci_conf_read32(pdev->sbdf, entry->msi.mpos);
 mask_bits &= ~((u32)1 << entry->msi_attrib.entry_nr);
 mask_bits |= (u32)flag << entry->msi_attrib.entry_nr;
-pci_conf_write32(seg, bus, slot, func, entry->msi.mpos, mask_bits);
+pci_conf_write32(pdev->sbdf, entry->msi.mpos, mask_bits);
 }
 break;
 case PCI_CAP_ID_MSIX:
@@ -716,7 +712,7 @@ static int msi_capability_init(struct pci_dev *dev,
 /* All MSIs are unmasked by default, Mask them all */
 maskbits = pci_conf_read32(dev->sbdf, mpos);
 maskbits |= ~(u32)0 >> (32 - maxvec);
-pci_conf_write32(seg, bus, slot, func, mpos, maskbits);
+pci_conf_write32(dev->sbdf, mpos, maskbits);
 }
 list_add_tail(&entry->list, &dev->msi_list);
 
diff --git a/xen/arch/x86/oprofile/op_model_athlon.c 
b/xen/arch/x86/oprofile/op_model_athlon.c
index 3bf0b0214d..5c48f868ae 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -472,7 +472,9 @@ static int __init init_ibs_nmi(void)
if ((vendor_id == PCI_VENDOR_ID_AMD) &&
(dev_id == 
PCI_DEVICE_ID_AMD_10H_NB_MISC)) {
 
-   pci_conf_write32(0, bus, dev, func, 
IBSCTL,
+   pci_conf_write32(
+   PCI_SBDF(0, bus, dev, func),
+   IBSCTL,
IBSCTL_LVTOFFSETVAL | 
APIC_EILVT_LVTOFF_IBS);
 
value = pci_conf_read32(PCI_SBDF(0, 
bus, dev, func),
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x

[Xen-devel] [PATCH v4 5/6] pci: switch pci_conf_write16 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
---
Changes since v3:
 - Remove stray change of pci_conf_write32 call.
---
 xen/arch/x86/msi.c| 53 ++-
 xen/arch/x86/x86_64/pci.c | 21 +---
 xen/drivers/char/ehci-dbgp.c  |  6 ++--
 xen/drivers/char/ns16550.c|  9 --
 xen/drivers/passthrough/pci.c | 18 ---
 xen/drivers/passthrough/x86/ats.c |  6 ++--
 xen/drivers/pci/pci.c |  6 +---
 xen/drivers/vpci/header.c | 22 -
 xen/drivers/vpci/msi.c|  4 +--
 xen/drivers/vpci/msix.c   |  2 +-
 xen/drivers/vpci/vpci.c   |  8 ++---
 xen/include/xen/pci.h |  4 +--
 12 files changed, 62 insertions(+), 97 deletions(-)

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 52617cd843..f3e5c5cb03 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -267,12 +267,10 @@ static int write_msi_msg(struct msi_desc *entry, struct 
msi_msg *msg)
 {
 pci_conf_write32(seg, bus, slot, func, msi_upper_address_reg(pos),
  msg->address_hi);
-pci_conf_write16(seg, bus, slot, func, msi_data_reg(pos, 1),
- msg->data);
+pci_conf_write16(dev->sbdf, msi_data_reg(pos, 1), msg->data);
 }
 else
-pci_conf_write16(seg, bus, slot, func, msi_data_reg(pos, 0),
- msg->data);
+pci_conf_write16(dev->sbdf, msi_data_reg(pos, 0), msg->data);
 break;
 }
 case PCI_CAP_ID_MSIX:
@@ -329,7 +327,8 @@ void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, 
int pos, int enable)
 control &= ~PCI_MSI_FLAGS_ENABLE;
 if ( enable )
 control |= PCI_MSI_FLAGS_ENABLE;
-pci_conf_write16(seg, bus, slot, func, pos + PCI_MSI_FLAGS, control);
+pci_conf_write16(PCI_SBDF(seg, bus, slot, func),
+ pos + PCI_MSI_FLAGS, control);
 }
 
 static void msi_set_enable(struct pci_dev *dev, int enable)
@@ -360,7 +359,7 @@ static void msix_set_enable(struct pci_dev *dev, int enable)
 control &= ~PCI_MSIX_FLAGS_ENABLE;
 if ( enable )
 control |= PCI_MSIX_FLAGS_ENABLE;
-pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), control);
+pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
 }
 }
 
@@ -406,7 +405,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool 
host, bool guest)
 if ( unlikely(!(control & PCI_MSIX_FLAGS_ENABLE)) )
 {
 pdev->msix->host_maskall = 1;
-pci_conf_write16(seg, bus, slot, func,
+pci_conf_write16(pdev->sbdf,
  msix_control_reg(entry->msi_attrib.pos),
  control | (PCI_MSIX_FLAGS_ENABLE |
 PCI_MSIX_FLAGS_MASKALL));
@@ -440,7 +439,7 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool 
host, bool guest)
 pdev->msix->host_maskall = maskall;
 if ( maskall || pdev->msix->guest_maskall )
 control |= PCI_MSIX_FLAGS_MASKALL;
-pci_conf_write16(seg, bus, slot, func,
+pci_conf_write16(pdev->sbdf,
  msix_control_reg(entry->msi_attrib.pos), control);
 return flag;
 default:
@@ -580,8 +579,7 @@ int setup_msi_irq(struct irq_desc *desc, struct msi_desc 
*msidesc)
 {
 control = pci_conf_read16(pdev->sbdf, cpos);
 if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
-pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
- PCI_FUNC(pdev->devfn), cpos,
+pci_conf_write16(pdev->sbdf, cpos,
  control | (PCI_MSIX_FLAGS_ENABLE |
 PCI_MSIX_FLAGS_MASKALL));
 }
@@ -591,8 +589,7 @@ int setup_msi_irq(struct irq_desc *desc, struct msi_desc 
*msidesc)
: &pci_msi_nonmaskable);
 
 if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
-pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
- PCI_FUNC(pdev->devfn), cpos, control);
+pci_conf_write16(pdev->sbdf, cpos, control);
 
 return rc;
 }
@@ -735,7 +732,7 @@ static int msi_capability_init(struct pci_dev *dev,
 pci_intx(dev, false);
 control |= PCI_MSI_FLAGS_ENABLE;
 }
-pci_conf_write16(seg, bus, slot, func, msi_control_reg(pos), control);
+pci_conf_write16(dev->sbdf, msi_control_reg(pos), control);
 
 return 0;
 }
@@ -856,13 +853,13 @@ static int msix_capability_init(struct pci_dev *dev,
  * fully set up.
 

[Xen-devel] [PATCH v4 1/6] pci: switch pci_conf_read8 to use pci_sbdf_t

[Roger Pau Monne]

This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
Acked-by: Brian Woods 
Reviewed-by: Kevin Tian 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
---
Changes since v3:
 - Drop stray change to one pci_conf_write8 instance.
---
 xen/arch/x86/cpu/amd.c   |  2 +-
 xen/arch/x86/msi.c   |  2 +-
 xen/arch/x86/x86_64/pci.c| 25 
 xen/drivers/char/ehci-dbgp.c |  5 +++--
 xen/drivers/char/ns16550.c   |  6 --
 xen/drivers/passthrough/amd/iommu_init.c |  2 +-
 xen/drivers/passthrough/pci.c| 21 
 xen/drivers/passthrough/vtd/dmar.c   |  6 +++---
 xen/drivers/passthrough/vtd/quirks.c |  6 +++---
 xen/drivers/pci/pci.c|  9 -
 xen/drivers/video/vga.c  |  3 +--
 xen/drivers/vpci/header.c|  3 +--
 xen/drivers/vpci/vpci.c  |  8 +++-
 xen/include/xen/pci.h|  4 +---
 14 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 839f19292d..c6c74e75f5 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -420,7 +420,7 @@ static void disable_c1_ramping(void)
nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1;
for (node = 0; node < nr_nodes; node++) {
/* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */
-   pmm7 = pci_conf_read8(0, 0, 0x18+node, 0x3, 0x87);
+   pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87);
/* Invalid read means we've updated every Northbridge. */
if (pmm7 == 0xFF)
break;
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 89e61160e9..ed8b89cc0f 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -800,7 +800,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 
func, u8 bir, int vf)
 disp = vf * pdev->vf_rlen[bir];
 limit = PCI_SRIOV_NUM_BARS;
 }
-else switch ( pci_conf_read8(seg, bus, slot, func,
+else switch ( pci_conf_read8(PCI_SBDF(seg, bus, slot, func),
  PCI_HEADER_TYPE) & 0x7f )
 {
 case PCI_HEADER_TYPE_NORMAL:
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index 6e3f5cf203..b70383fb03 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -8,27 +8,26 @@
 #include 
 #include 
 
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-(0x8000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+#define PCI_CONF_ADDRESS(sbdf, reg) \
+(0x8000 | ((sbdf).bdf << 8) | ((reg) & ~3))
 
-uint8_t pci_conf_read8(
-unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-unsigned int reg)
+uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {
-u32 value;
+uint32_t value;
 
-if ( seg || reg > 255 )
+if ( sbdf.seg || reg > 255 )
 {
-pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 1, &value);
+pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);
 return value;
 }
-else
-{
-BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 
1);
-}
+
+return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+(0x8000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 uint16_t pci_conf_read16(
 unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
 unsigned int reg)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 475dc41767..71f0aaa6ac 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -713,7 +713,7 @@ static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp,
 cap = __find_dbgp(bus, slot, func);
 if ( !cap || ehci_num-- )
 {
-if ( !func && !(pci_conf_read8(0, bus, slot, func,
+if ( !func && !(pci_conf_read8(PCI_SBDF(0, bus, slot, 
func),
PCI_HEADER_TYPE) & 0x80) )
 break;
 continue;
@@ -1312,7 +1312,8 @@ static void __init ehci_dbgp_init_preirq(struct 
serial_port *port)
 offset = (debug_port >> 16) & 0xfff;
 
 /* double check if the mem space is enabled */
-dbgp->pci_cr = pci_conf_read8(0, dbgp->bus, dbgp->slot, dbgp->func,
+dbgp->pci_cr = pci_conf_read8(PCI_SBDF(0, dbgp->bus, 

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

[Anthony PERARD]

On Mon, Jul 15, 2019 at 01:46:57PM +0200, Roger Pau Monné wrote:
> On Thu, Jul 04, 2019 at 03:42:04PM +0100, Anthony PERARD wrote:
> > diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm 
> > b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
> > new file mode 100644
> > index 00..958195bc5e
> > --- /dev/null
> > +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
> > @@ -0,0 +1,81 @@
> > +;--
> > +; @file
> > +; First code executed by processor after resetting.
> > +;
> > +; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
> 
> Extraneous  tag?

Maybe, but I can't change that. Blame the copyright owner ;-). I think
"All rights reserved." could also be removed, or may not apply
(anymore), but that's not something that this patch series can do and
not something I'm going to do :).

> > +; Copyright (c) 2019, Citrix Systems, Inc.
> > +;
> > +; SPDX-License-Identifier: BSD-2-Clause-Patent
> > +;
> > +;--
> > +
> > +BITS16
> > +
> > +ALIGN   16
> 
> Do you need the BITS and ALIGN here?
> 
> Isn't it enough with the BITS 32 below for the entry point, since DB
> is already explicitly sized?

Maybe, but those were already there, so I don't feel comfortable
removing/changing them, or investigating.

FYI, I wanted to send this patch series with --find-copies-harder, but
failed. That chunk would have been instead:

  diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm 
b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
  similarity index 72%
  copy from UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm
  copy to OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
  index 7538192876..958195bc5e 100644
  --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm
  +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
  @@ -3,6 +3,8 @@
   ; First code executed by processor after resetting.
   ;
   ; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
  +; Copyright (c) 2019, Citrix Systems, Inc.
  +;
   ; SPDX-License-Identifier: BSD-2-Clause-Patent
   ;
   
;--
  @@ -21,9 +23,23 @@ ALIGN   16
   ; located just below 0x1 (4GB) in the firmware device.
   ;
   %ifdef ALIGN_TOP_TO_4K_FOR_PAGING
  -TIMES (0x1000 - ($ - EndOfPageTables) - 0x20) DB 0
  +TIMES (0x1000 - ($ - EndOfPageTables) - (fourGigabytes - 
xenPVHEntryPoint)) DB 0
   %endif
  
  +BITS32
  +xenPVHEntryPoint:
  +;
  +; Entry point to use when running as a Xen PVH guest. (0xffd0)
  +;
  +; Description of the expected state of the machine when this entry point is
  +; used can be found at:
  +; https://xenbits.xenproject.org/docs/unstable/misc/pvh.html
  +;
  +jmp xenPVHMain
  +
  +BITS16
  +ALIGN   16
  +
   applicationProcessorEntryPoint:
   ;
   ; Application Processors entry point


> > +
> > +;
> > +; Pad the image size to 4k when page tables are in VTF0
> > +;
> > +; If the VTF0 image has page tables built in, then we need to make
> > +; sure the end of VTF0 is 4k above where the page tables end.
> > +;
> > +; This is required so the page tables will be 4k aligned when VTF0 is
> > +; located just below 0x1 (4GB) in the firmware device.
> > +;
> > +%ifdef ALIGN_TOP_TO_4K_FOR_PAGING
> > +TIMES (0x1000 - ($ - EndOfPageTables) - (fourGigabytes - 
> > xenPVHEntryPoint)) DB 0
> 
> What's the meaning of 0x1000 here?

I don't know. I tried to figure out, but couldn't find a useful answer.
I don't know enough about the build system to figure out how this module
gets build and how it is place exactly where it needs to be.

> > +%endif
> > +
> > +BITS32
> > +xenPVHEntryPoint:
> > +;
> > +; Entry point to use when running as a Xen PVH guest. (0xffd0)
> 
> Shouldn't this positioning be set on the linker script instead?

There is no such thing, at least not in a position that would be useful
for us. That code might be built into an ELF, but then that ELF (or just
the code maybe) gets packaged into a module that gets packaged into a FV
(firmware volume I think), which gets packaged into a flash device
image. (Hopefully, I'm not to far from the reality.)

> > +;
> > +; Description of the expected state of the machine when this entry point is
> > +; used can be found at:
> > +; https://xenbits.xenproject.org/docs/unstable/misc/pvh.html
> > +;
> > +jmp xenPVHMain
> > +
> > +BITS16
> > +ALIGN   16
> 
> Is it really needed to specify both?

I don't know, better safe than sorry.

> I would assume that setting BITS 16 will already set a suitable
> alignment.

I'm guessing they do have different meaning, one doesn't set the other.
I could try to find out in the NASM manual if you really want to know.

Now that I've read what ALIGN mean (see below), they are both needed.
BITS to switch to 16bits machine code, ALIGN so th

Re: [Xen-devel] [PATCH 00/60] xen: add core scheduling support

[Juergen Gross]

On 18.07.19 17:14, Sergey Dyasli wrote:

On 18/07/2019 15:48, Juergen Gross wrote:

On 15.07.19 16:08, Sergey Dyasli wrote:

On 05/07/2019 14:56, Dario Faggioli wrote:

On Fri, 2019-07-05 at 14:17 +0100, Sergey Dyasli wrote:

1) This crash is quite likely to happen:

[2019-07-04 18:22:46 UTC] (XEN) [ 3425.220660] Watchdog timer detects
that CPU2 is stuck!
[2019-07-04 18:22:46 UTC] (XEN) [ 3425.226293] [ Xen-4.13.0-
8.0.6-d  x86_64  debug=y   Not tainted ]
[...]
[2019-07-04 18:22:47 UTC] (XEN) [ 3425.501989] Xen call trace:
[2019-07-04 18:22:47 UTC] (XEN) [
3425.505278]    [] vcpu_sleep_sync+0x50/0x71
[2019-07-04 18:22:47 UTC] (XEN) [
3425.511518]    [] vcpu_pause+0x21/0x23
[2019-07-04 18:22:47 UTC] (XEN) [
3425.517326]    []
vcpu_set_periodic_timer+0x27/0x73
[2019-07-04 18:22:47 UTC] (XEN) [
3425.524258]    [] do_vcpu_op+0x2c9/0x668
[2019-07-04 18:22:47 UTC] (XEN) [
3425.530238]    [] compat_vcpu_op+0x250/0x390
[2019-07-04 18:22:47 UTC] (XEN) [
3425.536566]    [] pv_hypercall+0x364/0x564
[2019-07-04 18:22:47 UTC] (XEN) [
3425.542719]    [] do_entry_int82+0x26/0x2d
[2019-07-04 18:22:47 UTC] (XEN) [
3425.548876]    [] entry_int82+0xbb/0xc0


Mmm... vcpu_set_periodic_timer?

What kernel is this and when does this crash happen?


Hi Dario,

I can easily reproduce this crash using a Debian 7 PV VM (2 vCPUs, 2GB RAM)
which has the following kernel:

# uname -a

Linux localhost 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux

All I need to do is suspend and resume the VM.


Happens with a more recent kernel, too.

I can easily reproduce the issue with any PV guest with more than 1 vcpu
by doing "xl save" and then "xl restore" again.

With the reproducer being available I'm now diving into the issue...


One further thing to add is that I was able to avoid the crash by reverting

xen/sched: rework and rename vcpu_force_reschedule()

which is a part of the series. This made all tests with PV guests pass.

Another question I have is do you have a git branch with core-scheduling
patches rebased on top of current staging available somewhere?


I have now a git branch with the two problems corrected and rebased to
current staging available:

github.com/jgross1/xen.git sched-v1b


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] Xen Security Advisory 300 v2 - Linux: No grant table and foreign mapping limits

[Xen . org security team]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Xen Security Advisory XSA-300
  version 2

 Linux: No grant table and foreign mapping limits

UPDATES IN VERSION 2


Drop inapplicable "Deployment during embargo" section.

Rewrite for clarity, and to remove most references to dom0.  The issue
is equally applicable to domU's providing backend services.

Add information about the arbitrary limit for userspace backends.

ISSUE DESCRIPTION
=

Virtual device backends and device models running in domain 0, or
other backend driver domains, need to be able to map guest memory
(either via grant mappings, or via the foreign mapping interface).

Inside Xen, mapped grants are tracked by the maptrack structure.  The
size of this structure is chosen during domain creation, and has a
fixed upper bound for the lifetime of the domain.

For Linux to keep track of these mappings, it needs to have a page
structure for each one.  In practice the number of page structures is
usually limited.  In PV guests, a range of pfns are typically set
aside at boot ("pre-ballooned") for this purpose.  For HVM/PVH and Arm
guests, no memory is set aside to begin with.  In either case, when
more of this "foreign / grant map pfn space" is needed, Linux will
balloon out extra pages to use for this purpose.

Unfortunately, in Linux, there are no limits, either on the total
amount of memory which the domain will attempt to balloon out, nor on
the amount of "foreign / grant map" memory which any individual guest
can consume.

For Linux userspace backends (e.g. QEMU) which use /dev/xen/gnttab or
/proc/xen/gnttab, there is an arbitrary mapping limit which, if hit,
will prevent further mappings from being established.

As a result, a malicious guest may be able to, with crafted requests,
cause a backend Linux domain to either:

 1) Fill the maptrack table in Xen and/or hit the userspace limit.
This will starve I/O from other guests served by the same backend.

 2) Balloon out sufficient RAM to cause it to swap excessively, or run
completely out of memory.  This may starve all operations from the
domain, including I/O from other guests, or may cause a crash of
the domain.

IMPACT
==

Guest may be able to crash backend Linux domains, or starve operations
inside the domain, including the processing of guest I/O requests
(Guest Denial-of-Service).

If the backend is domain 0, which is the most common configuration,
then host-wide operations may be starved, or the host may crash (Host
Denial-of-Service).

VULNERABLE SYSTEMS
==

All versions of Linux are vulnerable.  Only Linux guests acting as
backend domains for other guests may be exploited.

All Arm domains are vulnerable, as are x86 PVH/HVM guests.  The
vulnerability of x86 PV guests depends on how they were configured at
boot.

MITIGATION
==

PV guests can be constructed with "pre-ballooned" memory, by building
it with maxmem > memory.  See `man 5 xl.cfg` for full details of these
two parameters.

For PV dom0, these are controlled by Xen's "dom0_mem=$X,max:$Y"
command line parameter.

The larger the difference between memory and maxmem, the more space
Linux has to fill with grant/foreign mappings before it will start
ballooning out real memory to satisfy further mapping requests.  This
makes the attack more difficult to accomplish.

CREDITS
===

This issue was discovered by Julien Grall of ARM.

RESOLUTION
==

Applying the appropriate attached patch resolves the backend memory
exhaustion issue.

NOTE: This does NOT fix the guest starvation issue.  Fixing fixing
this issue is more complex, and it was determined that it was better
to work on a robust fix for the issue in public.  This advisory will
be updated when fixes are available.

xsa300-linux-5.2.patch Linux 4.4 ... 5.2

$ sha256sum xsa300*
9c8a9aec52b147f8e8ef41444e1dd11803bacf3bd4d0f6efa863b16f7a9621ac  
xsa300-linux-5.2.patch
$

NOTE ON LACK OF EMBARGO
===

The lack of predisclosure is due to a short schedule set by the
discoverer, and efforts to resolve the advisory wording.
-BEGIN PGP SIGNATURE-

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAl0xyy0MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZyzUH/3hhOLPLuiTnKQd3idx0iIrpRkQfcdl9pxWWARWx
xiVKyyMIajokrq5besT01Ztizz6B80DN+m4W14yi+j8nDyR3W4v/JriZQY48Tj1i
nd+jvBGfvQcjNc5WaVjBtU/x9j0HDCUrBP+uJMGdt9jl6fppvMwnBcv/OeEvl/eE
TjwEMs/RQ69LcjpwGGPSAh8AR2i1+oL3LiHtwO31hdkw0Ritqa32Uw4c+ENuo/OE
PApIX8O8TMgRX0/LriGy6dtlb/L4SljTPa592EHH1cPfDelHmzpWEeIx77nbq8v/
/Ex6Gjd/19ArWvofxQkQk1+aNfvBPnPCaboc7JrlCuFEDP4=
=OcOD
-END PGP SIGNATURE-


xsa300-linux-5.2.patch
Description: Binary data
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen backports

[Jan Beulich]

On 16.07.2019 14:27, Andrew Cooper wrote:
> Bugfixes:
> 
> 65c165d6595f - x86/xstate: Don't special case feature collection
> 013896cb8b2f - x86/msr: Fix handling of
> MSR_AMD_PATCHLEVEL/MSR_IA32_UCODE_REV
> 7d161f653755 - x86/svm: Fix svm_vmcb_dump() when used in current context
> 9b757bdc1794 - x86/boot: Don't leak the module_map allocation in
> __start_xen()
> 
> The backport for the microcode MSR is rather tricky to rebase over the
> x86_vendor bitmap changes.  I've already got it locally.

Not overly tricky I would say, but requiring attention. While doing
this I've run into two questions:

1) Was it really a good idea to remove the write to the MSR and the
CPUID invocation from the read path? The comment says 'A guest might
itself perform the "write 0, CPUID, read" sequence', but that won't
help at all if the specific vCPU gets re-scheduled in the middle. And
there may not have been any ucode load we've done, which we could
then guarantee was followed by a CPUID invocation.

2) We still haven't got confirmation that Hygon behaves the same ucode-
wise, have we?

> Enhancements:
> 
> 787619a0640e - tools: re-sync CPUID leaf 7 tables (perhaps only the
> xen-cpuid bits.)
> 260acc521db4 - x86/clear_page: Update clear_page_sse2() after dropping
> 32bit Xen
> 564d261687c0 - x86/ctxt-switch: Document and improve GDT handling

The last one doesn't even come close to applying, due to its dependency
on 12dce7ea5a. While I think that's a little too much, I've still
decided to pull in both (but for now I'll perhaps do this only for 4.12)
in anticipation of us wanting to at least consider a backport of the
core scheduling series (assuming it won't take too long to get fully
ready).

> Altp2m: While altp2m isn't supported yet, it would be very helpful to
> two downstreams to take these fixes while we work on getting it fully
> supported.
> 
> 44f3c3cdd315 - x86/altp2m: treat view 0 as the hostp2m in
> p2m_get_mem_access()
> 8228577ad1ba - x86/hvm: Fix altp2m_op hypercall continuations
> 9abcac7ff145 - x86/altp2m: cleanup p2m_altp2m_lazy_copy
> df4e4cafd28d - x86/altp2m: Fix style errors introduced with c/s 9abcac7ff

I'll apply all of these soon.

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Tamas K Lengyel]

On Fri, Jul 19, 2019 at 7:31 AM Julien Grall  wrote:
>
>
>
> On 19/07/2019 14:24, Julien Grall wrote:
> > Hi Tamas,
> >
> > On 19/07/2019 14:14, Tamas K Lengyel wrote:
> >> On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:
> >>>
> >>> Hi Tamas,
> >>>
> >>> On 19/07/2019 14:00, Tamas K Lengyel wrote:
>  On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  
>  wrote:
> >
> > Hi Tamas,
> >
> > On 18/07/2019 18:48, Tamas K Lengyel wrote:
> >>>   - Line 1025: The tools needs to be able to deal 
> >>> for_each_vcpu(...)
> >>> & co.
> >>
> >> These can be made OK by adding braces. Other than that the only way I
> >> found to make it not change the indentation is to add the comment "/*
> >> *INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.
> >
> > None of them looks really appealing because it means astyle will not 
> > correctly
> > indent if the user does not add braces or comments.
> >
> > Could astyle be easily modified to recognize foreach macros?
> 
>  Not that I'm aware of. If you don't want to manually annotate files
>  with unsupported macros then just exclude those files from astyle. I
>  wouldn't recommend adding this to the CI for all files, only for those
>  that their respective maintainers have confirmed to conform to the
>  style and want to enforce it going forward.
> >>>
> >>> So a couple use of an unsupported macros would make impossible to enforce 
> >>> the
> >>> coding style. This is not a very ideal position to be in.
> >>>
> >>> _if_ we are going to adopt astyle then we need to be able to enforce it 
> >>> on every
> >>> Xen files long-term. If it is not possible to do it with astyle, then 
> >>> maybe this
> >>> is not the right tool to use.
> >>>
> >>> For instance, I know that tools such as clang-format is able to deal with
> >>> foreach macros.
> >>
> >> If there are better tools then sure, I don't really mind using
> >> something else. I just don't have time to do the manual style check
> >> back-and-forth anymore, so the sooner we have something in place the
> >> better.
> >
> > I totally agree we need a tool so the reviewer can free-up some time to 
> > focus on
> > more important things. However, I think we should be careful on what we 
> > adopt here.
> >
> > Similar to Andrew, I am open with modifying the coding style to help the
> > automatic style check. But I am not happy to disable automatic style on 
> > part (or
> > entire) of files forever.
> >
> > At the moment, clang-format feels more powerful and there are people 
> > working on it.
>
> FYI, below a link to the clang-format changes:
>
> https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch
>

Thanks, I'll give this a shot. Since this requires patching clang it
looks to me like it may be a while before it's generally available
downstream. Perhaps it's still worth exploring adding .astylerc and
have at least partial coverage for the automated style checks for the
interim.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Rich Persaud]

On Jul 19, 2019, at 09:31, Julien Grall  wrote:
>> On 19/07/2019 14:24, Julien Grall wrote:
>> Hi Tamas,
>>> On 19/07/2019 14:14, Tamas K Lengyel wrote:
 On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:
 
 Hi Tamas,
 
> On 19/07/2019 14:00, Tamas K Lengyel wrote:
>> On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  
>> wrote:
>> 
>> Hi Tamas,
>> 
>> On 18/07/2019 18:48, Tamas K Lengyel wrote:
   - Line 1025: The tools needs to be able to deal 
 for_each_vcpu(...) & co.
>>> 
>>> These can be made OK by adding braces. Other than that the only way I
>>> found to make it not change the indentation is to add the comment "/*
>>> *INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.
>> 
>> None of them looks really appealing because it means astyle will not 
>> correctly
>> indent if the user does not add braces or comments.
>> 
>> Could astyle be easily modified to recognize foreach macros?
> 
> Not that I'm aware of. If you don't want to manually annotate files
> with unsupported macros then just exclude those files from astyle. I
> wouldn't recommend adding this to the CI for all files, only for those
> that their respective maintainers have confirmed to conform to the
> style and want to enforce it going forward.
 
 So a couple use of an unsupported macros would make impossible to enforce 
 the
 coding style. This is not a very ideal position to be in.
 
 _if_ we are going to adopt astyle then we need to be able to enforce it on 
 every
 Xen files long-term. If it is not possible to do it with astyle, then 
 maybe this
 is not the right tool to use.
 
 For instance, I know that tools such as clang-format is able to deal with
 foreach macros.
>>> 
>>> If there are better tools then sure, I don't really mind using
>>> something else. I just don't have time to do the manual style check
>>> back-and-forth anymore, so the sooner we have something in place the
>>> better.
>> I totally agree we need a tool so the reviewer can free-up some time to 
>> focus on more important things. However, I think we should be careful on 
>> what we adopt here.
>> Similar to Andrew, I am open with modifying the coding style to help the 
>> automatic style check. But I am not happy to disable automatic style on part 
>> (or entire) of files forever.
>> At the moment, clang-format feels more powerful and there are people working 
>> on it.
> 
> FYI, below a link to the clang-format changes:
> 
> https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch

Were these clang-format changes done for FuSa work?  Are they intended to be 
run within OSStest and/or Xen's Gitlab CI, which do not currently support 
OpenEmbedded/Yocto and xen-troops?

It would be helpful to have a xen-devel thread on the motivation for the 
clang-format work, the specific style being enforced (including the nuances 
discussed in this thread) and additional work needed before clang-format can 
perform automated style checking to address (a) existing Xen/Linux style 
requirements, (b) FuSa requirements.

Rich
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] golang/xenlight: Add libxl_utils support

[George Dunlap]

On 7/19/19 2:19 PM, Nicolas Belouin wrote:
> 
> 
> On 7/19/19 1:04 PM, George Dunlap wrote:
>> On 7/19/19 11:24 AM, Nicolas Belouin wrote:
>>>
>>> On 7/19/19 12:09 PM, George Dunlap wrote:
 On 7/19/19 11:03 AM, Nicolas Belouin wrote:
> On 7/19/19 10:50 AM, George Dunlap wrote:
>>> On Jul 19, 2019, at 9:47 AM, George Dunlap  
>>> wrote:
>>>
>>>
>>>
 On Jul 19, 2019, at 8:34 AM, Nicolas Belouin 
  wrote:



 On 7/18/19 11:54 PM, George Dunlap wrote:
> The Go bindings for libxl miss functions from libxl_utils, let's start
> with the simple libxl_domid_to_name and its counterpart
> libxl_name_to_domid.
>
> NB that C.GoString() will return "" if it's passed a NULL; see
> https://github.com/golang/go/issues/32734#issuecomment-506835432
>
> Signed-off-by: Nicolas Belouin 
> Signed-off-by: George Dunlap 
> ---
> v3:
> - Wire into build system
> - Add reference to C.GoString() handling NULL to commit message
>
> Nicolas, could you test to see if this actually works for you?
 Tested it, it works.

 I must confess I do not use that import path as the new modules 
 mechanism
 introduced in Go1.11 downloads and compile a versioned copy of every
 dependency per project, and this behavior is incompatible with the 
 build
 system used here.
>>> It’s possible that something fundamentally has changed, but I suspect 
>>> that rather you don’t quite understand how the current build system is 
>>> supposed to work.  (In which case a write-up in the tree would probably 
>>> be useful.)
>>>
>>> Go has always insisted that there be no binary compatibility between 
>>> versions; so it’s always been necessary to re-compile all your 
>>> libraries when upgrading from (say) 1.8 to 1.9.  Which means that any 
>>> useable distribution must also include all the source files necessary 
>>> to recompile when you bump the version number.
>>>
>>> So the core mechanism of the “install” is actually to copy all the 
>>> source files necessary into the right local directory such that the go 
>>> compiler can find them; ATM this is 
>>> /usr/share/gocode/golang.xenproject.org/xenlight
>> Nit:  This of course should have a `src/` between `gocode/` and 
>> `golang.xenproject.org/`.
>>
>> NB also that this naming scheme was designed so that at some point in 
>> the future, we could actually host the xenlight packages at the URL 
>> provided.
>>
>>  -George
>>
> This new mechanism of modules is described here:
> https://golang.org/cmd/go/#hdr-Modules__module_versions__and_more
>
> The module system is intended to supersede the GOPATH approach and
> provide a way to get versioned dependencies, as such
> it does not rely on GOPATH at all and doesn't use sources or compiled
> packages present in GOPATH elements such as /usr/share/gocode
> and systematically fetch (at the asked version) and compile a copy of
> the dependency as it might be a different version from the one
> in GOPATH.
>
> As far as I tried, I have been unable to build my module even with the
> library installed.
> I have to use xenbits.xen.org/git-http/xen.git/tools/golang/xenlight (or
> one of its mirror) in order to build the module using the new
> mechanism
>> This will break as soon as we have support in golang/xenlight for libxl
>> features not in the version of Xen you're using.
>>
>> E.g., suppose you're on Xen 4.12.  Someone introduces a new feature in
>> Xen 4.13, and plumbs it all the way from Xen, through libxl, *and*
>> golang/xenlight.  Now when *you* do a build, it will fail, because your
>> version of golang will expect libxl features which your system doesn't have.
> I know of that, and that can be overcome using modules as you can
> specify a branch version of the module you depends on (e.g you can set
> your dependency as being xxx/xenlight@stable-4.12).

That's not terrible I guess.

>> I had always planned on getting golang.xenproject.org set up such that
>> it could interact with the "normal" go get thing.  If you want to help
>> us figure out how to get that set up, that would be helpful.
> 
> As far as I looked into vanity URLs, you can't serve a subdirectory of a
> repository directly, but you can trick the system using a go-proxy.
> To do that you need two things. First, you need
> https://golang.xenproject.org/xenlight?go-get=1
>  to point to a page containing a
>     https://golang.xenproject.org/moduleproxy ">
> And have golang.xenproject.org/moduleproxy to follow the specifications
> of module proxies by proposing '.zip' files containing the different
> versions of the module. Th

Re: [Xen-devel] [PATCH] include/public/memory.h: remove the XENMEM_rsrc_acq_caller_owned flag

[Andrew Cooper]

On 19/07/2019 14:31, Jan Beulich wrote:
> On 19.07.2019 14:41, Andrew Cooper wrote:
>> On 19/07/2019 13:25, Paul Durrant wrote:
>>> When commit 3f8f1228 "x86/mm: add HYPERVISOR_memory_op to acquire guest
>>> resources" introduced the concept of directly mapping some guest resources,
>>> it was envisaged that the memory for some resources associated with a guest
>>> may not actually be assigned to that guest, specifically the IOREQ server
>>> resource introduces in commit 6e387461 "x86/hvm/ioreq: add a new mappable
>>> resource type...". Such resources were dubbed "caller owned" and resulted
>>> in the owned resources" and acquiring them resulted in the
>>> XENMEM_rsrc_acq_caller_owned flag being passed back to the caller of the
>>> memory op.
>>>
>>> Unfortunately the implementation led to XSA-276, which was mitigated
>>> by commit f6b6ae78 "x86/hvm/ioreq: fix page referencing" and then a related
>>> memory accounting problem was worked around by commit e862e6ce
>>> "x86/hvm/ioreq: use ref-counted target-assigned shared pages". This latter
>>> commit removed the only instance of a "caller owned" resource, but the
>>> flag was left in header and checked in one place in the core code.
>>> This patch removes that now redundant check and removes the definition of
>>> XENMEM_rsrc_acq_caller_owned from the public header. Also, since this was
>>> the only flag defined for the XENMEM_acquire_resource memory op, it removes
>>> the 'flags' field of struct xen_mem_acquire_resource and replaces it with
>>> an equivalently sized 'pad' field.
>>>
>>> Signed-off-by: Paul Durrant 
>> This is a modification to a public header, but in this specific case, I
>> think it is the correct thing to do.
> This is in a large "#if defined(__XEN__) || defined(__XEN_TOOLS__)" section,
> and we consider public interface parts in such sections volatile anyway.

Oh - even better.  I'd not spotted that.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

[Jan Beulich]

On 19.07.2019 15:30, Razvan Cojocaru wrote:
> On 7/19/19 4:18 PM, Jan Beulich wrote:
>> On 19.07.2019 14:34, Alexandru Stefan ISAILA wrote:
>>> On 18.07.2019 15:58, Jan Beulich wrote:
 On 03.07.2019 12:56, Alexandru Stefan ISAILA wrote:
> A/D bit writes (on page walks) can be considered benign by an 
> introspection
> agent, so receiving vm_events for them is a pessimization. We try here to
> optimize by fitering these events out.

 But you add the sending of more events - how does "filter out" match
 the actual implementation?
>>>
>>> The events are send only if there is a mem access violation therefore we
>>> are filtering and only sending the events that are interesting to
>>> introspection.
>>
>> Where is it that you prevent any event from being sent? As said,
>> reading the patch I only see new sending sites to get added.
> 
> If we don't emulate, we would receive the page-walk-generated events
> _and_ the touching-the-page-the-instruction-is-touching events.

Since the patch here alters emulation paths only, how do you know
whether to emulate? In order to not receive undue events it would
seem to me that you'd first have to intercept the guest on insns
of interest ... Overall I think that the patch description, while
it has improved, is still lacking sufficient information for a
person like me (not knowing much about your monitor tools) to be
able to sensibly review this (which includes understanding the
precise scenario you want to improve).

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Tamas K Lengyel]

On Fri, Jul 19, 2019 at 7:34 AM Julien Grall  wrote:
>
> Hi Tamas,
>
> On 19/07/2019 14:05, Tamas K Lengyel wrote:
> > On Fri, Jul 19, 2019 at 3:03 AM Julien Grall  wrote:
> >>
> >> Hi,
> >>
> >> On 18/07/2019 19:34, Tamas K Lengyel wrote:
> >>> On Thu, Jul 18, 2019 at 11:59 AM Andrew Cooper
> >>>  wrote:
> 
>  On 18/07/2019 15:43, Tamas K Lengyel wrote:
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 6cc5b774cf..0b37f7ae4d 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -60,8 +60,8 @@ Bracing
> >---
> >
> >Braces ('{' and '}') are usually placed on a line of their own, 
> > except
> > -for the do/while loop.  This is unlike the Linux coding style and
> > -unlike K&R.  do/while loops are an exception. e.g.:
> > +for the while-part of do/while loops.  This is unlike the Linux coding 
> > style
> > +and unlike K&R.  do/while loops are an exception. e.g.:
> >
> >if ( condition )
> >{
> > @@ -77,7 +77,8 @@ while ( condition )
> >/* Do stuff. */
> >}
> >
> > -do {
> > +do
> > +{
> 
>  I'd happily take this adjustment to Xen's style if it helps us end up
>  with auto-formatter.
> >>>
> >>> Yay!
> >>>
> 
>  Also, there are a number of files which are technically Linux style, but
>  have totally diverged from Linux, and would be easier to move to Xen 
>  style.
> 
>  Do you have an updated .astylerc based on Julien's observations?
> >>>
> >>> Yes, this is it:
> >>>
> >>> style=bsd
> >>> suffix=none
> >>> align-pointer=name
> >>> align-reference=name
> >>> indent=spaces=4
> >>> max-code-length=80
> >>> min-conditional-indent=0
> >>> max-continuation-indent=78
> >>> attach-closing-while
> >>> remove-braces
> >>> break-one-line-headers
> >>> pad-comma
> >>> pad-header
> >>
> >> Unfortunately this style does not work with the astyle provided by Debian 
> >> Stretch:
> >>
> >> 42sh> astyle xen/arch/arm/setup.c
> >> Invalid option file options:
> >> max-continuation-indent=78
> >> attach-closing-while
> >> remove-braces
> >> For help on options type 'astyle -h'
> >>
> >> Artistic Style has terminated
> >
> > I'm already on buster and it works fine. Perhaps we'll need to specify
> > a minimum version of astyle.
>
> That would be good.
>
> >
> >>
> >> Also, I needed to copy the .astylerc in $(HOME) in order to use the style. 
> >> It is
> >> probably worth considering implementing a wrapper that set
> >> ARTISTIC_STYLE_OPTIONS and call astyle to keep everthing in Xen internals.
> >
> > You don't have to copy to to $(HOME), as I point out by the addition
> > to the CODING_STYLE this works from the Xen root folder:
> >
> > export ARTISTIC_STYLE_OPTIONS=".astylerc"
> > astyle 
>
> I misread your first e-mail. Sorry for the noise.
>
> >
> >>
> >>>
> >>> With this it's down to 860 files that are formatted.
> >>
> >> Do you mind providing the new diff?
> >
> > I've updated the same gist with the new diff, the url is the same:
> > https://gist.github.com/tklengyel/c5cac14a0d57f119dd7747a1be6fb260
>
> Do you mind to create a new gist everytime? This would help to see the
> difference before and after at least until I build a new version of astyle :).

Sure, no problem. Also a tip: just enable the buster repo and do
"apt-get -t buster install astyle" ;)

Cheers,
Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 00/60] xen: add core scheduling support

[Juergen Gross]

On 16.07.19 17:45, Sergey Dyasli wrote:

On 05/07/2019 14:17, Sergey Dyasli wrote:

[2019-07-05 00:37:16 UTC] (XEN) [24907.482686] Watchdog timer detects that 
CPU30 is stuck!
[2019-07-05 00:37:16 UTC] (XEN) [24907.514180] [ Xen-4.13.0-8.0.6-d  x86_64 
 debug=y   Not tainted ]
[2019-07-05 00:37:16 UTC] (XEN) [24907.552070] CPU:30
[2019-07-05 00:37:16 UTC] (XEN) [24907.565281] RIP:
e008:[] sched_context_switched+0xaf/0x101
[2019-07-05 00:37:16 UTC] (XEN) [24907.601232] RFLAGS: 0202   
CONTEXT: hypervisor
[2019-07-05 00:37:16 UTC] (XEN) [24907.629998] rax: 0002   rbx: 
83202782e880   rcx: 001e
[2019-07-05 00:37:16 UTC] (XEN) [24907.669651] rdx: 83202782e904   rsi: 
832027823000   rdi: 832027823000
[2019-07-05 00:37:16 UTC] (XEN) [24907.706560] rbp: 83403cab7d20   rsp: 
83403cab7d00   r8:  
[2019-07-05 00:37:16 UTC] (XEN) [24907.743258] r9:     r10: 
0200200200200200   r11: 0100100100100100
[2019-07-05 00:37:16 UTC] (XEN) [24907.779940] r12: 832027823000   r13: 
832027823000   r14: 83202782e7b0
[2019-07-05 00:37:16 UTC] (XEN) [24907.816849] r15: 83202782e880   cr0: 
8005003b   cr4: 000426e0
[2019-07-05 00:37:16 UTC] (XEN) [24907.854125] cr3: bd8a1000   cr2: 
1851b798
[2019-07-05 00:37:16 UTC] (XEN) [24907.881483] fsb:    gsb: 
   gss: 
[2019-07-05 00:37:16 UTC] (XEN) [24907.918309] ds:    es:    fs:    
gs:    ss:    cs: e008
[2019-07-05 00:37:16 UTC] (XEN) [24907.952619] Xen code around 
 (sched_context_switched+0xaf/0x101):
[2019-07-05 00:37:16 UTC] (XEN) [24907.990277]  00 00 eb 18 f3 90 8b 02 <85> c0 
75 f8 eb 0e 49 8b 7e 30 48 85 ff 74 05 e8
[2019-07-05 00:37:16 UTC] (XEN) [24908.032393] Xen stack trace from 
rsp=83403cab7d00:
[2019-07-05 00:37:16 UTC] (XEN) [24908.061298]832027823000 
832027823000  83202782e880
[2019-07-05 00:37:16 UTC] (XEN) [24908.098529]83403cab7d60 
82d0802407c0 0082 83202782e7c8
[2019-07-05 00:37:16 UTC] (XEN) [24908.135622]001e 
83202782e7c8 001e 82d080602628
[2019-07-05 00:37:16 UTC] (XEN) [24908.172671]83403cab7dc0 
82d080240d83 df99 001e
[2019-07-05 00:37:16 UTC] (XEN) [24908.210212]832027823000 
16a62dc8c6bc 00fc 001e
[2019-07-05 00:37:16 UTC] (XEN) [24908.247181]83202782e7c8 
82d080602628 82d0805da460 001e
[2019-07-05 00:37:16 UTC] (XEN) [24908.284279]83403cab7e60 
82d080240ea4 0002802aecc5 832027823000
[2019-07-05 00:37:16 UTC] (XEN) [24908.321128]83202782e7b0 
83202782e880 83403cab7e10 82d080273b4e
[2019-07-05 00:37:16 UTC] (XEN) [24908.358308]83403cab7e10 
82d080242f7f 83403cab7e60 82d08024663a
[2019-07-05 00:37:17 UTC] (XEN) [24908.395662]83403cab7ea0 
82d0802ec32a 834000ff 82d0805bc880
[2019-07-05 00:37:17 UTC] (XEN) [24908.432376]82d0805bb980 
 83403cab7fff 001e
[2019-07-05 00:37:17 UTC] (XEN) [24908.469812]83403cab7e90 
82d080242575 0f00 82d0805bb980
[2019-07-05 00:37:17 UTC] (XEN) [24908.508373]001e 
82d0806026f0 83403cab7ea0 82d0802425ca
[2019-07-05 00:37:17 UTC] (XEN) [24908.549856]83403cab7ef0 
82d08027a601 82d080242575 001e7ffde000
[2019-07-05 00:37:17 UTC] (XEN) [24908.588022]832027823000 
832027823000 83127ffde000 83203ffe5000
[2019-07-05 00:37:17 UTC] (XEN) [24908.625217]001e 
831204092000 83403cab7d78 ffed
[2019-07-05 00:37:17 UTC] (XEN) [24908.662932]8180 
 8180 
[2019-07-05 00:37:17 UTC] (XEN) [24908.703246]818f4580 
880039118848 0e6a3c4b2698 148900db
[2019-07-05 00:37:17 UTC] (XEN) [24908.743671] 
8101e650 8185c3e0 
[2019-07-05 00:37:17 UTC] (XEN) [24908.781927] 
 beefbeef 81054eb2
[2019-07-05 00:37:17 UTC] (XEN) [24908.820986] Xen call trace:
[2019-07-05 00:37:17 UTC] (XEN) [24908.836789][] 
sched_context_switched+0xaf/0x101
[2019-07-05 00:37:17 UTC] (XEN) [24908.869916][] 
schedule.c#sched_context_switch+0x72/0x151
[2019-07-05 00:37:17 UTC] (XEN) [24908.907384][] 
schedule.c#sched_slave+0x2a3/0x2b2
[2019-07-05 00:37:17 UTC] (XEN) [24908.941241][] 
schedule.c#schedule+0x112/0x2a1
[2019-07-05 00:37:17 UTC] (XEN) [24908.973939][] 
softirq.c#__do_softirq+0x85/0x90
[2019-07-05 00:37:17 UTC] (XEN) [24909.007101][] 
do_softirq+0x13/0x15
[2019-07-05 00:37:17 UTC] (XEN) [24909.035971][] 
domain.c#idle_loop+0xad/0xc0
[2019-07-05 00:37:17 UTC] (XEN) [24909.070546]
[2019-07-05 00:37:

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Julien Grall]

Hi Tamas,

On 19/07/2019 14:05, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 3:03 AM Julien Grall  wrote:


Hi,

On 18/07/2019 19:34, Tamas K Lengyel wrote:

On Thu, Jul 18, 2019 at 11:59 AM Andrew Cooper
 wrote:


On 18/07/2019 15:43, Tamas K Lengyel wrote:

diff --git a/CODING_STYLE b/CODING_STYLE
index 6cc5b774cf..0b37f7ae4d 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -60,8 +60,8 @@ Bracing
   ---

   Braces ('{' and '}') are usually placed on a line of their own, except
-for the do/while loop.  This is unlike the Linux coding style and
-unlike K&R.  do/while loops are an exception. e.g.:
+for the while-part of do/while loops.  This is unlike the Linux coding style
+and unlike K&R.  do/while loops are an exception. e.g.:

   if ( condition )
   {
@@ -77,7 +77,8 @@ while ( condition )
   /* Do stuff. */
   }

-do {
+do
+{


I'd happily take this adjustment to Xen's style if it helps us end up
with auto-formatter.


Yay!



Also, there are a number of files which are technically Linux style, but
have totally diverged from Linux, and would be easier to move to Xen style.

Do you have an updated .astylerc based on Julien's observations?


Yes, this is it:

style=bsd
suffix=none
align-pointer=name
align-reference=name
indent=spaces=4
max-code-length=80
min-conditional-indent=0
max-continuation-indent=78
attach-closing-while
remove-braces
break-one-line-headers
pad-comma
pad-header


Unfortunately this style does not work with the astyle provided by Debian 
Stretch:

42sh> astyle xen/arch/arm/setup.c
Invalid option file options:
max-continuation-indent=78
attach-closing-while
remove-braces
For help on options type 'astyle -h'

Artistic Style has terminated


I'm already on buster and it works fine. Perhaps we'll need to specify
a minimum version of astyle.


That would be good.





Also, I needed to copy the .astylerc in $(HOME) in order to use the style. It is
probably worth considering implementing a wrapper that set
ARTISTIC_STYLE_OPTIONS and call astyle to keep everthing in Xen internals.


You don't have to copy to to $(HOME), as I point out by the addition
to the CODING_STYLE this works from the Xen root folder:

export ARTISTIC_STYLE_OPTIONS=".astylerc"
astyle 


I misread your first e-mail. Sorry for the noise.







With this it's down to 860 files that are formatted.


Do you mind providing the new diff?


I've updated the same gist with the new diff, the url is the same:
https://gist.github.com/tklengyel/c5cac14a0d57f119dd7747a1be6fb260


Do you mind to create a new gist everytime? This would help to see the 
difference before and after at least until I build a new version of astyle :).


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] include/public/memory.h: remove the XENMEM_rsrc_acq_caller_owned flag

[Jan Beulich]

On 19.07.2019 14:41, Andrew Cooper wrote:
> On 19/07/2019 13:25, Paul Durrant wrote:
>> When commit 3f8f1228 "x86/mm: add HYPERVISOR_memory_op to acquire guest
>> resources" introduced the concept of directly mapping some guest resources,
>> it was envisaged that the memory for some resources associated with a guest
>> may not actually be assigned to that guest, specifically the IOREQ server
>> resource introduces in commit 6e387461 "x86/hvm/ioreq: add a new mappable
>> resource type...". Such resources were dubbed "caller owned" and resulted
>> in the owned resources" and acquiring them resulted in the
>> XENMEM_rsrc_acq_caller_owned flag being passed back to the caller of the
>> memory op.
>>
>> Unfortunately the implementation led to XSA-276, which was mitigated
>> by commit f6b6ae78 "x86/hvm/ioreq: fix page referencing" and then a related
>> memory accounting problem was worked around by commit e862e6ce
>> "x86/hvm/ioreq: use ref-counted target-assigned shared pages". This latter
>> commit removed the only instance of a "caller owned" resource, but the
>> flag was left in header and checked in one place in the core code.
>> This patch removes that now redundant check and removes the definition of
>> XENMEM_rsrc_acq_caller_owned from the public header. Also, since this was
>> the only flag defined for the XENMEM_acquire_resource memory op, it removes
>> the 'flags' field of struct xen_mem_acquire_resource and replaces it with
>> an equivalently sized 'pad' field.
>>
>> Signed-off-by: Paul Durrant 
> 
> This is a modification to a public header, but in this specific case, I
> think it is the correct thing to do.

This is in a large "#if defined(__XEN__) || defined(__XEN_TOOLS__)" section,
and we consider public interface parts in such sections volatile anyway.

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Julien Grall]

On 19/07/2019 14:24, Julien Grall wrote:

Hi Tamas,

On 19/07/2019 14:14, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:


Hi Tamas,

On 19/07/2019 14:00, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  wrote:


Hi Tamas,

On 18/07/2019 18:48, Tamas K Lengyel wrote:
  - Line 1025: The tools needs to be able to deal for_each_vcpu(...) 
& co.


These can be made OK by adding braces. Other than that the only way I
found to make it not change the indentation is to add the comment "/*
*INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.


None of them looks really appealing because it means astyle will not correctly
indent if the user does not add braces or comments.

Could astyle be easily modified to recognize foreach macros?


Not that I'm aware of. If you don't want to manually annotate files
with unsupported macros then just exclude those files from astyle. I
wouldn't recommend adding this to the CI for all files, only for those
that their respective maintainers have confirmed to conform to the
style and want to enforce it going forward.


So a couple use of an unsupported macros would make impossible to enforce the
coding style. This is not a very ideal position to be in.

_if_ we are going to adopt astyle then we need to be able to enforce it on every
Xen files long-term. If it is not possible to do it with astyle, then maybe this
is not the right tool to use.

For instance, I know that tools such as clang-format is able to deal with
foreach macros.


If there are better tools then sure, I don't really mind using
something else. I just don't have time to do the manual style check
back-and-forth anymore, so the sooner we have something in place the
better.


I totally agree we need a tool so the reviewer can free-up some time to focus on 
more important things. However, I think we should be careful on what we adopt here.


Similar to Andrew, I am open with modifying the coding style to help the 
automatic style check. But I am not happy to disable automatic style on part (or 
entire) of files forever.


At the moment, clang-format feels more powerful and there are people working on 
it.


FYI, below a link to the clang-format changes:

https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

[Razvan Cojocaru]

On 7/19/19 4:18 PM, Jan Beulich wrote:

On 19.07.2019 14:34, Alexandru Stefan ISAILA wrote:

On 18.07.2019 15:58, Jan Beulich wrote:

On 03.07.2019 12:56, Alexandru Stefan ISAILA wrote:

A/D bit writes (on page walks) can be considered benign by an introspection
agent, so receiving vm_events for them is a pessimization. We try here to
optimize by fitering these events out.


But you add the sending of more events - how does "filter out" match
the actual implementation?


The events are send only if there is a mem access violation therefore we
are filtering and only sending the events that are interesting to
introspection.


Where is it that you prevent any event from being sent? As said,
reading the patch I only see new sending sites to get added.


If we don't emulate, we would receive the page-walk-generated events 
_and_ the touching-the-page-the-instruction-is-touching events.


In comparison to the "hardware" case, the case where we emulate the 
instruction with the page walk ignoring the EPT results in less events, 
hence the prevention of some events being sent out.



Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2 RESEND] CODING_STYLE: explicitly call out label indentation

[Tamas K Lengyel]

On Fri, Jul 19, 2019 at 7:22 AM Jan Beulich  wrote:
>
> On 19.07.2019 15:18, Tamas K Lengyel wrote:
> > On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich  wrote:
> >>
> >> Since the behavior of "diff -p" to use an unindented label as context
> >> identifier often makes it harder to review patches, make explicit the
> >> requirement for labels to be indented.
> >>
> >> Signed-off-by: Jan Beulich 
> >
> > This style requirement wouldn't really work with astyle as-is.
>
> Personally I view proper "diff -p" context in patches as quite
> a bit more important than automatic style checking. But perhaps
> that's just because I do quite a lot of patch review ...

Which is a valid point. I don't really care which style option it
really is, if it helps you, I'm fine with it. But as a maintainer who
does this on a volunteer basis when I have a 5-minute window, I'm not
going to spend my time looking for style issues. So in the ongoing
vm_event series that's under review where you explicitly called out
that the vm_event maintainers have to review and point out all style
issues, that's a no-go from my side. So either we have automatic style
checks or I'm just going to Ack patches with style issues.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 06/13] x86/IOMMU: don't restrict IRQ affinities to online CPUs

[Andrew Cooper]

On 16/07/2019 08:40, Jan Beulich wrote:
> In line with "x86/IRQ: desc->affinity should strictly represent the
> requested value" the internally used IRQ(s) also shouldn't be restricted
> to online ones. Make set_desc_affinity() (set_msi_affinity() then does
> by implication) cope with a NULL mask being passed (just like
> assign_irq_vector() does), and have IOMMU code pass NULL instead of
> &cpu_online_map (when, for VT-d, there's no NUMA node information
> available).
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Rich Persaud]

> On Jul 18, 2019, at 10:43, Tamas K Lengyel  wrote:
> 
> Using astyle (http://astyle.sourceforge.net) can greatly reduce the overhead 
> of
> manually checking and applying style-fixes to source-code. The included
> .astylerc is the closest approximation of the established Xen style (including
> styles not formally spelled out by CODING_STYLE but commonly requested).
> 
> Checking the comment styles are not included in the automation.
> 
> Incorporating Xen's exception to the do-while style is only partially 
> possible,
> thus a change is proposed to the CODING_STYLE of moving the brace from "do {"
> to the next line.
> 
> Most of Xen's code-base is non-conforming at the moment: 289 files pass
> unchanged, 876 have some style issues.
> 
> Ideally we can slowly migrate the entire code-base to be conforming, thus
> eliminating the need of discussing and enforcing style issues manually on the
> mailinglist.

Thanks for taking the lead on this, Tamas.  New Xen contributors are unlikely 
to be aware of the style ambiguities discussed in this thread. A frequent topic 
on Xen monthly calls is the lack of time to perform patch reviews.  Automated 
style checking will increase the S/N ratio of xen-devel, reviewer efficiency, 
patch quality from new contributors, and style consistency across Xen trees.  
This automation will complement upcoming static analysis of Xen for functional 
safety compliance.

Rich
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 03/13] x86/IRQ: desc->affinity should strictly represent the requested value

[Andrew Cooper]

On 16/07/2019 08:38, Jan Beulich wrote:
> desc->arch.cpu_mask reflects the actual set of target CPUs. Don't ever
> fiddle with desc->affinity itself, except to store caller requested
> values. Note that assign_irq_vector() now takes a NULL incoming CPU mask
> to mean "all CPUs" now, rather than just "all currently online CPUs".
> This way no further affinity adjustment is needed after onlining further
> CPUs.
>
> This renders both set_native_irq_info() uses (which weren't using proper
> locking anyway) redundant - drop the function altogether.
>
> Signed-off-by: Jan Beulich 
> Reviewed-by: Roger Pau Monné 

Acked-by: Andrew Cooper 

There are utf8 encoding problems here, but the patch in 0/$N does look
to be ok.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Julien Grall]

Hi Tamas,

On 19/07/2019 14:14, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:


Hi Tamas,

On 19/07/2019 14:00, Tamas K Lengyel wrote:

On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  wrote:


Hi Tamas,

On 18/07/2019 18:48, Tamas K Lengyel wrote:

  - Line 1025: The tools needs to be able to deal for_each_vcpu(...) & co.


These can be made OK by adding braces. Other than that the only way I
found to make it not change the indentation is to add the comment "/*
*INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.


None of them looks really appealing because it means astyle will not correctly
indent if the user does not add braces or comments.

Could astyle be easily modified to recognize foreach macros?


Not that I'm aware of. If you don't want to manually annotate files
with unsupported macros then just exclude those files from astyle. I
wouldn't recommend adding this to the CI for all files, only for those
that their respective maintainers have confirmed to conform to the
style and want to enforce it going forward.


So a couple use of an unsupported macros would make impossible to enforce the
coding style. This is not a very ideal position to be in.

_if_ we are going to adopt astyle then we need to be able to enforce it on every
Xen files long-term. If it is not possible to do it with astyle, then maybe this
is not the right tool to use.

For instance, I know that tools such as clang-format is able to deal with
foreach macros.


If there are better tools then sure, I don't really mind using
something else. I just don't have time to do the manual style check
back-and-forth anymore, so the sooner we have something in place the
better.


I totally agree we need a tool so the reviewer can free-up some time to focus on 
more important things. However, I think we should be careful on what we adopt here.


Similar to Andrew, I am open with modifying the coding style to help the 
automatic style check. But I am not happy to disable automatic style on part (or 
entire) of files forever.


At the moment, clang-format feels more powerful and there are people working on 
it.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2 RESEND] CODING_STYLE: explicitly call out label indentation

[Jan Beulich]

On 19.07.2019 15:18, Tamas K Lengyel wrote:
> On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich  wrote:
>>
>> Since the behavior of "diff -p" to use an unindented label as context
>> identifier often makes it harder to review patches, make explicit the
>> requirement for labels to be indented.
>>
>> Signed-off-by: Jan Beulich 
> 
> This style requirement wouldn't really work with astyle as-is.

Personally I view proper "diff -p" context in patches as quite
a bit more important than automatic style checking. But perhaps
that's just because I do quite a lot of patch review ...

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 01/13] x86/IRQ: deal with move-in-progress state in fixup_irqs()

[Andrew Cooper]

On 16/07/2019 08:37, Jan Beulich wrote:
> The flag being set may prevent affinity changes, as these often imply
> assignment of a new vector. When there's no possible destination left
> for the IRQ, the clearing of the flag needs to happen right from
> fixup_irqs().
>
> Additionally _assign_irq_vector() needs to avoid setting the flag when
> there's no online CPU left in what gets put into ->arch.old_cpu_mask.
> The old vector can be released right away in this case.
>
> Also extend the log message about broken affinity to include the new
> affinity as well, allowing to notice issues with affinity changes not
> actually having taken place. Swap the if/else-if order there at the
> same time to reduce the amount of conditions checked.
>
> At the same time replace two open coded instances of the new helper
> function.
>
> Signed-off-by: Jan Beulich 
> Reviewed-by: Roger Pau Monné 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2 RESEND] CODING_STYLE: explicitly call out label indentation

[Tamas K Lengyel]

On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich  wrote:
>
> Since the behavior of "diff -p" to use an unindented label as context
> identifier often makes it harder to review patches, make explicit the
> requirement for labels to be indented.
>
> Signed-off-by: Jan Beulich 

This style requirement wouldn't really work with astyle as-is.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] golang/xenlight: Add libxl_utils support

[Nicolas Belouin]

On 7/19/19 1:04 PM, George Dunlap wrote:
> On 7/19/19 11:24 AM, Nicolas Belouin wrote:
>>
>> On 7/19/19 12:09 PM, George Dunlap wrote:
>>> On 7/19/19 11:03 AM, Nicolas Belouin wrote:
 On 7/19/19 10:50 AM, George Dunlap wrote:
>> On Jul 19, 2019, at 9:47 AM, George Dunlap  
>> wrote:
>>
>>
>>
>>> On Jul 19, 2019, at 8:34 AM, Nicolas Belouin 
>>>  wrote:
>>>
>>>
>>>
>>> On 7/18/19 11:54 PM, George Dunlap wrote:
 The Go bindings for libxl miss functions from libxl_utils, let's start
 with the simple libxl_domid_to_name and its counterpart
 libxl_name_to_domid.

 NB that C.GoString() will return "" if it's passed a NULL; see
 https://github.com/golang/go/issues/32734#issuecomment-506835432

 Signed-off-by: Nicolas Belouin 
 Signed-off-by: George Dunlap 
 ---
 v3:
 - Wire into build system
 - Add reference to C.GoString() handling NULL to commit message

 Nicolas, could you test to see if this actually works for you?
>>> Tested it, it works.
>>>
>>> I must confess I do not use that import path as the new modules 
>>> mechanism
>>> introduced in Go1.11 downloads and compile a versioned copy of every
>>> dependency per project, and this behavior is incompatible with the build
>>> system used here.
>> It’s possible that something fundamentally has changed, but I suspect 
>> that rather you don’t quite understand how the current build system is 
>> supposed to work.  (In which case a write-up in the tree would probably 
>> be useful.)
>>
>> Go has always insisted that there be no binary compatibility between 
>> versions; so it’s always been necessary to re-compile all your libraries 
>> when upgrading from (say) 1.8 to 1.9.  Which means that any useable 
>> distribution must also include all the source files necessary to 
>> recompile when you bump the version number.
>>
>> So the core mechanism of the “install” is actually to copy all the 
>> source files necessary into the right local directory such that the go 
>> compiler can find them; ATM this is 
>> /usr/share/gocode/golang.xenproject.org/xenlight
> Nit:  This of course should have a `src/` between `gocode/` and 
> `golang.xenproject.org/`.
>
> NB also that this naming scheme was designed so that at some point in the 
> future, we could actually host the xenlight packages at the URL provided.
>
>  -George
>
 This new mechanism of modules is described here:
 https://golang.org/cmd/go/#hdr-Modules__module_versions__and_more

 The module system is intended to supersede the GOPATH approach and
 provide a way to get versioned dependencies, as such
 it does not rely on GOPATH at all and doesn't use sources or compiled
 packages present in GOPATH elements such as /usr/share/gocode
 and systematically fetch (at the asked version) and compile a copy of
 the dependency as it might be a different version from the one
 in GOPATH.

 As far as I tried, I have been unable to build my module even with the
 library installed.
 I have to use xenbits.xen.org/git-http/xen.git/tools/golang/xenlight (or
 one of its mirror) in order to build the module using the new
 mechanism
> This will break as soon as we have support in golang/xenlight for libxl
> features not in the version of Xen you're using.
>
> E.g., suppose you're on Xen 4.12.  Someone introduces a new feature in
> Xen 4.13, and plumbs it all the way from Xen, through libxl, *and*
> golang/xenlight.  Now when *you* do a build, it will fail, because your
> version of golang will expect libxl features which your system doesn't have.
I know of that, and that can be overcome using modules as you can
specify a branch version of the module you depends on (e.g you can set
your dependency as being xxx/xenlight@stable-4.12).
>
> I had always planned on getting golang.xenproject.org set up such that
> it could interact with the "normal" go get thing.  If you want to help
> us figure out how to get that set up, that would be helpful.

As far as I looked into vanity URLs, you can't serve a subdirectory of a
repository directly, but you can trick the system using a go-proxy.
To do that you need two things. First, you need
https://golang.xenproject.org/xenlight?go-get=1
 to point to a page containing a
    https://golang.xenproject.org/moduleproxy ">
And have golang.xenproject.org/moduleproxy to follow the specifications
of module proxies by proposing '.zip' files containing the different
versions of the module. This part for sure can be scripted to do the
hard work of dynamically packaging the versions from the git repository
on demand.


>
> What would be *really* ideal is if we didn't have to link golang against
> on

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

[Jan Beulich]

On 19.07.2019 14:34, Alexandru Stefan ISAILA wrote:
> On 18.07.2019 15:58, Jan Beulich wrote:
>> On 03.07.2019 12:56, Alexandru Stefan ISAILA wrote:
>>> A/D bit writes (on page walks) can be considered benign by an introspection
>>> agent, so receiving vm_events for them is a pessimization. We try here to
>>> optimize by fitering these events out.
>>
>> But you add the sending of more events - how does "filter out" match
>> the actual implementation?
> 
> The events are send only if there is a mem access violation therefore we
> are filtering and only sending the events that are interesting to
> introspection.

Where is it that you prevent any event from being sent? As said,
reading the patch I only see new sending sites to get added.

>>> Currently, we are fully emulating the instruction at RIP when the hardware 
>>> sees
>>> an EPT fault with npfec.kind != npfec_kind_with_gla. This is, however,
>>> incorrect, because the instruction at RIP might legitimately cause an
>>> EPT fault of its own while accessing a _different_ page from the original 
>>> one,
>>> where A/D were set.
>>> The solution is to perform the whole emulation,
>>
>> Above you said fully emulating such an insn is incorrect. To me the
>> two statements contradict one another.
>>
>>> while ignoring EPT restrictions
>>> for the walk part, and taking them into account for the "actual" emulating 
>>> of
>>> the instruction at RIP.
>>
>> So the "ignore" part here is because the walk doesn't currently send
>> any events? That's an omission after all, which ultimately wants to
>> get fixed. This in turn makes me wonder whether there couldn't be
>> cases where a monitor actually wants to see these violations, too.
>> After all one may be able to abuse to page walker to set bits in
>> places you actually care to protect from undue modification.
> 
> There is no need for events from page walk. Further work will have to be
> done, when page-walk will send events, so that we can toggle that new
> feature on/off.

Please can you move over to thinking in more general terms,
not just what you need for your application. In this case
"There is no need" != "We don't have a need for". And I think
the VM event _interface_ should be arranged in a way that it
already accounts for eventually correct behavior of the page
walk paths.

>>> After the emulation stops we'll call hvm_vm_event_do_resume() again after 
>>> the
>>> introspection agent treats the event and resumes the guest. There, the
>>> instruction at RIP will be fully emulated (with the EPT ignored) if the
>>> introspection application allows it, and the guest will continue to run past
>>> the instruction.
>>>
>>> We use hvmemul_map_linear_addr() to intercept r/w access and
>>> __hvm_copy() to intercept exec access.
>>
>> Btw I continue to be unhappy about this asymmetry. Furthermore in
>> the former case you only handle write and rmw accesses, but not
>> reads afaics. I assume you don't care about reads, but this should
>> then be made explicit. Furthermore EPT allows read protection, and
>> there are p2m_access_w, p2m_access_wx, and p2m_access_x, so I guess
>> ignoring reads can at best be an option picked by the monitor, not
>> something to be left out of the interface altogether.
> 
> That is correct, we are not interested in read events but there is
> another problem, we are checking access and pfec to fill the event flag
> and pfec only has a write flag(PFEC_write_access), in __hvmemul_read()
> pfec only gets PFEC_page_present and there is no way to differentiate
> write from read.

By the PFEC model, anything that's not a write or insn fetch is a
read. The main anomaly is elsewhere: The write flag is also going
to be set for RMW operations.

>>> hvm_emulate_send_vm_event() can return false if there was no violation,
>>> if there was an error from monitor_traps() or p2m_get_mem_access().
>>
>> As said before - I don't think errors and lack of a violation can
>> sensibly be treated the same way. Is the implication perhaps that
>> emulation then will fail later anyway? If so, is such an
>> assumption taking into consideration possible races?
> 
> The only place that I can see a problem is the error from
> monitor_traps(). That can be checked and accompanied by a warning msg.

How would a warning message help?

> Or if you can give me a different idea to go forward with this issue I
> will be glad to review it.

I'm afraid you'll have to first of all give me an idea what the
correct action is in case of such an error. And once you've done
so, I'm pretty sure you'll recognize yourself whether the current
code you have is appropriate (and I'll then know whether I want
to insist on you changing the code).

>>> @@ -629,6 +697,14 @@ static void *hvmemul_map_linear_addr(
>>> 
>>> ASSERT(p2mt == p2m_ram_logdirty || !p2m_is_readonly(p2mt));
>>> }
>>> +
>>> +if ( curr->arch.vm_event &&
>>> +curr->arch.vm_event->send_event &&
>>> +hvm_emulate_se

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Tamas K Lengyel]

On Fri, Jul 19, 2019 at 7:11 AM Julien Grall  wrote:
>
> Hi Tamas,
>
> On 19/07/2019 14:00, Tamas K Lengyel wrote:
> > On Fri, Jul 19, 2019 at 2:43 AM Julien Grall  wrote:
> >>
> >> Hi Tamas,
> >>
> >> On 18/07/2019 18:48, Tamas K Lengyel wrote:
>   - Line 1025: The tools needs to be able to deal for_each_vcpu(...) 
>  & co.
> >>>
> >>> These can be made OK by adding braces. Other than that the only way I
> >>> found to make it not change the indentation is to add the comment "/*
> >>> *INDENT-OFF* */" before the block and "/* *INDENT-ON* */" afterwards.
> >>
> >> None of them looks really appealing because it means astyle will not 
> >> correctly
> >> indent if the user does not add braces or comments.
> >>
> >> Could astyle be easily modified to recognize foreach macros?
> >
> > Not that I'm aware of. If you don't want to manually annotate files
> > with unsupported macros then just exclude those files from astyle. I
> > wouldn't recommend adding this to the CI for all files, only for those
> > that their respective maintainers have confirmed to conform to the
> > style and want to enforce it going forward.
>
> So a couple use of an unsupported macros would make impossible to enforce the
> coding style. This is not a very ideal position to be in.
>
> _if_ we are going to adopt astyle then we need to be able to enforce it on 
> every
> Xen files long-term. If it is not possible to do it with astyle, then maybe 
> this
> is not the right tool to use.
>
> For instance, I know that tools such as clang-format is able to deal with
> foreach macros.

If there are better tools then sure, I don't really mind using
something else. I just don't have time to do the manual style check
back-and-forth anymore, so the sooner we have something in place the
better.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel