[Xen-devel] [xen-4.3-testing test] 96017: regressions - FAIL

[osstest service owner]

flight 96017 xen-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96017/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-libvirt5 libvirt-build fail REGR. vs. 87893
 build-amd64-libvirt   5 libvirt-build fail REGR. vs. 87893
 build-armhf   5 xen-build fail REGR. vs. 87893
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 
87893
 test-amd64-i386-xend-qemut-winxpsp3  9 windows-installfail REGR. vs. 87893

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 87893
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 87893

Tests which did not succeed, but are not blocking:
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 build-i386-rumpuserxen6 xen-buildfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass

version targeted for testing:
 xen  0a8c94fae993dd8f2b27fd4cc694f61c21de84bf
baseline version:
 xen  8fa31952e2d08ef63897c43b5e8b33475ebf5d93

Last test of basis87893  2016-03-29 13:49:52 Z   83 days
Failing since 92180  2016-04-20 17:49:21 Z   61 days   22 attempts
Testing same since96017  2016-06-20 17:22:27 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony Liguori 
  Anthony PERARD 
  Gerd Hoffmann 
  Ian Jackson 
  Jan Beulich 
  Jim Paris 
  Stefan Hajnoczi 
  Tim Deegan 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  fail
 build-i386   pass
 build-amd64-libvirt  fail
 build-armhf-libvirt  blocked 
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  blocked 
 test-amd64-i386-xl   pass
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64pass
 test-amd64-i386-xl-qemut-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail
 test-amd64-amd64-rumpuserxen-amd64   blocked 
 

[Xen-devel] [xen-4.6-testing test] 96006: regressions - FAIL

[osstest service owner]

flight 96006 xen-4.6-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96006/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-vhd   6 xen-boot  fail REGR. vs. 95849
 test-armhf-armhf-xl-credit2   9 debian-installfail REGR. vs. 95849
 test-armhf-armhf-libvirt-raw  9 debian-di-install fail REGR. vs. 95849

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 11 guest-start  fail   like 95776
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95849
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95849
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95849
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95849

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 xen  285248d91b20bc8245f9241e21d3e7b23f67b550
baseline version:
 xen  c68f2364d54fb7c3707aa91882b54c9529a1d445

Last test of basis95849  2016-06-17 07:40:53 Z3 days
Testing same since96006  2016-06-20 12:42:11 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Jan Beulich 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass
 

[Xen-devel] [qemu-upstream-4.3-testing test] 96003: regressions - FAIL

[osstest service owner]

flight 96003 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96003/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   5 libvirt-build fail REGR. vs. 80927
 build-i386-libvirt5 libvirt-build fail REGR. vs. 80927

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 80927
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 80927

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass

version targeted for testing:
 qemuu12e8fccf5b5460be7aecddc71d27eceaba6e1f15
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  135 days
Failing since 93977  2016-05-10 11:09:16 Z   41 days  136 attempts
Testing same since95534  2016-06-11 00:59:46 Z9 days   16 attempts


People who touched revisions under test:
  Anthony PERARD 
  Gerd Hoffmann 
  Ian Jackson 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-credit2  pass
 test-amd64-i386-freebsd10-i386   pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpupass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair pass
 test-amd64-amd64-pv  pass
 test-amd64-i386-pv   pass
 test-amd64-amd64-amd64-pvgrubpass
 test-amd64-amd64-i386-pvgrub pass
 test-amd64-amd64-pygrub  pass
 test-amd64-amd64-xl-qcow2pass
 test-amd64-i386-xl-raw   pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-libvirt-vhd blocked 
 test-amd64-amd64-xl-qemuu-winxpsp3   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.


commit 12e8fccf5b5460be7aecddc71d27eceaba6e1f15
Author: Ian Jackson 
Date:   Thu May 26 16:21:56 2016 +0100

main loop: Big hammer to fix logfile disk DoS in Xen setups

[Xen-devel] [xen-4.7-testing test] 96002: regressions - FAIL

[osstest service owner]

flight 96002 xen-4.7-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96002/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 95728

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95653
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95728

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 build-i386-rumpuserxen6 xen-buildfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  9a6cc4f5c14b3d7542b7523f88a1b65464733d3a
baseline version:
 xen  81722e7d443d33f48416c31f03ab4c2a9ba54b23

Last test of basis95728  2016-06-14 15:56:10 Z6 days
Testing same since96002  2016-06-20 11:20:51 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvops

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 94856
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 94856

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale 15 guest-start/debian.repeat fail in 95888 pass in 
95982
 test-amd64-amd64-xl-rtds  6 xen-boot   fail in 95909 pass in 95982
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
pass in 95888
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail pass 
in 95888
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeat   fail pass in 95888
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail pass in 
95909

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 94856
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94856
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94856
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 94856

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail in 95888 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu482b61844ae7c6df39df0b48ac90ffbc87bed7d2
baseline version:
 qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe

Last test of basis94856  2016-05-27 20:14:49 Z   23 days
Failing since 94983  2016-05-31 09:40:12 Z   20 days   27 attempts
Testing same since95868  2016-06-17 17:52:56 Z3 days6 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alex Bligh 
  Alex Williamson 
  Alexander Graf 
  Alexey Kardashevskiy 
  Alistair Francis 
  Amit Shah 
  Andrea Arcangeli 
  Andrew Jeffery 
  Andrew Jones 
  Anthony PERARD 

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Andrey Grodzovsky]

Just a small correction -
Not if (req_end >= field_start && field_end >= req_start)
But if (req_end *>* field_start && field_end *>* req_start)

On Mon, Jun 20, 2016 at 12:23 PM, Andrey Grodzovsky 
wrote:

>
>
> On Mon, Jun 20, 2016 at 11:38 AM, Jan Beulich  wrote:
>
>> >>> On 20.06.16 at 17:15,  wrote:
>> > On Mon, Jun 20, 2016 at 4:56 AM, Jan Beulich  wrote:
>> >
>> >> >>> On 20.06.16 at 00:03,  wrote:
>> >> > Follow up on
>> >> http://www.gossamer-threads.com/lists/xen/devel/436000#436000
>> >> > Using
>> >> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>> >> > reference.
>> >> >
>> >> >   New value
>> >> >   |---|
>> >> >
>> >> > f1  f5
>> >> > |---| |-|
>> >> >   f2f4
>> >> > |-|f3   |-|
>> >> >   |-|
>> >> >
>> >> > Given a new value of the type above, Current logic will not
>> >> > allow applying parts of the new value overlapping with f3 filter.
>> >> > only f2 and f4.
>> >>
>> >> I remains unclear what f1...f5 are meant to represent here.
>> >>
>> >
>> > f1-f5 would be config_field[] such as header_common in
>> conf_space_header.c
>> > or any other config_field added (by adding quirks for example).
>> >
>> >>
>> >> > This change allows all 3 types of overlapes to be included.
>> >> > More specifically for passthrough an Industrial Ethernet Interface
>> >> > (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
>> >> > Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
>> >> > given a quirk to allow read/write for that field is already in place.
>> >> > Device driver logic is such that the entire confspace  is
>> >> > written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
>> >> > arriving together in one call to xen_pcibk_config_write.
>> >>
>> >> Tweaks to make individual devices work are dubious. Any
>> >> explanation should outline why current behavior is _generally_
>> >> wrong. In particular with the original issue being with the
>> >> Latency Timer field, and with that field now being allowed to
>> >> be written by its entry in header_common[], ...
>> >
>> >
>> > To me current behaviour looks generally inconsistent  because given a
>> > request to wrote
>> > 4 bytes starting with 0xC let's look what's happening
>> > inside xen_pcibk_config_write
>> >
>> > *[81664.632262 <0.35>] xen-pciback: :06:00.0: write request
>> 4
>> > bytes at 0xc = 4000*
>> > *[81664.632264 <0.02>] xen-pciback: :06:00.0: read 1 bytes
>> at
>> > 0xc*
>> > *[81664.632275 <0.11>] xen-pciback: :06:00.0: read 1 bytes
>> at
>> > 0xc = 0 *
>> > *[81664.632282 <0.02>] xen-pciback: :06:00.0: read 1 bytes
>> at
>> > 0xf*
>> > *[81664.632292 <0.10>] xen-pciback: :06:00.0: read 1 bytes
>> at
>> > 0xf = 0*
>> >
>> > So you can see that current logic will allow to read/write 0xc which
>> > is PCI_CACHE_LINE_SIZE,
>> > skips PCI_LATENCY_TIMER also there is a quirk in place there which
>> allows
>> > writes to this memory,
>> > skips 0xE (which is fine since this field is not allowed to be accessed)
>> > and then writes 0xf PCI_BIST
>> >
>> > So using my previous sketch adjusted for this use case
>> >
>> >  |4b write request starting at 0xc|
>> >
>> >  |--f1--|  |--f2--| |--f3--|
>> >
>> > Where
>> > f1 ==   PCI_CACHE_LINE_SIZE
>> > f2 ==   PCI_LATENCY_TIMER
>> > f3 ==   PCI_BIST
>> >
>> > With ciurrent logic Only f1 and f3 are allowed but not f2 even when
>> there
>> > is a field and
>> > a quirk in place allowing read write access to that memory location.
>>
>> Let's leave quirks out of the picture for now. And without quirk,
>> f2 is not writable (and cannot be made by adding a quirk, as
>> explained before).
>>
>> Yes, i guess due to not allowing duplicates.
>
>
>> > To me it seems as a generally inconsistent behaviour and not
>> specifically
>> > related to our driver.
>> >
>> > With my patch (and a fix from || to && Thanks a lot for pointing this
>> out
>> > to me.)
>> > f1, f2 and f3 are being treated the same which IMHO is more correct.
>>
>> Hmm, with that and the >= -> > adjustment, I can't really see
>> how your variant is different from the original (other than being
>> more compact). What am I overlooking here?
>>
>> >> > --- a/drivers/xen/xen-pciback/conf_space.c
>> >> > +++ b/drivers/xen/xen-pciback/conf_space.c
>> >> > @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev,
>> int
>> >> > offset, int size, u32 value)
>> >> >   field_start = OFFSET(cfg_entry);
>> >> >   field_end = OFFSET(cfg_entry) + field->size;
>> >> >
>> >> > - if ((req_start >= field_start && req_start < field_end)
>> >> > - || (req_end > field_start && req_end <=
>> field_end)) {
>> >> > +  if (req_end >= 

Re: [Xen-devel] [PATCH v3 2/2] xen: add update indicator to vcpu_runstate_info

[Julien Grall]

Hi Juergen,

On 17/06/16 06:26, Juergen Gross wrote:

In order to support reading another vcpu's mapped vcpu_runstate_info
an indicator for an occurring update of the runstate information is
needed.

Add the possibility to activate setting this indicator in the highest
bit of state_entry_time via a vm_assist hypercall. When activated the
update indicator will be set before the runstate information is
modified in guest memory and it will be reset after modification is
done. As state_entry_time is guaranteed to be different after each
update the guest can detect any update (either in progress or while
reading the runstate data) by comparing state_entry_time before and
after reading runstate data: in case the values differ or the update
indicator was set the data might be inconsistent and should be reread.

Signed-off-by: Juergen Gross 


For the ARM bits:

Acked-by: Julien Grall 

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)

[Konrad Rzeszutek Wilk]

>   xtf-microvm-suite

+1

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)

[Ian Jackson]

We are in danger of getting stuck on this naming question.  I would
like everyone to put forward some suggestions for the name of thisr
toplevel epo on xenbits.

Hopefully we can find one that Andrew likes and that's acceptable to
the committers.

I suggest
  xen-microvm-test-framework
  xen-microvm-test-suite
  xtf-microvm-suite

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Doug Goldstein]

On 6/20/16 10:28 AM, Jan Beulich wrote:
 On 20.06.16 at 17:11,  wrote:
>> On 06/20/2016 10:46 AM, Doug Goldstein wrote:
>>> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
 Since enabling XSM is required to enable FLASK, place the option for
 FLASK below the one for XSM.  In addition, since it does not make sense
 to enable XSM without any XSM providers, and FLASK is the only XSM
 provider, hide the option to disable FLASK under EXPERT.

 Signed-off-by: Daniel De Graaf 
 ---
>>>
 @@ -137,6 +119,25 @@ config XSM

  If unsure, say N.

 +config FLASK
 +  def_bool y
 +  bool "FLux Advanced Security Kernel support" if EXPERT = "y"
>>>
>>> Ok. Here's the real review. I think you want the prompt to be optional
>>> if EXPERT is enabled then I think you need to use "prompt" instead of
>>> "bool". You've already got this set to a bool with the "def_bool" line.
>>
>> OK.  This version also apparently works, since I tested it, but if
>> prompt is the preferred keyword I'll change it.
> 
> Yeah, the bool is redundant here. It should be either a
> def_bool/prompt pair, or a bool/default one. Personally I'd prefer
> the latter, but since Doug asked for the former that's fine too.
> 
> Jan
> 

I'm honestly in different.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 2/3] console: use warning infrastructure for sync console warning

[Wei Liu]

Move the warning text to a static variable and marked that as initconst
data. Call warning_add in console_init_preirq. Finally remove all
unused bits.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/drivers/char/console.c | 38 ++
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f4f6141..6c771dc 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -18,7 +18,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -29,6 +28,7 @@
 #include 
 #include  /* for do_console_io */
 #include 
+#include 
 
 /* console: comma-separated list of console outputs. */
 static char __initdata opt_console[30] = OPT_CONSOLE_STR;
@@ -44,6 +44,14 @@ string_param("conswitch", opt_conswitch);
 /* sync_console: force synchronous console output (useful for debugging). */
 static bool_t __initdata opt_sync_console;
 boolean_param("sync_console", opt_sync_console);
+static const char __initconst *warning_sync_console =
+"**\n"
+"*** WARNING: CONSOLE OUTPUT IS SYNCHRONOUS\n"
+"*** This option is intended to aid debugging of Xen by ensuring\n"
+"*** that all output is synchronously delivered on the serial line.\n"
+"*** However it can introduce SIGNIFICANT latencies and affect\n"
+"*** timekeeping. It is NOT recommended for production use!\n"
+"**\n";
 
 /* console_to_ring: send guest (incl. dom 0) console data to console ring. */
 static bool_t __read_mostly opt_console_to_ring;
@@ -739,6 +747,7 @@ void __init console_init_preirq(void)
 serial_start_sync(sercon_handle);
 add_taint(TAINT_SYNC_CONSOLE);
 printk("Console output is synchronous.\n");
+warning_add(warning_sync_console);
 }
 }
 
@@ -786,8 +795,6 @@ void __init console_init_postirq(void)
 
 void __init console_endboot(void)
 {
-int i, j;
-
 printk("Std. Loglevel: %s", loglvl_str(xenlog_lower_thresh));
 if ( xenlog_upper_thresh != xenlog_lower_thresh )
 printk(" (Rate-limited: %s)", loglvl_str(xenlog_upper_thresh));
@@ -796,31 +803,6 @@ void __init console_endboot(void)
 printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
 printk("\n");
 
-if ( opt_sync_console )
-{
-printk("**\n");
-printk("*** WARNING: CONSOLE OUTPUT IS SYNCHRONOUS\n");
-printk("*** This option is intended to aid debugging "
-   "of Xen by ensuring\n");
-printk("*** that all output is synchronously delivered "
-   "on the serial line.\n");
-printk("*** However it can introduce SIGNIFICANT latencies "
-   "and affect\n");
-printk("*** timekeeping. It is NOT recommended for "
-   "production use!\n");
-printk("**\n");
-for ( i = 0; i < 3; i++ )
-{
-printk("%d... ", 3-i);
-for ( j = 0; j < 100; j++ )
-{
-process_pending_softirqs();
-mdelay(10);
-}
-}
-printk("\n");
-}
-
 video_endboot();
 
 /*
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 0/3] Make hvm_fep available to non-debug build

[Wei Liu]

Wei Liu (3):
  xen: add warning infrastructure
  console: use warning infrastructure for sync console warning
  xen: make available hvm_fep to non-debug build as well

 docs/misc/xen-command-line.markdown |  8 --
 xen/arch/x86/Kconfig| 17 +
 xen/arch/x86/hvm/hvm.c  | 15 ++-
 xen/arch/x86/setup.c|  3 +++
 xen/common/Makefile |  1 +
 xen/common/kernel.c |  6 +++--
 xen/common/warning.c| 50 +
 xen/drivers/char/console.c  | 38 
 xen/include/asm-x86/hvm/hvm.h   |  2 +-
 xen/include/xen/lib.h   |  1 +
 xen/include/xen/warning.h   |  7 ++
 11 files changed, 114 insertions(+), 34 deletions(-)
 create mode 100644 xen/common/warning.c
 create mode 100644 xen/include/xen/warning.h

-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Andrey Grodzovsky]

On Mon, Jun 20, 2016 at 11:38 AM, Jan Beulich  wrote:

> >>> On 20.06.16 at 17:15,  wrote:
> > On Mon, Jun 20, 2016 at 4:56 AM, Jan Beulich  wrote:
> >
> >> >>> On 20.06.16 at 00:03,  wrote:
> >> > Follow up on
> >> http://www.gossamer-threads.com/lists/xen/devel/436000#436000
> >> > Using
> >> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
> >> > reference.
> >> >
> >> >   New value
> >> >   |---|
> >> >
> >> > f1  f5
> >> > |---| |-|
> >> >   f2f4
> >> > |-|f3   |-|
> >> >   |-|
> >> >
> >> > Given a new value of the type above, Current logic will not
> >> > allow applying parts of the new value overlapping with f3 filter.
> >> > only f2 and f4.
> >>
> >> I remains unclear what f1...f5 are meant to represent here.
> >>
> >
> > f1-f5 would be config_field[] such as header_common in
> conf_space_header.c
> > or any other config_field added (by adding quirks for example).
> >
> >>
> >> > This change allows all 3 types of overlapes to be included.
> >> > More specifically for passthrough an Industrial Ethernet Interface
> >> > (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
> >> > Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
> >> > given a quirk to allow read/write for that field is already in place.
> >> > Device driver logic is such that the entire confspace  is
> >> > written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
> >> > arriving together in one call to xen_pcibk_config_write.
> >>
> >> Tweaks to make individual devices work are dubious. Any
> >> explanation should outline why current behavior is _generally_
> >> wrong. In particular with the original issue being with the
> >> Latency Timer field, and with that field now being allowed to
> >> be written by its entry in header_common[], ...
> >
> >
> > To me current behaviour looks generally inconsistent  because given a
> > request to wrote
> > 4 bytes starting with 0xC let's look what's happening
> > inside xen_pcibk_config_write
> >
> > *[81664.632262 <0.35>] xen-pciback: :06:00.0: write request 4
> > bytes at 0xc = 4000*
> > *[81664.632264 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
> > 0xc*
> > *[81664.632275 <0.11>] xen-pciback: :06:00.0: read 1 bytes at
> > 0xc = 0 *
> > *[81664.632282 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
> > 0xf*
> > *[81664.632292 <0.10>] xen-pciback: :06:00.0: read 1 bytes at
> > 0xf = 0*
> >
> > So you can see that current logic will allow to read/write 0xc which
> > is PCI_CACHE_LINE_SIZE,
> > skips PCI_LATENCY_TIMER also there is a quirk in place there which allows
> > writes to this memory,
> > skips 0xE (which is fine since this field is not allowed to be accessed)
> > and then writes 0xf PCI_BIST
> >
> > So using my previous sketch adjusted for this use case
> >
> >  |4b write request starting at 0xc|
> >
> >  |--f1--|  |--f2--| |--f3--|
> >
> > Where
> > f1 ==   PCI_CACHE_LINE_SIZE
> > f2 ==   PCI_LATENCY_TIMER
> > f3 ==   PCI_BIST
> >
> > With ciurrent logic Only f1 and f3 are allowed but not f2 even when there
> > is a field and
> > a quirk in place allowing read write access to that memory location.
>
> Let's leave quirks out of the picture for now. And without quirk,
> f2 is not writable (and cannot be made by adding a quirk, as
> explained before).
>
> Yes, i guess due to not allowing duplicates.


> > To me it seems as a generally inconsistent behaviour and not specifically
> > related to our driver.
> >
> > With my patch (and a fix from || to && Thanks a lot for pointing this out
> > to me.)
> > f1, f2 and f3 are being treated the same which IMHO is more correct.
>
> Hmm, with that and the >= -> > adjustment, I can't really see
> how your variant is different from the original (other than being
> more compact). What am I overlooking here?
>
> >> > --- a/drivers/xen/xen-pciback/conf_space.c
> >> > +++ b/drivers/xen/xen-pciback/conf_space.c
> >> > @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev,
> int
> >> > offset, int size, u32 value)
> >> >   field_start = OFFSET(cfg_entry);
> >> >   field_end = OFFSET(cfg_entry) + field->size;
> >> >
> >> > - if ((req_start >= field_start && req_start < field_end)
> >> > - || (req_end > field_start && req_end <= field_end))
> {
> >> > +  if (req_end >= field_start || field_end >= req_start) {
> >> >   tmp_val = 0;
> >>
> >> ... any change to the logic here which results in writes to the field
> >> getting issued would seem wrong without even looking at the
> >> particular nature of the field.
> >
> > I am not sure I understand - please clarify.
>
> See above - to me the original expression looks (a) 

[Xen-devel] [ovmf test] 95987: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. 
vs. 94748
 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail 
REGR. vs. 94748

version targeted for testing:
 ovmf cc0b456a05f8dd1ebfb9be485465be37e96999e7
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   26 days
Failing since 94750  2016-05-25 03:43:08 Z   26 days   47 attempts
Testing same since95884  2016-06-18 13:23:39 Z2 days6 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Chao Zhang 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Hao Wu 
  Hegde Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  lushifex 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Ruiyu Ni 
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Sriram Subramanian 
  Star Zeng 
  Tapan Shah 
  Thomas Palmer 
  Yonghong Zhu 
  Zhang, Chao B 

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



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

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

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

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


Not pushing.

(No revision log; it would be 2232 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Boris Ostrovsky]

On 06/20/2016 11:14 AM, Jan Beulich wrote:
 On 20.06.16 at 17:01,  wrote:
>> On 06/20/2016 09:55 AM, Jan Beulich wrote:
>> On 20.06.16 at 15:36,  wrote:
 On 06/20/2016 09:30 AM, Jan Beulich wrote:
 On 20.06.16 at 14:58,  wrote:
>> On 06/20/2016 04:56 AM, Jan Beulich wrote:
>> On 20.06.16 at 00:03,  wrote:
 Follow up on 
 http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
 Using 
 http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
 reference.

New value
|---|

 f1   f5
 |---|  |-|
 f2   f4
   |-|f3   |-|
|-|

 Given a new value of the type above, Current logic will not
 allow applying parts of the new value overlapping with f3 filter.
 only f2 and f4.
>>> I remains unclear what f1...f5 are meant to represent here.
>>>
 This change allows all 3 types of overlapes to be included.
 More specifically for passthrough an Industrial Ethernet Interface
 (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
 Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
 given a quirk to allow read/write for that field is already in place.
 Device driver logic is such that the entire confspace  is
 written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
 arriving together in one call to xen_pcibk_config_write.
>>> Tweaks to make individual devices work are dubious. Any
>>> explanation should outline why current behavior is _generally_
>>> wrong. In particular with the original issue being with the
>>> Latency Timer field, and with that field now being allowed to
>>> be written by its entry in header_common[], ...
>>>
 --- a/drivers/xen/xen-pciback/conf_space.c
 +++ b/drivers/xen/xen-pciback/conf_space.c
 @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
 offset, int size, u32 value)
field_start = OFFSET(cfg_entry);
field_end = OFFSET(cfg_entry) + field->size;

 -  if ((req_start >= field_start && req_start < field_end)
 -  || (req_end > field_start && req_end <= field_end)) 
 {
 +   if (req_end >= field_start || field_end >= req_start) {
tmp_val = 0;
>>> ... any change to the logic here which results in writes to the field
>>> getting issued would seem wrong without even looking at the
>>> particular nature of the field.
>>>
>>> As to the actual change - the two _end variables hold values
>>> pointing _past_ the region of interest, so the use of >= seems
>>> wrong here (ought to be >). And in the end we're talking of a
>>> classical overlap check here, which indeed can be simplified (to
>>> just two comparisons), but such simplification mustn't result in a
>>> change of behavior. (Such a simplification would end up being
>>>
>>> if (req_end > field_start && field_end > req_start) {
>>>
>>> afaict - note the || instead of && used in your change.)
>> Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and
>> adding a proper comment) solve this problem?
> How would that work? We mean to not allow writes, or else we
> could simply add a .u.b.write handler for PCI_LATENCY_TIMER.
 I thought you suggested (in another thread) to make PCI_LATENCY_TIMER 
 writable, just like PCI_CACHE_LINE_SIZE?
>>> Well, I did put this up for discussion (as much as I questioned
>>> whether Cache Line Size should perhaps not be writable). And if
>>> we wanted to make it writable, then we should do so the ordinary
>>> way, not via some hacks. 
>> That depends on how we want to view header_common's offset field:
>> whether it's address of a specific register (which is what it is now) or
>> it's just an offset and length of an access within config space (which
>> AFAIUI is how the driver in question wants to treat config space) and
>> it's up to read/write ops to sort out specifics if necessary.
> I don't see how this matters: The merge logic should be taking care
> of these aspects when wider width writes get used than what an
> individual register covers. Which isn't to say that I'm excluding an
> issue in that merge logic.

Yes, I was assuming the list_for_each_entry() loop breaks as soon as it
finds a match. It doesn't, so what I suggested is not especially useful
(if a write op is added to PCI_LATENCY TIMER)


-boris


___

[Xen-devel] Question about unable to add disk device after VM is destroyed and recreated on NVIDIA Jetson board

[Meng Xu]

Hi all,

I'm running Xen on NVIDIA Jetson TK1 board. The Xen code is from Ian's
repo.: git://xenbits.xen.org/people/ianc/xen.git with the commit point
c78d51660446d33dac4bb07c3c17e1d14d62ebc2

Right now, I can boot dom0 on Xen on the Jetson board. After the
system boots up, I can boot up a VM1 using the vm1.config (attached
below). However, after I use "xl destroy vm1" to destroy the vm1 and
try to boot up vm1 again with the exact same configuration file, it
starts to reports the following error:

--- Start of the output of the  "xl create vm1.xl"  ---
libxl: error: libxl_device.c:952:device_backend_callback: unable to
add device with path /local/domain/0/backend/vbd/5/51712

libxl: error: libxl_create.c:1161:domcreate_launch_dm: unable to add
disk devices

libxl: error: libxl_device.c:952:device_backend_callback: unable to
remove device with path /local/domain/0/backend/vbd/5/51712

libxl: error: libxl.c:1650:devices_destroy_cb: libxl__devices_destroy
failed for 5

---End of of the output of the  "xl create vm1.xl"  ---

I found that this issue was raised before in 2013 at
http://lists.xen.org/archives/html/xen-devel/2013-02/msg00704.html
However, I didn't see the solution on that thread.

I'm wondering if someone has encountered this issue before and know
how to fix it?

Thank you very much for your help and time! Any advice is really appreciated!

I attached the configurations and more log messages as below:

---The vm1's configuration file vm1.config---

kernel = "/boot/zImage-domU" # zImage is kernel domU will uses. zImage
is inside dom0 and it’s dom0’s path.

memory = 512

name = "vm1"

vcpus = 1

disk = [ 'file:/home/ubuntu/sdcard/vm1.disk,xvda,w' ]

vif = ['bridge=xenbr0']

extra = 'console=hvc0 xencons=tty root=/dev/xvda'


--- The log message when I create vm1---

# xl -vvv create -c vm1.xl

Parsing config from vm1.xl

libxl: debug: libxl_create.c:1512:do_domain_create: ao 0x3cdd8:
create: how=(nil) callback=(nil) poller=0x3ce20

libxl: debug: libxl_device.c:269:libxl__device_disk_set_backend: Disk
vdev=xvda spec.backend=unknown

libxl: debug: libxl_device.c:298:libxl__device_disk_set_backend: Disk
vdev=xvda, using backend phy

libxl: debug: libxl_create.c:915:initiate_domain_create: running bootloader

libxl: debug: libxl_bootloader.c:329:libxl__bootloader_run: no
bootloader configured, using user supplied kernel

libxl: debug: libxl_event.c:629:libxl__ev_xswatch_deregister: watch
w=0x3d2dc: deregister unregistered

domainbuilder: detail: xc_dom_allocate: cmdline="console=hvc0
xencons=tty root=/dev/xvda", features="(null)"

libxl: debug: libxl_dom.c:536:libxl__build_pv: pv kernel mapped 0 path
/boot/zImage-domU

domainbuilder: detail: xc_dom_kernel_file: filename="/boot/zImage-domU"

domainbuilder: detail: xc_dom_malloc_filemap: 5091 kB

domainbuilder: detail: xc_dom_boot_xen_init: ver 4.6, caps xen-3.0-armv7l

domainbuilder: detail: xc_dom_rambase_init: RAM starts at 4

domainbuilder: detail: xc_dom_parse_image: called

domainbuilder: detail: xc_dom_find_loader: trying multiboot-binary loader ...

domainbuilder: detail: loader probe failed

domainbuilder: detail: xc_dom_find_loader: trying Linux zImage (ARM64)
loader ...

domainbuilder: detail: xc_dom_probe_zimage64_kernel: kernel is not an
arm64 Image

domainbuilder: detail: loader probe failed

domainbuilder: detail: xc_dom_find_loader: trying Linux zImage (ARM32)
loader ...

domainbuilder: detail: loader probe OK

domainbuilder: detail: xc_dom_parse_zimage32_kernel: called

domainbuilder: detail: xc_dom_parse_zimage32_kernel: xen-3.0-armv7l:
0x40008000 -> 0x40500c20

libxl: debug: libxl_arm.c:537:libxl__arch_domain_init_hw_description:
configure the domain

libxl: debug: libxl_arm.c:545:libxl__arch_domain_init_hw_description:
constructing DTB for Xen version 4.6 guest

libxl: debug: libxl_arm.c:546:libxl__arch_domain_init_hw_description:
 - vGIC version: V2

libxl: debug: libxl_arm.c:303:make_memory_nodes: Creating placeholder
node /memory@4000

libxl: debug: libxl_arm.c:303:make_memory_nodes: Creating placeholder
node /memory@2

libxl: debug: libxl_arm.c:620:libxl__arch_domain_init_hw_description:
fdt total size 1266

domainbuilder: detail: xc_dom_devicetree_mem: called

domainbuilder: detail: xc_dom_mem_init: mem 512 MB, pages 0x2 pages, 4k each

domainbuilder: detail: xc_dom_mem_init: 0x2 pages

domainbuilder: detail: xc_dom_boot_mem_init: called

domainbuilder: detail: set_mode: guest xen-3.0-armv7l, address size 32

domainbuilder: detail: xc_dom_malloc: 1024 kB

domainbuilder: detail: populate_guest_memory: populating RAM @
4000-6000 (512MB)

domainbuilder: detail: populate_one_size: populated 0x100/0x100
entries with shift 9

domainbuilder: detail: arch_setup_meminit: placing boot modules at 0x4800

domainbuilder: detail: arch_setup_meminit: devicetree: 0x4800 -> 0x48001000

libxl: debug: libxl_arm.c:651:finalise_one_memory_node: Populating
placeholder node 

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Jan Beulich]

>>> On 20.06.16 at 17:15,  wrote:

Argh - I had started a second reply. This was actually meant to be
part of it.

> On Mon, Jun 20, 2016 at 4:56 AM, Jan Beulich  wrote:
> 
>> >>> On 20.06.16 at 00:03,  wrote:
>> > Follow up on
>> http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
>> > Using
>> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>> > reference.
>> >
>> >   New value
>> >   |---|
>> >
>> > f1  f5
>> > |---| |-|
>> >   f2f4
>> > |-|f3   |-|
>> >   |-|
>> >
>> > Given a new value of the type above, Current logic will not
>> > allow applying parts of the new value overlapping with f3 filter.
>> > only f2 and f4.
>>
>> I remains unclear what f1...f5 are meant to represent here.
>>
> 
> f1-f5 would be config_field[] such as header_common in conf_space_header.c
> or any other config_field added (by adding quirks for example).

Ah, but these are intended to not overlap.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

[Julien Grall]

On 20/06/16 16:36, Andrew Cooper wrote:

On 20/06/16 16:03, Julien Grall wrote:

Hi Andrew,

On 20/06/16 15:53, Andrew Cooper wrote:

On 20/06/16 14:37, Julien Grall wrote:

p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
the variable to *gfn* and use typesafe to avoid possible misusage.

Signed-off-by: Julien Grall 


On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long.

Is the truncation ok?


The PFN will be encoded on 28 bits maximum (40 bits address). Unless
we want to check that the guest effectively zeroed the unused bits, I
think the truncation is fine.


Ok - I was just checking that it wasn't happening accidentally.


I will mention it in the commit message.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Jan Beulich]

>>> On 20.06.16 at 17:15,  wrote:
> On Mon, Jun 20, 2016 at 4:56 AM, Jan Beulich  wrote:
> 
>> >>> On 20.06.16 at 00:03,  wrote:
>> > Follow up on
>> http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
>> > Using
>> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>> > reference.
>> >
>> >   New value
>> >   |---|
>> >
>> > f1  f5
>> > |---| |-|
>> >   f2f4
>> > |-|f3   |-|
>> >   |-|
>> >
>> > Given a new value of the type above, Current logic will not
>> > allow applying parts of the new value overlapping with f3 filter.
>> > only f2 and f4.
>>
>> I remains unclear what f1...f5 are meant to represent here.
>>
> 
> f1-f5 would be config_field[] such as header_common in conf_space_header.c
> or any other config_field added (by adding quirks for example).
> 
>>
>> > This change allows all 3 types of overlapes to be included.
>> > More specifically for passthrough an Industrial Ethernet Interface
>> > (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
>> > Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
>> > given a quirk to allow read/write for that field is already in place.
>> > Device driver logic is such that the entire confspace  is
>> > written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
>> > arriving together in one call to xen_pcibk_config_write.
>>
>> Tweaks to make individual devices work are dubious. Any
>> explanation should outline why current behavior is _generally_
>> wrong. In particular with the original issue being with the
>> Latency Timer field, and with that field now being allowed to
>> be written by its entry in header_common[], ...
> 
> 
> To me current behaviour looks generally inconsistent  because given a
> request to wrote
> 4 bytes starting with 0xC let's look what's happening
> inside xen_pcibk_config_write
> 
> *[81664.632262 <0.35>] xen-pciback: :06:00.0: write request 4
> bytes at 0xc = 4000*
> *[81664.632264 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
> 0xc*
> *[81664.632275 <0.11>] xen-pciback: :06:00.0: read 1 bytes at
> 0xc = 0 *
> *[81664.632282 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
> 0xf*
> *[81664.632292 <0.10>] xen-pciback: :06:00.0: read 1 bytes at
> 0xf = 0*
> 
> So you can see that current logic will allow to read/write 0xc which
> is PCI_CACHE_LINE_SIZE,
> skips PCI_LATENCY_TIMER also there is a quirk in place there which allows
> writes to this memory,
> skips 0xE (which is fine since this field is not allowed to be accessed)
> and then writes 0xf PCI_BIST
> 
> So using my previous sketch adjusted for this use case
> 
>  |4b write request starting at 0xc|
> 
>  |--f1--|  |--f2--| |--f3--|
> 
> Where
> f1 ==   PCI_CACHE_LINE_SIZE
> f2 ==   PCI_LATENCY_TIMER
> f3 ==   PCI_BIST
> 
> With ciurrent logic Only f1 and f3 are allowed but not f2 even when there
> is a field and
> a quirk in place allowing read write access to that memory location.

Let's leave quirks out of the picture for now. And without quirk,
f2 is not writable (and cannot be made by adding a quirk, as
explained before).

> To me it seems as a generally inconsistent behaviour and not specifically
> related to our driver.
> 
> With my patch (and a fix from || to && Thanks a lot for pointing this out
> to me.)
> f1, f2 and f3 are being treated the same which IMHO is more correct.

Hmm, with that and the >= -> > adjustment, I can't really see
how your variant is different from the original (other than being
more compact). What am I overlooking here?

>> > --- a/drivers/xen/xen-pciback/conf_space.c
>> > +++ b/drivers/xen/xen-pciback/conf_space.c
>> > @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
>> > offset, int size, u32 value)
>> >   field_start = OFFSET(cfg_entry);
>> >   field_end = OFFSET(cfg_entry) + field->size;
>> >
>> > - if ((req_start >= field_start && req_start < field_end)
>> > - || (req_end > field_start && req_end <= field_end)) {
>> > +  if (req_end >= field_start || field_end >= req_start) {
>> >   tmp_val = 0;
>>
>> ... any change to the logic here which results in writes to the field
>> getting issued would seem wrong without even looking at the
>> particular nature of the field.
> 
> I am not sure I understand - please clarify.

See above - to me the original expression looks (a) correct and
(b) identical in effect to the corrected version of yours. Hence
if behavior changes, something must be wrong in either old or new
code, and due to (a) I would imply it's in the new one. But as said
above - I guess I'm missing something here.

Jan

___
Xen-devel mailing list

Re: [Xen-devel] [PATCH v2 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

[Andrew Cooper]

On 20/06/16 16:03, Julien Grall wrote:
> Hi Andrew,
>
> On 20/06/16 15:53, Andrew Cooper wrote:
>> On 20/06/16 14:37, Julien Grall wrote:
>>> p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
>>> the variable to *gfn* and use typesafe to avoid possible misusage.
>>>
>>> Signed-off-by: Julien Grall 
>>
>> On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long.
>>
>> Is the truncation ok?
>
> The PFN will be encoded on 28 bits maximum (40 bits address). Unless
> we want to check that the guest effectively zeroed the unused bits, I
> think the truncation is fine.

Ok - I was just checking that it wasn't happening accidentally.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Jan Beulich]

>>> On 20.06.16 at 17:11,  wrote:
> On 06/20/2016 10:46 AM, Doug Goldstein wrote:
>> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
>>> Since enabling XSM is required to enable FLASK, place the option for
>>> FLASK below the one for XSM.  In addition, since it does not make sense
>>> to enable XSM without any XSM providers, and FLASK is the only XSM
>>> provider, hide the option to disable FLASK under EXPERT.
>>>
>>> Signed-off-by: Daniel De Graaf 
>>> ---
>>
>>> @@ -137,6 +119,25 @@ config XSM
>>>
>>>   If unsure, say N.
>>>
>>> +config FLASK
>>> +   def_bool y
>>> +   bool "FLux Advanced Security Kernel support" if EXPERT = "y"
>>
>> Ok. Here's the real review. I think you want the prompt to be optional
>> if EXPERT is enabled then I think you need to use "prompt" instead of
>> "bool". You've already got this set to a bool with the "def_bool" line.
> 
> OK.  This version also apparently works, since I tested it, but if
> prompt is the preferred keyword I'll change it.

Yeah, the bool is redundant here. It should be either a
def_bool/prompt pair, or a bool/default one. Personally I'd prefer
the latter, but since Doug asked for the former that's fine too.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 02/11] hvmctl: convert HVMOP_set_pci_intx_level

[Jan Beulich]

>>> On 20.06.16 at 16:48,  wrote:
> Daniel De Graaf writes ("Re: [PATCH 02/11] hvmctl: convert 
> HVMOP_set_pci_intx_level"):
>> On 06/20/2016 08:53 AM, Jan Beulich wrote:
>> > Note that this adds validation of the "domain" interface structure
>> > field, which previously got ignored.
>> >
>> > Note further that this retains the hvmop interface definitions as those
>> > had (wrongly) been exposed to non-tool stack consumers (albeit the
>> > operation wouldn't have succeeded when requested by a domain for
>> > itself).
>> >
>> > Signed-off-by: Jan Beulich 
>> > ---
>> > TBD: xen/xsm/flask/policy/access_vectors says "also needs hvmctl", but
>> >  I don't see how this has been done so far. With the change here,
>> >  doing two checks in flask_hvm_control() (the generic one always
>> >  and a specific one if needed) would of course be simple, but it's
>> >  unclear how subsequently added sub-ops should then be dealt with
>> >  (which don't have a similar remark).
>> 
>> I am not sure why that remark is there: it seems like it refers to an
>> overall check in the HVM operation hypercall, which does not exist.
>> 
>> There is no reason to have an operation protected by two different
>> access checks, so I think that both the previous and patched code
>> are correct and the "also needs hvmctl" comment should be removed.
>> With that, Acked-by: Daniel De Graaf 
> 
> This is a slight digression, but is it intended that all of these
> hvmctl's are safe to expose to a deprivileged device model process in
> dom0, or to a device model stub domain ?

Yes, afaict (they've been exposed the same way before).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/8] x86/time: correctly honor late clearing of TSC related feature flags

[Jan Beulich]

>>> On 20.06.16 at 16:32,  wrote:
> On 15/06/16 11:28, Jan Beulich wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1358,6 +1358,24 @@ static void time_calibration(void *unuse
>>   , 1);
>>  }
>>  
>> +void __init clear_tsc_cap(unsigned int feature)
>> +{
>> +void (*rendezvous_fn)(void *) = time_calibration_std_rendezvous;
> 
> This should read time_calibration_rendezvous_fn rather than assuming
> time_calibration_std_rendezvous.
> 
> Otherwise, there is a risk that it could be reset back to
> time_calibration_std_rendezvous.

But that's the purpose: We may need to switch back.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/8] x86/time: calibrate TSC against platform timer

[Jan Beulich]

>>> On 20.06.16 at 16:20,  wrote:
> On 15/06/16 11:28, Jan Beulich wrote:
>> --- a/xen/arch/x86/i8259.c
>> +++ b/xen/arch/x86/i8259.c
>> @@ -359,12 +359,7 @@ void __init init_IRQ(void)
>>  
>>  apic_intr_init();
>>  
>> -/* Set the clock to HZ Hz */
>> -#define CLOCK_TICK_RATE 1193182 /* crystal freq (Hz) */
>> -#define LATCH (((CLOCK_TICK_RATE)+(HZ/2))/HZ)
>> -outb_p(0x34, PIT_MODE);/* binary, mode 2, LSB/MSB, ch 0 */
>> -outb_p(LATCH & 0xff, PIT_CH0); /* LSB */
>> -outb(LATCH >> 8, PIT_CH0); /* MSB */
>> +preinit_pit();
> 
> This highlights the fact that we have two unconditional calls to
> preinit_pit() during startup, which is one too many.
> 
> init_IRQ() is called rather earlier than early_time_init(), but I can't
> spot anything inbetween the two calls which would require the PIT to be
> set up.  AFAICT, it is safe to just drop the preinit_pit() call here.

LAPIC initialization makes use of the PIT, and I think that would
break when removing it here. And since doing it twice is benign,
I'd also like to not drop it from early_time_init().

>> @@ -340,7 +348,8 @@ static struct platform_timesource __init
>>  .frequency = CLOCK_TICK_RATE,
>>  .read_counter = read_pit_count,
>>  .counter_bits = 32,
>> -.init = init_pit
>> +.init = init_pit,
>> +.resume = resume_pit
> 
> Please add a redundant comma at the end, to reduce the next diff to
> change this block.

Well, I'd like the three instance to remain consistent in this regard
(yet touching the others doesn't seem warranted). And a change
here isn't all that likely.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Doug Goldstein]

On 6/20/16 10:07 AM, Daniel De Graaf wrote:
> On 06/20/2016 10:35 AM, Doug Goldstein wrote:
>> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
>>> This operation has no known users, and is primarily useful when an MLS
>>> policy is in use (which has never been shipped with Xen).  In addition,
>>> the information it provides does not actually depend on hypervisor
>>> state (only on the XSM policy), so an application that needs it could
>>> compute the results without needing to involve the hypervisor.
>>>
>>
>> So if I read this language correctly. Removing this does not affect
>> someone being able to build a MLS policy at a later date right?
> 
> Correct; that support is still there.  This hypercall was used to
> compute a list of reachable security contexts for a given user, which
> is trivial in a non-MLS policy but more complex when one is being
> used.  This computation makes more sense on Linux (where creating
> new contexts via "exec" is common) than on Xen (where normally a
> domain cannot create another).
> 

Makes sense. Thanks for clarifying.

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Andrey Grodzovsky]

On Mon, Jun 20, 2016 at 4:56 AM, Jan Beulich  wrote:

> >>> On 20.06.16 at 00:03,  wrote:
> > Follow up on
> http://www.gossamer-threads.com/lists/xen/devel/436000#436000
> > Using
> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
> > reference.
> >
> >   New value
> >   |---|
> >
> > f1  f5
> > |---| |-|
> >   f2f4
> > |-|f3   |-|
> >   |-|
> >
> > Given a new value of the type above, Current logic will not
> > allow applying parts of the new value overlapping with f3 filter.
> > only f2 and f4.
>
> I remains unclear what f1...f5 are meant to represent here.
>

f1-f5 would be config_field[] such as header_common in conf_space_header.c
or any other config_field added (by adding quirks for example).

>
> > This change allows all 3 types of overlapes to be included.
> > More specifically for passthrough an Industrial Ethernet Interface
> > (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
> > Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
> > given a quirk to allow read/write for that field is already in place.
> > Device driver logic is such that the entire confspace  is
> > written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
> > arriving together in one call to xen_pcibk_config_write.
>
> Tweaks to make individual devices work are dubious. Any
> explanation should outline why current behavior is _generally_
> wrong. In particular with the original issue being with the
> Latency Timer field, and with that field now being allowed to
> be written by its entry in header_common[], ...


To me current behaviour looks generally inconsistent  because given a
request to wrote
4 bytes starting with 0xC let's look what's happening
inside xen_pcibk_config_write

*[81664.632262 <0.35>] xen-pciback: :06:00.0: write request 4
bytes at 0xc = 4000*
*[81664.632264 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
0xc*
*[81664.632275 <0.11>] xen-pciback: :06:00.0: read 1 bytes at
0xc = 0 *
*[81664.632282 <0.02>] xen-pciback: :06:00.0: read 1 bytes at
0xf*
*[81664.632292 <0.10>] xen-pciback: :06:00.0: read 1 bytes at
0xf = 0*

So you can see that current logic will allow to read/write 0xc which
is PCI_CACHE_LINE_SIZE,
skips PCI_LATENCY_TIMER also there is a quirk in place there which allows
writes to this memory,
skips 0xE (which is fine since this field is not allowed to be accessed)
and then writes 0xf PCI_BIST

So using my previous sketch adjusted for this use case

 |4b write request starting at 0xc|

 |--f1--|  |--f2--| |--f3--|

Where
f1 ==   PCI_CACHE_LINE_SIZE
f2 ==   PCI_LATENCY_TIMER
f3 ==   PCI_BIST

With ciurrent logic Only f1 and f3 are allowed but not f2 even when there
is a field and
a quirk in place allowing read write access to that memory location.

To me it seems as a generally inconsistent behaviour and not specifically
related to our driver.

With my patch (and a fix from || to && Thanks a lot for pointing this out
to me.)
f1, f2 and f3 are being treated the same which IMHO is more correct.



> > --- a/drivers/xen/xen-pciback/conf_space.c
> > +++ b/drivers/xen/xen-pciback/conf_space.c
> > @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
> > offset, int size, u32 value)
> >   field_start = OFFSET(cfg_entry);
> >   field_end = OFFSET(cfg_entry) + field->size;
> >
> > - if ((req_start >= field_start && req_start < field_end)
> > - || (req_end > field_start && req_end <= field_end)) {
> > +  if (req_end >= field_start || field_end >= req_start) {
> >   tmp_val = 0;
>
> ... any change to the logic here which results in writes to the field
> getting issued would seem wrong without even looking at the
> particular nature of the field.
>

I am not sure I understand - please clarify.

>
> As to the actual change - the two _end variables hold values
> pointing _past_ the region of interest, so the use of >= seems
> wrong here (ought to be >). And in the end we're talking of a
> classical overlap check here, which indeed can be simplified (to
> just two comparisons), but such simplification mustn't result in a
> change of behavior. (Such a simplification would end up being
>
> if (req_end > field_start && field_end > req_start) {
>
> afaict - note the || instead of && used in your change.)
>

Totally agree, than you for this insight!

>
> Jan
>
> Andrey
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Jan Beulich]

>>> On 20.06.16 at 17:01,  wrote:
> On 06/20/2016 09:55 AM, Jan Beulich wrote:
> On 20.06.16 at 15:36,  wrote:
>>> On 06/20/2016 09:30 AM, Jan Beulich wrote:
>>> On 20.06.16 at 14:58,  wrote:
> On 06/20/2016 04:56 AM, Jan Beulich wrote:
> On 20.06.16 at 00:03,  wrote:
>>> Follow up on 
>>> http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
>>> Using 
>>> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>>> reference.
>>>
>>> New value
>>> |---|
>>>
>>> f1f5
>>> |---|   |-|
>>> f2f4
>>>   |-|f3   |-|
>>> |-|
>>>
>>> Given a new value of the type above, Current logic will not
>>> allow applying parts of the new value overlapping with f3 filter.
>>> only f2 and f4.
>> I remains unclear what f1...f5 are meant to represent here.
>>
>>> This change allows all 3 types of overlapes to be included.
>>> More specifically for passthrough an Industrial Ethernet Interface
>>> (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
>>> Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
>>> given a quirk to allow read/write for that field is already in place.
>>> Device driver logic is such that the entire confspace  is
>>> written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
>>> arriving together in one call to xen_pcibk_config_write.
>> Tweaks to make individual devices work are dubious. Any
>> explanation should outline why current behavior is _generally_
>> wrong. In particular with the original issue being with the
>> Latency Timer field, and with that field now being allowed to
>> be written by its entry in header_common[], ...
>>
>>> --- a/drivers/xen/xen-pciback/conf_space.c
>>> +++ b/drivers/xen/xen-pciback/conf_space.c
>>> @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
>>> offset, int size, u32 value)
>>> field_start = OFFSET(cfg_entry);
>>> field_end = OFFSET(cfg_entry) + field->size;
>>>
>>> -   if ((req_start >= field_start && req_start < field_end)
>>> -   || (req_end > field_start && req_end <= field_end)) 
>>> {
>>> +if (req_end >= field_start || field_end >= req_start) {
>>> tmp_val = 0;
>> ... any change to the logic here which results in writes to the field
>> getting issued would seem wrong without even looking at the
>> particular nature of the field.
>>
>> As to the actual change - the two _end variables hold values
>> pointing _past_ the region of interest, so the use of >= seems
>> wrong here (ought to be >). And in the end we're talking of a
>> classical overlap check here, which indeed can be simplified (to
>> just two comparisons), but such simplification mustn't result in a
>> change of behavior. (Such a simplification would end up being
>>
>>  if (req_end > field_start && field_end > req_start) {
>>
>> afaict - note the || instead of && used in your change.)
> Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and
> adding a proper comment) solve this problem?
 How would that work? We mean to not allow writes, or else we
 could simply add a .u.b.write handler for PCI_LATENCY_TIMER.
>>> I thought you suggested (in another thread) to make PCI_LATENCY_TIMER 
>>> writable, just like PCI_CACHE_LINE_SIZE?
>> Well, I did put this up for discussion (as much as I questioned
>> whether Cache Line Size should perhaps not be writable). And if
>> we wanted to make it writable, then we should do so the ordinary
>> way, not via some hacks. 
> 
> That depends on how we want to view header_common's offset field:
> whether it's address of a specific register (which is what it is now) or
> it's just an offset and length of an access within config space (which
> AFAIUI is how the driver in question wants to treat config space) and
> it's up to read/write ops to sort out specifics if necessary.

I don't see how this matters: The merge logic should be taking care
of these aspects when wider width writes get used than what an
individual register covers. Which isn't to say that I'm excluding an
issue in that merge logic.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Daniel De Graaf]

On 06/20/2016 10:46 AM, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

Since enabling XSM is required to enable FLASK, place the option for
FLASK below the one for XSM.  In addition, since it does not make sense
to enable XSM without any XSM providers, and FLASK is the only XSM
provider, hide the option to disable FLASK under EXPERT.

Signed-off-by: Daniel De Graaf 
---



@@ -137,6 +119,25 @@ config XSM

  If unsure, say N.

+config FLASK
+   def_bool y
+   bool "FLux Advanced Security Kernel support" if EXPERT = "y"


Ok. Here's the real review. I think you want the prompt to be optional
if EXPERT is enabled then I think you need to use "prompt" instead of
"bool". You've already got this set to a bool with the "def_bool" line.


OK.  This version also apparently works, since I tested it, but if
prompt is the preferred keyword I'll change it.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/4] Xen: Support adding DT nodes

[Andre Przywara]

From: Christoffer Dall 

Support adding xen,xen-bootargs node via --with-xen-bootargs to the
configure script and automatically add the Dom0 node to the DT as well.

Signed-off-by: Christoffer Dall 
Signed-off-by: Andre Przywara 
---
 Makefile.am  | 34 +-
 configure.ac |  9 +
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1a801c0..d83b417 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -93,24 +93,32 @@ FILESYSTEM_END  := $(shell echo $$(($(FILESYSTEM_START) 
+ $(FILESYSTEM_SIZE
 
 FDT_OFFSET := 0x0800
 
+if XEN
+XEN:= -DXEN=$(XEN_IMAGE)
+XEN_OFFSET := 0x0820
+KERNEL_SIZE:= $(shell stat -Lc %s $(KERNEL_IMAGE) 2>/dev/null || echo 0)
+DOM0_OFFSET:= $(shell echo $$(($(PHYS_OFFSET) + $(KERNEL_OFFSET
+XEN_BOOTARGS   := xen,xen-bootargs = \"$(BOOTARGS)\";  \
+  \#address-cells = <2>;   \
+  \#size-cells = <2>;  \
+  module@1 {   \
+   compatible = \"xen,linux-zimage\", 
\"xen,multiboot-module\"; \
+   reg = <0x0 $(DOM0_OFFSET) 0x0 $(KERNEL_SIZE)>;  \
+  };
+endif
+
+
 if INITRD
 INITRD_FLAGS   := -DUSE_INITRD
+INITRD_CHOSEN   := linux,initrd-start = <$(FILESYSTEM_START)>; \
+  linux,initrd-end = <$(FILESYSTEM_END)>;
+endif
+
 CHOSEN_NODE:= chosen { \
bootargs = \"$(CMDLINE)\";  \
-   linux,initrd-start = <$(FILESYSTEM_START)>; \
-   linux,initrd-end = <$(FILESYSTEM_END)>; \
-  };
-else
-INITRD_FLAGS   :=
-CHOSEN_NODE:= chosen { \
-   bootargs = \"$(CMDLINE)\";  \
+   $(INITRD_CHOSEN)\
+   $(XEN_BOOTARGS) \
   };
-endif
-
-if XEN
-XEN:= -DXEN=$(XEN_IMAGE)
-XEN_OFFSET := 0x0820
-endif
 
 CPPFLAGS   += $(INITRD_FLAGS)
 CFLAGS += -Iinclude/ -I$(ARCH_SRC)/include/
diff --git a/configure.ac b/configure.ac
index 2441f8b..b001939 100644
--- a/configure.ac
+++ b/configure.ac
@@ -95,6 +95,12 @@ AC_ARG_WITH([cmdline],
[C_CMDLINE=$withval])
 AC_SUBST([CMDLINE], [$C_CMDLINE])
 
+X_BOOTARGS="console=dtuart dtuart=serial0 no-bootscrub"
+AC_ARG_WITH([xen-bootargs],
+   AS_HELP_STRING([--with-xen-bootargs], [set Xen bootargs]),
+   [X_BOOTARGS=$withval])
+AC_SUBST([BOOTARGS], [$X_BOOTARGS])
+
 # Allow a user to pass --enable-gicv3
 AC_ARG_ENABLE([gicv3],
AS_HELP_STRING([--enable-gicv3], [enable GICv3 instead of GICv2]),
@@ -133,4 +139,7 @@ echo "  Use GICv3? ${USE_GICV3}"
 echo "  Boot-wrapper execution state:  AArch${BOOTWRAPPER_ES}"
 echo "  Kernel execution state:AArch${KERNEL_ES}"
 echo "  Xen image  ${X_IMAGE:-NONE}"
+if test "x${X_IMAGE}" != "x"; then
+echo "  Xen Bootargs:  ${X_BOOTARGS}"
+fi
 echo ""
-- 
2.9.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/4] Xen: Select correct dom0 console

[Andre Przywara]

From: Ian Campbell 

If Xen is enabled, tell Dom0 to use the 'hvc0' console, and fall back to
the usual ttyAMA0 otherwise.

Signed-off-by: Ian Campbell 
Signed-off-by: Christoffer Dall 
Signed-off-by: Andre Przywara 
---
 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index b001939..f381a80 100644
--- a/configure.ac
+++ b/configure.ac
@@ -89,7 +89,8 @@ AC_ARG_WITH([initrd],
 AC_SUBST([FILESYSTEM], [$USE_INITRD])
 AM_CONDITIONAL([INITRD], [test "x$USE_INITRD" != "x"])
 
-C_CMDLINE="console=ttyAMA0 earlyprintk=pl011,0x1c09"
+AS_IF([test "x$X_IMAGE" = "x"],[C_CONSOLE="ttyAMA0"],[C_CONSOLE="hvc0"])
+C_CMDLINE="console=$C_CONSOLE earlyprintk=pl011,0x1c09"
 AC_ARG_WITH([cmdline],
AS_HELP_STRING([--with-cmdline], [set a command line for the kernel]),
[C_CMDLINE=$withval])
-- 
2.9.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/4] Support for building in a Xen binary

[Andre Przywara]

From: Christoffer Dall 

Add support for building a Xen binary which includes a Dom0 image and
the Dom0 command-line.

If the user specifies --with-xen=, where Xen is an appropriate
AArch64 Xen binary, the build system will generate a xen-system.axf
instead of a linux-system.axf.

Original patch from Ian Campbell, but I modified most of it so all bugs
are probably mine.
[Andre: adapt to newest boot-wrapper branch, increase load address]

Cc: Ian Campbell 
Signed-off-by: Christoffer Dall 
Signed-off-by: Andre Przywara 
---
 .gitignore|  1 +
 Makefile.am   | 12 
 boot_common.c |  4 ++--
 configure.ac  | 14 ++
 model.lds.S   | 14 ++
 5 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/.gitignore b/.gitignore
index 8653852..80770c0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,7 @@ configure
 dtc
 fdt.dtb
 Image
+Xen
 install-sh
 Makefile
 Makefile.in
diff --git a/Makefile.am b/Makefile.am
index 692d2cc..1a801c0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -85,7 +85,6 @@ TEXT_LIMIT:= 0x8008
 endif
 
 LD_SCRIPT  := model.lds.S
-IMAGE  := linux-system.axf
 
 FS_OFFSET  := 0x1000
 FILESYSTEM_START:= $(shell echo $$(($(PHYS_OFFSET) + $(FS_OFFSET
@@ -108,6 +107,11 @@ CHOSEN_NODE:= chosen { 
\
   };
 endif
 
+if XEN
+XEN:= -DXEN=$(XEN_IMAGE)
+XEN_OFFSET := 0x0820
+endif
+
 CPPFLAGS   += $(INITRD_FLAGS)
 CFLAGS += -Iinclude/ -I$(ARCH_SRC)/include/
 CFLAGS += -Wall -fomit-frame-pointer
@@ -117,11 +121,11 @@ LDFLAGS   += --gc-sections
 OFILES += boot_common.o bakery_lock.o platform.o $(GIC) cache.o lib.o
 OFILES += $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
 
-all: $(IMAGE)
+all: $(IMAGE) $(XIMAGE)
 
 CLEANFILES = $(IMAGE) $(OFILES) model.lds fdt.dtb
 
-$(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM)
+$(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) 
$(XEN_IMAGE)
$(LD) $(LDFLAGS) $(OFILES) -o $@ --script=model.lds
 
 %.o: %.S Makefile
@@ -131,7 +135,7 @@ $(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) 
$(FILESYSTEM)
$(CC) $(CPPFLAGS) $(CFLAGS) $(DEFINES) -c -o $@ $<
 
 model.lds: $(LD_SCRIPT) Makefile
-   $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) 
-DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) 
-DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) -DKERNEL=$(KERNEL_IMAGE) 
-DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
+   $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) 
-DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) 
-DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) 
-DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) 
-DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
 
 fdt.dtb: $(KERNEL_DTB) Makefile gen-cpu-nodes.sh
( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) 
$(PSCI_NODE) $(CPUS_NODE) };" ) | $(DTC) -O dtb -o $@ -
diff --git a/boot_common.c b/boot_common.c
index 4947fe3..e7b8e1d 100644
--- a/boot_common.c
+++ b/boot_common.c
@@ -9,7 +9,7 @@
 #include 
 #include 
 
-extern unsigned long kernel;
+extern unsigned long entrypoint;
 extern unsigned long dtb;
 
 void init_platform(void);
@@ -67,7 +67,7 @@ void __noreturn first_spin(unsigned int cpu, unsigned long 
*mbox,
if (cpu == 0) {
init_platform();
 
-   *mbox = (unsigned long)
+   *mbox = (unsigned long)
sevl();
spin(mbox, invalid, 1);
} else {
diff --git a/configure.ac b/configure.ac
index ab8f5b3..2441f8b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,6 +40,15 @@ AC_ARG_WITH([dtb],
AS_HELP_STRING([--with-dtb], [Specify a particular DTB to use]),
[KERN_DTB="$withval"])
 
+AC_ARG_WITH([xen],
+   AS_HELP_STRING([--with-xen], [Compile for Xen, and Specify a particular 
Xen to use]),
+   X_IMAGE=$withval)
+
+AS_IF([test "x$X_IMAGE" == "x"], [],
+   [AC_CHECK_FILE([$X_IMAGE], [], AC_MSG_ERROR([No such file or directory: 
$X_IMAGE]))])
+AC_SUBST([XEN_IMAGE], [$X_IMAGE])
+AM_CONDITIONAL([XEN], [test "x$X_IMAGE" != "x"])
+
 # Ensure that the user has provided us with a sane kernel dir.
 m4_define([CHECKFILES], [KERN_DIR,
KERN_DTB,
@@ -50,6 +59,10 @@ m4_foreach([checkfile], [CHECKFILES],
 
 AC_SUBST([KERNEL_IMAGE], [$KERN_IMAGE])
 AC_SUBST([KERNEL_DTB], [$KERN_DTB])
+AS_IF([test "x$X_IMAGE" != "x"],
+  [AC_SUBST([IMAGE], ["xen-system.axf"])],
+  [AC_SUBST([IMAGE], ["linux-system.axf"])]
+)
 
 # Allow a user to pass --enable-psci
 AC_ARG_ENABLE([psci],
@@ -119,4 +132,5 @@ echo "  CPU IDs:   ${CPU_IDS}"
 echo "  Use GICv3? ${USE_GICV3}"
 echo "  Boot-wrapper 

[Xen-devel] [PATCH 4/4] Explicitly clean linux-system.axf and xen-system.axf

[Andre Przywara]

From: Christoffer Dall 

When doing a make clean, only the output image currently configured to
build is being removed.  However, one would expect all build artifacts
to be removed when doing a 'make clean' and when switching between Xen
and Linux builds, it is easy to accidentally run an older build than
intended.  Simply hardcode the axf image file names.

Signed-off-by: Christoffer Dall 
Signed-off-by: Andre Przywara 
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index d83b417..cd022e9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -131,7 +131,7 @@ OFILES  += $(addprefix $(ARCH_SRC),boot.o 
stack.o $(BOOTMETHOD) utils.o)
 
 all: $(IMAGE) $(XIMAGE)
 
-CLEANFILES = $(IMAGE) $(OFILES) model.lds fdt.dtb
+CLEANFILES = $(IMAGE) linux-system.axf xen-system.axf $(OFILES) model.lds 
fdt.dtb
 
 $(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) 
$(XEN_IMAGE)
$(LD) $(LDFLAGS) $(OFILES) -o $@ --script=model.lds
-- 
2.9.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 0/4] boot-wrapper: arm64: Xen support

[Andre Przywara]

These patches allow to include a Xen hypervisor binary into a boot-wrapper
ELF file, so that a Foundation Platform or a Fast Model can boot a Xen
system (including a Dom0 kernel).
This has been floating around for a while, I just updated the patches 
to apply on the latest boot-wrapper tree. Also I increased Xen's load
address to accomodate for Dom0 kernels bigger than 16 MB.
For testing this just add: "--with-xen=/path/to/xen/xen/xen" to the
./configure command line and feed the resulting xen-system.axf file to
the model.

Cheers,
Andre.

Christoffer Dall (3):
  Support for building in a Xen binary
  Xen: Support adding DT nodes
  Explicitly clean linux-system.axf and xen-system.axf

Ian Campbell (1):
  Xen: Select correct dom0 console

 .gitignore|  1 +
 Makefile.am   | 38 +-
 boot_common.c |  4 ++--
 configure.ac  | 26 +-
 model.lds.S   | 14 ++
 5 files changed, 67 insertions(+), 16 deletions(-)

-- 
2.9.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Daniel De Graaf]

On 06/20/2016 10:35 AM, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

This operation has no known users, and is primarily useful when an MLS
policy is in use (which has never been shipped with Xen).  In addition,
the information it provides does not actually depend on hypervisor
state (only on the XSM policy), so an application that needs it could
compute the results without needing to involve the hypervisor.



So if I read this language correctly. Removing this does not affect
someone being able to build a MLS policy at a later date right?


Correct; that support is still there.  This hypercall was used to
compute a list of reachable security contexts for a given user, which
is trivial in a non-MLS policy but more complex when one is being
used.  This computation makes more sense on Linux (where creating
new contexts via "exec" is common) than on Xen (where normally a
domain cannot create another).

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 6/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...

[Jan Beulich]

>>> On 20.06.16 at 15:37,  wrote:
> to avoid mixing machine frame with guest frame.
> 
> Signed-off-by: Julien Grall 

Irrespective whether you address the missing mfn_add() (which
are really benign):
Acked-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

[Julien Grall]

Hi Andrew,

On 20/06/16 15:53, Andrew Cooper wrote:

On 20/06/16 14:37, Julien Grall wrote:

p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
the variable to *gfn* and use typesafe to avoid possible misusage.

Signed-off-by: Julien Grall 


On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long.

Is the truncation ok?


The PFN will be encoded on 28 bits maximum (40 bits address). Unless we 
want to check that the guest effectively zeroed the unused bits, I think 
the truncation is fine.


FWIW, this is not the only place where the truncation xen_pfn_t  (aka 
uin64_t) -> unsigned long happens.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Boris Ostrovsky]

On 06/20/2016 09:55 AM, Jan Beulich wrote:
 On 20.06.16 at 15:36,  wrote:
>> On 06/20/2016 09:30 AM, Jan Beulich wrote:
>> On 20.06.16 at 14:58,  wrote:
 On 06/20/2016 04:56 AM, Jan Beulich wrote:
 On 20.06.16 at 00:03,  wrote:
>> Follow up on 
>> http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
>> Using 
>> http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>> reference.
>>
>>  New value
>>  |---|
>>
>> f1 f5
>> |---||-|
>> f2 f4
>>   |-|f3   |-|
>>  |-|
>>
>> Given a new value of the type above, Current logic will not
>> allow applying parts of the new value overlapping with f3 filter.
>> only f2 and f4.
> I remains unclear what f1...f5 are meant to represent here.
>
>> This change allows all 3 types of overlapes to be included.
>> More specifically for passthrough an Industrial Ethernet Interface
>> (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
>> Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
>> given a quirk to allow read/write for that field is already in place.
>> Device driver logic is such that the entire confspace  is
>> written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
>> arriving together in one call to xen_pcibk_config_write.
> Tweaks to make individual devices work are dubious. Any
> explanation should outline why current behavior is _generally_
> wrong. In particular with the original issue being with the
> Latency Timer field, and with that field now being allowed to
> be written by its entry in header_common[], ...
>
>> --- a/drivers/xen/xen-pciback/conf_space.c
>> +++ b/drivers/xen/xen-pciback/conf_space.c
>> @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
>> offset, int size, u32 value)
>>  field_start = OFFSET(cfg_entry);
>>  field_end = OFFSET(cfg_entry) + field->size;
>>
>> -if ((req_start >= field_start && req_start < field_end)
>> -|| (req_end > field_start && req_end <= field_end)) 
>> {
>> + if (req_end >= field_start || field_end >= req_start) {
>>  tmp_val = 0;
> ... any change to the logic here which results in writes to the field
> getting issued would seem wrong without even looking at the
> particular nature of the field.
>
> As to the actual change - the two _end variables hold values
> pointing _past_ the region of interest, so the use of >= seems
> wrong here (ought to be >). And in the end we're talking of a
> classical overlap check here, which indeed can be simplified (to
> just two comparisons), but such simplification mustn't result in a
> change of behavior. (Such a simplification would end up being
>
>   if (req_end > field_start && field_end > req_start) {
>
> afaict - note the || instead of && used in your change.)
 Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and
 adding a proper comment) solve this problem?
>>> How would that work? We mean to not allow writes, or else we
>>> could simply add a .u.b.write handler for PCI_LATENCY_TIMER.
>> I thought you suggested (in another thread) to make PCI_LATENCY_TIMER 
>> writable, just like PCI_CACHE_LINE_SIZE?
> Well, I did put this up for discussion (as much as I questioned
> whether Cache Line Size should perhaps not be writable). And if
> we wanted to make it writable, then we should do so the ordinary
> way, not via some hacks. 

That depends on how we want to view header_common's offset field:
whether it's address of a specific register (which is what it is now) or
it's just an offset and length of an access within config space (which
AFAIUI is how the driver in question wants to treat config space) and
it's up to read/write ops to sort out specifics if necessary.

> (And looking at it again I don't even
> understand why Cache Line Size has an entry in header_common[]
> - identical behavior would result with the entry dropped afaict.)

I think so too.

-boris
>



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Andrew Cooper]

On 20/06/16 15:50, Daniel De Graaf wrote:
> On 06/20/2016 10:35 AM, Andrew Cooper wrote:
>> On 20/06/16 15:27, Doug Goldstein wrote:
>>> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
 These permissions were initially split because they were in separate
 domctls, but this split is very unlikely to actually provide security
 benefits: it would require a carefully contrived situation for a
 domain
 to both need access to one type of CPU register and also need to be
 prohibited from accessing another type.

 Signed-off-by: Daniel De Graaf 
 Reviewed-by: Konrad Rzeszutek Wilk 
>>> I'm a:
>>>
>>> Reviewed-by: Doug Goldstein 
>>>
>>> But I'd like to see Andrew Cooper's R-b or comments as well.
>>>
>>
>> I agree.  I can't see a plausible usecase for an entity being entitled
>> to read vcpu content, but not to modify it.
>>
>> Reviewed-by: Andrew Cooper 
>
> That's not exactly what this patch does: the get and set permissions
> are still split, but unified across the different types of registers.
> Where previously there were 6 permissions, now there are 2.

The boundaries for those hypercalls were somewhat arbitrary, and
definitely awkward to use.  Some information is duplicated between
them.  I plan to make them all disappear, in favour of something more
consistent when altering the migration stream semantics.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

[Andrew Cooper]

On 20/06/16 14:37, Julien Grall wrote:
> p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
> the variable to *gfn* and use typesafe to avoid possible misusage.
>
> Signed-off-by: Julien Grall 

On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long.

Is the truncation ok?

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> This adds a Kconfig option and support for including the XSM policy from
> tools/flask/policy in the hypervisor so that the bootloader does not
> need to provide a policy to get sane behavior from an XSM-enabled
> hypervisor.  The policy provided by the bootloader, if present, will
> override the built-in policy.
> 
> Enabling this option only builds the policy if checkpolicy is available
> during compilation of the hypervisor; otherwise, it does nothing.  The
> XSM policy is not moved out of tools because that remains the primary
> location for installing and configuring the policy.
> 
> Signed-off-by: Daniel De Graaf 
> Reviewed-by: Konrad Rzeszutek Wilk 

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH XTF v2] tests: add fep test

[Wei Liu]

Signed-off-by: Wei Liu 
---
v2:
1. Add test to index file
2. Only test hvm32 environment
3. Add more description
---
 docs/all-tests.dox |  2 ++
 tests/fep/Makefile | 12 
 tests/fep/main.c   | 34 ++
 3 files changed, 48 insertions(+)
 create mode 100644 tests/fep/Makefile
 create mode 100644 tests/fep/main.c

diff --git a/docs/all-tests.dox b/docs/all-tests.dox
index 3a64b93..60ba484 100644
--- a/docs/all-tests.dox
+++ b/docs/all-tests.dox
@@ -41,5 +41,7 @@ Coveres XSA-106 and XSA-156.
 
 @subpage test-cpuid - Print CPUID information.
 
+@subpage test-fep - Test availability of HVM Forced Emulation Prefix.
+
 @subpage test-msr - Print MSR information.
 */
diff --git a/tests/fep/Makefile b/tests/fep/Makefile
new file mode 100644
index 000..e9410c2
--- /dev/null
+++ b/tests/fep/Makefile
@@ -0,0 +1,12 @@
+MAKEFLAGS += -r
+ROOT := $(abspath $(CURDIR)/../..)
+
+include $(ROOT)/build/common.mk
+
+NAME  := fep
+CATEGORY  := utility
+TEST-ENVS := hvm32
+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/fep/main.c b/tests/fep/main.c
new file mode 100644
index 000..de00461
--- /dev/null
+++ b/tests/fep/main.c
@@ -0,0 +1,34 @@
+/**
+ * @file tests/fep/main.c
+ * @ref test-fep
+ *
+ * @page test-fep FEP
+ *
+ * Test the availability of HVM Forced Emulation Prefix (FEP), which
+ * allows HVM guest arbitrarily exercise the instruction emulator.
+ *
+ * Returns SUCCESS if FEP is available, FAILURE if not.
+ *
+ * @sa tests/fep/main.c
+ */
+#include 
+
+void test_main(void)
+{
+printk("Test availability of HVM forced emulation prefix\n");
+
+if ( xtf_has_fep )
+xtf_success(NULL);
+else
+xtf_failure(NULL);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> This operation has no known users, and is primarily useful when an MLS
> policy is in use (which has never been shipped with Xen).  In addition,
> the information it provides does not actually depend on hypervisor
> state (only on the XSM policy), so an application that needs it could
> compute the results without needing to involve the hypervisor.
> 

So if I read this language correctly. Removing this does not affect
someone being able to build a MLS policy at a later date right?

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [seabios test] 95995: tolerable FAIL - PUSHED

[osstest service owner]

flight 95995 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/95995/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95197

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass

version targeted for testing:
 seabios  0e21548b15e25e010c362ea13d170c61a3fcc899
baseline version:
 seabios  d97c200c6e2bf69f32857937e1f278134b392e2a

Last test of basis95197  2016-06-02 13:13:00 Z   18 days
Testing same since95995  2016-06-20 08:44:28 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  pass
 test-amd64-amd64-qemuu-nested-intel  pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-xl-qemuu-winxpsp3   pass
 test-amd64-i386-xl-qemuu-winxpsp3pass



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

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

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

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


Pushing revision :

+ branch=seabios
+ revision=0e21548b15e25e010c362ea13d170c61a3fcc899
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push seabios 
0e21548b15e25e010c362ea13d170c61a3fcc899
+ branch=seabios
+ revision=0e21548b15e25e010c362ea13d170c61a3fcc899
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=seabios
+ xenbranch=xen-unstable
+ '[' xseabios = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' x0e21548b15e25e010c362ea13d170c61a3fcc899 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ 

Re: [Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Daniel De Graaf]

On 06/20/2016 10:35 AM, Andrew Cooper wrote:

On 20/06/16 15:27, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

These permissions were initially split because they were in separate
domctls, but this split is very unlikely to actually provide security
benefits: it would require a carefully contrived situation for a domain
to both need access to one type of CPU register and also need to be
prohibited from accessing another type.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 

I'm a:

Reviewed-by: Doug Goldstein 

But I'd like to see Andrew Cooper's R-b or comments as well.



I agree.  I can't see a plausible usecase for an entity being entitled
to read vcpu content, but not to modify it.

Reviewed-by: Andrew Cooper 


That's not exactly what this patch does: the get and set permissions
are still split, but unified across the different types of registers.
Where previously there were 6 permissions, now there are 2.

A use case where you would be entitled to read but not modify is a
monitoring domain (remote virus scanner, for example) which needs
read access to scan but does not do remediation itself.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH XTF v2] tests: add fep test

[Andrew Cooper]

On 20/06/16 15:44, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Reviewed-by: Andrew Cooper  and committed.
Thanks.

~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 16/17] xen: Make FLASK_AVC_STATS kconfig option visible

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> Signed-off-by: Daniel De Graaf 

Reviewed-by: Doug Goldstein 


> ---
>  xen/common/Kconfig | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 6a51fd5..8fb5a68 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -134,9 +134,14 @@ config FLASK
>  
>  config FLASK_AVC_STATS
>   def_bool y
> + prompt "Maintain statistics on the FLASK access vector cache" if EXPERT 
> = "y"

This is what I was thinking you need to do for patch 13.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/17] xsm: clean up unregistration

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> The only possible value of original_ops was _xsm_ops, and
> unregister_xsm was never used.
> 
> Signed-off-by: Daniel De Graaf 
> Reviewed-by: Andrew Cooper 
> Reviewed-by: Konrad Rzeszutek Wilk 

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 02/11] hvmctl: convert HVMOP_set_pci_intx_level

[Ian Jackson]

Daniel De Graaf writes ("Re: [PATCH 02/11] hvmctl: convert 
HVMOP_set_pci_intx_level"):
> On 06/20/2016 08:53 AM, Jan Beulich wrote:
> > Note that this adds validation of the "domain" interface structure
> > field, which previously got ignored.
> >
> > Note further that this retains the hvmop interface definitions as those
> > had (wrongly) been exposed to non-tool stack consumers (albeit the
> > operation wouldn't have succeeded when requested by a domain for
> > itself).
> >
> > Signed-off-by: Jan Beulich 
> > ---
> > TBD: xen/xsm/flask/policy/access_vectors says "also needs hvmctl", but
> >  I don't see how this has been done so far. With the change here,
> >  doing two checks in flask_hvm_control() (the generic one always
> >  and a specific one if needed) would of course be simple, but it's
> >  unclear how subsequently added sub-ops should then be dealt with
> >  (which don't have a similar remark).
> 
> I am not sure why that remark is there: it seems like it refers to an
> overall check in the HVM operation hypercall, which does not exist.
> 
> There is no reason to have an operation protected by two different
> access checks, so I think that both the previous and patched code
> are correct and the "also needs hvmctl" comment should be removed.
> With that, Acked-by: Daniel De Graaf 

This is a slight digression, but is it intended that all of these
hvmctl's are safe to expose to a deprivileged device model process in
dom0, or to a device model stub domain ?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> Since enabling XSM is required to enable FLASK, place the option for
> FLASK below the one for XSM.  In addition, since it does not make sense
> to enable XSM without any XSM providers, and FLASK is the only XSM
> provider, hide the option to disable FLASK under EXPERT.
> 
> Signed-off-by: Daniel De Graaf 
> ---

> @@ -137,6 +119,25 @@ config XSM
>  
> If unsure, say N.
>  
> +config FLASK
> + def_bool y
> + bool "FLux Advanced Security Kernel support" if EXPERT = "y"

Ok. Here's the real review. I think you want the prompt to be optional
if EXPERT is enabled then I think you need to use "prompt" instead of
"bool". You've already got this set to a bool with the "def_bool" line.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Doug Goldstein]

On 6/20/16 9:41 AM, Doug Goldstein wrote:
> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
>> Since enabling XSM is required to enable FLASK, place the option for
>> FLASK below the one for XSM.  In addition, since it does not make sense
>> to enable XSM without any XSM providers, and FLASK is the only XSM
>> provider, hide the option to disable FLASK under EXPERT.
>>
>> Signed-off-by: Daniel De Graaf 
>> ---
>>  xen/common/Kconfig | 37 +++--
>>  1 file changed, 19 insertions(+), 18 deletions(-)
>>
>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>> index cd59574..6a51fd5 100644
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -11,24 +11,6 @@ config COMPAT
>>  config CORE_PARKING
>>  bool
>>  
>> -config FLASK
>> -bool "FLux Advanced Security Kernel support"
>> -default y
>> -depends on XSM
>> ----help---
>> -  Enables the FLASK (FLux Advanced Security Kernel) support which
>> -  provides a mandatory access control framework by which security
>> -  enforcement, isolation, and auditing can be achieved with fine
>> -  granular control via a security policy.
>> -
>> -  If unsure, say N.
>> -
> 
> 
> 
>>  
>> +config FLASK
>> +def_bool y
>> +bool "FLux Advanced Security Kernel support" if EXPERT = "y"
> 
> Why did FLASK become dependent on EXPERT? It wasn't previously.
> 

Gah. Helps to read the commit message. Ignore the noise.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> Since enabling XSM is required to enable FLASK, place the option for
> FLASK below the one for XSM.  In addition, since it does not make sense
> to enable XSM without any XSM providers, and FLASK is the only XSM
> provider, hide the option to disable FLASK under EXPERT.
> 
> Signed-off-by: Daniel De Graaf 
> ---
>  xen/common/Kconfig | 37 +++--
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index cd59574..6a51fd5 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -11,24 +11,6 @@ config COMPAT
>  config CORE_PARKING
>   bool
>  
> -config FLASK
> - bool "FLux Advanced Security Kernel support"
> - default y
> - depends on XSM
> - ---help---
> -   Enables the FLASK (FLux Advanced Security Kernel) support which
> -   provides a mandatory access control framework by which security
> -   enforcement, isolation, and auditing can be achieved with fine
> -   granular control via a security policy.
> -
> -   If unsure, say N.
> -



>  
> +config FLASK
> + def_bool y
> + bool "FLux Advanced Security Kernel support" if EXPERT = "y"

Why did FLASK become dependent on EXPERT? It wasn't previously.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 6/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...

[Julien Grall]

On 20/06/16 14:37, Julien Grall wrote:

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index a3add21..2710ce8 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2214,9 +2214,9 @@ static unsigned int mmio_order(const struct domain *d,
  #define MAP_MMIO_MAX_ITER 64 /* pretty arbitrary */

  int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
   unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
  {
  int ret = 0;
  unsigned long i;
@@ -2229,10 +2229,11 @@ int map_mmio_regions(struct domain *d,
i += 1UL << order, ++iter )
  {
  /* OR'ing gfn and mfn values will return an order suitable to both. */
-for ( order = mmio_order(d, (start_gfn + i) | (mfn + i), nr - i); ;
+for ( order = mmio_order(d, (gfn_x(start_gfn) + i) | (mfn_x(mfn) + i), 
nr - i); ;
order = ret - 1 )
  {
-ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i), order,
+ret = set_mmio_p2m_entry(d, gfn_x(start_gfn) + i,
+ _mfn(mfn_x(mfn) + i), order,


Hmm I forgot to convert this one to mfn_add(mfn, i).


--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Andrew Cooper]

On 20/06/16 15:27, Doug Goldstein wrote:
> On 6/20/16 9:04 AM, Daniel De Graaf wrote:
>> These permissions were initially split because they were in separate
>> domctls, but this split is very unlikely to actually provide security
>> benefits: it would require a carefully contrived situation for a domain
>> to both need access to one type of CPU register and also need to be
>> prohibited from accessing another type.
>>
>> Signed-off-by: Daniel De Graaf 
>> Reviewed-by: Konrad Rzeszutek Wilk 
> I'm a:
>
> Reviewed-by: Doug Goldstein 
>
> But I'd like to see Andrew Cooper's R-b or comments as well.
>

I agree.  I can't see a plausible usecase for an entity being entitled
to read vcpu content, but not to modify it.

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 3/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers

[Jan Beulich]

>>> On 20.06.16 at 15:37,  wrote:
> Also rename some variables to gfn or mfn when it does not require much
> rework.
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 
with one remark:

> @@ -787,39 +792,39 @@ guest_physmap_add_entry(struct domain *d, unsigned long 
> gfn,
>  /* Then, look for m->p mappings for this range and deal with them */
>  for ( i = 0; i < (1UL << page_order); i++ )
>  {
> -if ( page_get_owner(mfn_to_page(_mfn(mfn + i))) == dom_cow )
> +if ( page_get_owner(mfn_to_page(mfn_add(mfn, i))) == dom_cow )
>  {
>  /* This is no way to add a shared page to your physmap! */
> -gdprintk(XENLOG_ERR, "Adding shared mfn %lx directly to dom %hu 
> "
> -"physmap not allowed.\n", mfn+i, d->domain_id);
> +gdprintk(XENLOG_ERR, "Adding shared mfn %lx directly to dom %hu 
> physmap not allowed.\n",
> + mfn_x(mfn_add(mfn, i)), d->domain_id);

The %hu here would better become %d (and perhaps the space
ahead of it also removed).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 02/11] hvmctl: convert HVMOP_set_pci_intx_level

[Daniel De Graaf]

On 06/20/2016 08:53 AM, Jan Beulich wrote:

Note that this adds validation of the "domain" interface structure
field, which previously got ignored.

Note further that this retains the hvmop interface definitions as those
had (wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich 
---
TBD: xen/xsm/flask/policy/access_vectors says "also needs hvmctl", but
 I don't see how this has been done so far. With the change here,
 doing two checks in flask_hvm_control() (the generic one always
 and a specific one if needed) would of course be simple, but it's
 unclear how subsequently added sub-ops should then be dealt with
 (which don't have a similar remark).


I am not sure why that remark is there: it seems like it refers to an
overall check in the HVM operation hypercall, which does not exist.

There is no reason to have an operation protected by two different
access checks, so I think that both the previous and patched code
are correct and the "also needs hvmctl" comment should be removed.
With that, Acked-by: Daniel De Graaf 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 09/17] flask: remove unused AVC callback functions

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> These callbacks are not used in Xen.
> 
> Signed-off-by: Daniel De Graaf 

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH XTF] tests: add fep test

[Wei Liu]

On Mon, Jun 20, 2016 at 02:43:47PM +0100, Andrew Cooper wrote:
> On 17/06/16 15:21, Wei Liu wrote:
> > Signed-off-by: Wei Liu 
> 
> LGTM, although a couple of comments.
> 
> > ---
> >  tests/fep/Makefile | 12 
> >  tests/fep/main.c   | 31 +++
> 
> Please add this to the test index in docs/all-tests.dox
> 

Done.

> >  2 files changed, 43 insertions(+)
> >  create mode 100644 tests/fep/Makefile
> >  create mode 100644 tests/fep/main.c
> >
> > diff --git a/tests/fep/Makefile b/tests/fep/Makefile
> > new file mode 100644
> > index 000..8702123
> > --- /dev/null
> > +++ b/tests/fep/Makefile
> > @@ -0,0 +1,12 @@
> > +MAKEFLAGS += -r
> > +ROOT := $(abspath $(CURDIR)/../..)
> > +
> > +include $(ROOT)/build/common.mk
> > +
> > +NAME  := fep
> > +CATEGORY  := utility
> > +TEST-ENVS := $(HVM_ENVIRONMENTS)
> 
> This really doesn't need to be all HVM environments.  FEP is a property
> of the HVM container, not of the running mode of the domain.  hvm32
> would be fine here, and the most simple option.
> 

Done.

> > +
> > +obj-perenv += main.o
> > +
> > +include $(ROOT)/build/gen.mk
> > diff --git a/tests/fep/main.c b/tests/fep/main.c
> > new file mode 100644
> > index 000..34a93c0
> > --- /dev/null
> > +++ b/tests/fep/main.c
> > @@ -0,0 +1,31 @@
> > +/**
> > + * @file tests/fep/main.c
> > + * @ref test-fep
> > + *
> > + * @page test-fep FEP
> > + *
> > + * Returns SUCCESS if FEP is available, FAILURE if not.
> 
> This is the content one will find from the test index, and as such,
> should be the most complete.  At the very least, I would add a sentence
> explaining what FEP is.
> 

Sure.

V2 coming soon.

Wei.

> ~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Doug Goldstein]

On 6/20/16 9:04 AM, Daniel De Graaf wrote:
> These permissions were initially split because they were in separate
> domctls, but this split is very unlikely to actually provide security
> benefits: it would require a carefully contrived situation for a domain
> to both need access to one type of CPU register and also need to be
> prohibited from accessing another type.
> 
> Signed-off-by: Daniel De Graaf 
> Reviewed-by: Konrad Rzeszutek Wilk 

I'm a:

Reviewed-by: Doug Goldstein 

But I'd like to see Andrew Cooper's R-b or comments as well.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 2/8] xen/mm: Introduce a bunch of helpers for the typesafes mfn and gfn

[Jan Beulich]

>>> On 20.06.16 at 15:37,  wrote:
> Those helpers will be useful to do common operations without having to
> unbox/box manually the GFNs/MFNs.
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 

But - since iirc Andrew asked for it during v1 review - any particular
reason you made these macros rather than inline functions?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/8] x86/time: calibrate TSC against platform timer

[Andrew Cooper]

On 15/06/16 11:28, Jan Beulich wrote:
> --- a/xen/arch/x86/i8259.c
> +++ b/xen/arch/x86/i8259.c
> @@ -359,12 +359,7 @@ void __init init_IRQ(void)
>  
>  apic_intr_init();
>  
> -/* Set the clock to HZ Hz */
> -#define CLOCK_TICK_RATE 1193182 /* crystal freq (Hz) */
> -#define LATCH (((CLOCK_TICK_RATE)+(HZ/2))/HZ)
> -outb_p(0x34, PIT_MODE);/* binary, mode 2, LSB/MSB, ch 0 */
> -outb_p(LATCH & 0xff, PIT_CH0); /* LSB */
> -outb(LATCH >> 8, PIT_CH0); /* MSB */
> +preinit_pit();

This highlights the fact that we have two unconditional calls to
preinit_pit() during startup, which is one too many.

init_IRQ() is called rather earlier than early_time_init(), but I can't
spot anything inbetween the two calls which would require the PIT to be
set up.  AFAICT, it is safe to just drop the preinit_pit() call here.

> @@ -340,7 +348,8 @@ static struct platform_timesource __init
>  .frequency = CLOCK_TICK_RATE,
>  .read_counter = read_pit_count,
>  .counter_bits = 32,
> -.init = init_pit
> +.init = init_pit,
> +.resume = resume_pit

Please add a redundant comma at the end, to reduce the next diff to
change this block.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Jan Beulich]

>>> On 20.06.16 at 16:04,  wrote:
> This operation has no known users, and is primarily useful when an MLS
> policy is in use (which has never been shipped with Xen).  In addition,
> the information it provides does not actually depend on hypervisor
> state (only on the XSM policy), so an application that needs it could
> compute the results without needing to involve the hypervisor.
> 
> Signed-off-by: Daniel De Graaf 

Non-flask part
Acked-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 05/17] flask/policy: xenstore stubdom policy

[Daniel De Graaf]

This adds the xenstore_t type to the example policy for use by a
xenstore stub domain; see the init-xenstore-domain tool for how this
type needs to be used.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Doug Goldstein 
---
 tools/flask/policy/modules/modules.conf |  3 +++
 tools/flask/policy/modules/xenstore.te  | 24 
 2 files changed, 27 insertions(+)
 create mode 100644 tools/flask/policy/modules/xenstore.te

diff --git a/tools/flask/policy/modules/modules.conf 
b/tools/flask/policy/modules/modules.conf
index 9aac6a0..6dba0a3 100644
--- a/tools/flask/policy/modules/modules.conf
+++ b/tools/flask/policy/modules/modules.conf
@@ -33,6 +33,9 @@ nomigrate = on
 # Example device policy.  Also see policy/device_contexts.
 nic_dev = on
 
+# Xenstore stub domain (see init-xenstore-domain).
+xenstore = on
+
 # This allows any domain type to be created using the system_r role.  When it 
is
 # disabled, domains not using the default types (dom0_t, domU_t, dm_dom_t) must
 # use another role (such as vm_r from the vm_role module below).
diff --git a/tools/flask/policy/modules/xenstore.te 
b/tools/flask/policy/modules/xenstore.te
new file mode 100644
index 000..519566a
--- /dev/null
+++ b/tools/flask/policy/modules/xenstore.te
@@ -0,0 +1,24 @@
+
+#
+# Xenstore stubdomain
+#
+
+declare_singleton_domain(xenstore_t)
+create_domain(dom0_t, xenstore_t)
+manage_domain(dom0_t, xenstore_t)
+
+# Xenstore requires the global VIRQ for domain destroy operations
+allow dom0_t xenstore_t:domain set_virq_handler;
+# Current xenstore stubdom uses the hypervisor console, not "xl console"
+allow xenstore_t xen_t:xen writeconsole;
+# Xenstore queries domaininfo on all domains
+allow xenstore_t domain_type:domain getdomaininfo;
+
+# As a shortcut, the following 3 rules are used instead of adding a 
domain_comms
+# rule between xenstore_t and every domain type that talks to xenstore
+create_channel(xenstore_t, domain_type, xenstore_t_channel)
+allow event_type xenstore_t: event bind;
+allow xenstore_t domain_type:grant { map_read map_write unmap };
+
+# Xenstore is a utility domain, so it should use the system role
+role system_r types xenstore_t;
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 02/17] flask/policy: split out rules for system_r

[Daniel De Graaf]

When the all_system_role module is enabled, any domain type can be
created using the system_r role, which was the default.  When it is
disabled, domains not using the default types (dom0_t and domU_t) must
use another role such as vm_r.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Doug Goldstein 
---
 tools/flask/policy/modules/all_system_role.te |  8 
 tools/flask/policy/modules/domU.te|  3 +++
 tools/flask/policy/modules/modules.conf   |  5 +
 tools/flask/policy/modules/xen.te | 11 +++
 4 files changed, 19 insertions(+), 8 deletions(-)
 create mode 100644 tools/flask/policy/modules/all_system_role.te

diff --git a/tools/flask/policy/modules/all_system_role.te 
b/tools/flask/policy/modules/all_system_role.te
new file mode 100644
index 000..74f870f
--- /dev/null
+++ b/tools/flask/policy/modules/all_system_role.te
@@ -0,0 +1,8 @@
+# Allow all domains to use system_r so that systems that are not using the
+# user/role separation feature will work properly.
+role system_r types domain_type;
+
+# The vm role is used as part of user separation.  Allow all domain types to 
use
+# this role except dom0.
+role vm_r;
+role vm_r types { domain_type -dom0_t };
diff --git a/tools/flask/policy/modules/domU.te 
b/tools/flask/policy/modules/domU.te
index ca5eecd..b77df29 100644
--- a/tools/flask/policy/modules/domU.te
+++ b/tools/flask/policy/modules/domU.te
@@ -23,3 +23,6 @@ make_device_model(dom0_t, dm_dom_t, domU_t)
 
 # This is required for PCI (or other device) passthrough
 delegate_devices(dom0_t, domU_t)
+
+# Both of these domain types can be created using the default (system) role
+role system_r types { domU_t dm_dom_t };
diff --git a/tools/flask/policy/modules/modules.conf 
b/tools/flask/policy/modules/modules.conf
index dba4b40..d875dbf 100644
--- a/tools/flask/policy/modules/modules.conf
+++ b/tools/flask/policy/modules/modules.conf
@@ -32,3 +32,8 @@ nomigrate = on
 
 # Example device policy.  Also see policy/device_contexts.
 nic_dev = on
+
+# This allows any domain type to be created using the system_r role.  When it 
is
+# disabled, domains not using the default types (dom0_t and domU_t) must use
+# another role (such as vm_r) from the vm_role module.
+all_system_role = on
diff --git a/tools/flask/policy/modules/xen.te 
b/tools/flask/policy/modules/xen.te
index 3ee5e75..f374dc5 100644
--- a/tools/flask/policy/modules/xen.te
+++ b/tools/flask/policy/modules/xen.te
@@ -78,12 +78,7 @@ neverallow * ~event_type:event { create send status };
 # The object role (object_r) is used for devices, resources, and event 
channels;
 # it does not need to be defined here and should not be used for domains.
 
-# The system role is used for utility domains and pseudo-domains
+# The system role is used for utility domains and pseudo-domains.  If roles are
+# not being used for separation, all domains can use the system role.
 role system_r;
-role system_r types { xen_type domain_type };
-# If you want to prevent domUs from being placed in system_r:
-##role system_r types { xen_type dom0_t };
-
-# The vm role is used for customer virtual machines
-role vm_r;
-role vm_r types { domain_type -dom0_t };
+role system_r types { xen_type dom0_t };
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Daniel De Graaf]

These permissions were initially split because they were in separate
domctls, but this split is very unlikely to actually provide security
benefits: it would require a carefully contrived situation for a domain
to both need access to one type of CPU register and also need to be
prohibited from accessing another type.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 tools/flask/policy/modules/dom0.te  |  1 -
 tools/flask/policy/modules/xen.if   |  7 +++
 xen/xsm/flask/hooks.c   | 20 ++--
 xen/xsm/flask/policy/access_vectors | 16 ++--
 4 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index ef6a986..d228b24 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -34,7 +34,6 @@ allow dom0_t dom0_t:domain {
setvcpucontext max_vcpus setaffinity getaffinity getscheduler
getdomaininfo getvcpuinfo getvcpucontext setdomainmaxmem setdomainhandle
setdebugging hypercall settime setaddrsize getaddrsize trigger
-   getextvcpucontext setextvcpucontext getvcpuextstate setvcpuextstate
getpodtarget setpodtarget set_misc_info set_virq_handler
 };
 allow dom0_t dom0_t:domain2 {
diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index 00d1bbb..fd96303 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -47,9 +47,8 @@ define(`declare_build_label', `
 
 define(`create_domain_common', `
allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
-   getdomaininfo hypercall setvcpucontext setextvcpucontext
-   getscheduler getvcpuinfo getvcpuextstate getaddrsize
-   getaffinity setaffinity setvcpuextstate };
+   getdomaininfo hypercall setvcpucontext getscheduler
+   getvcpuinfo getaddrsize getaffinity setaffinity };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
psr_cmt_op psr_cat_op soft_reset };
@@ -94,7 +93,7 @@ define(`migrate_domain_out', `
allow $1 domxen_t:mmu map_read;
allow $1 $2:hvm { gethvmc getparam irqlevel };
allow $1 $2:mmu { stat pageinfo map_read };
-   allow $1 $2:domain { getaddrsize getvcpucontext getextvcpucontext 
getvcpuextstate pause destroy };
+   allow $1 $2:domain { getaddrsize getvcpucontext pause destroy };
allow $1 $2:domain2 gettsc;
allow $1 $2:shadow { enable disable logdirty };
 ')
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 20d46c8..a8d45e7 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -630,10 +630,16 @@ static int flask_domctl(struct domain *d, int cmd)
 case XEN_DOMCTL_setdomainhandle:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETDOMAINHANDLE);
 
+case XEN_DOMCTL_set_ext_vcpucontext:
+case XEN_DOMCTL_set_vcpu_msrs:
 case XEN_DOMCTL_setvcpucontext:
+case XEN_DOMCTL_setvcpuextstate:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETVCPUCONTEXT);
 
+case XEN_DOMCTL_get_ext_vcpucontext:
+case XEN_DOMCTL_get_vcpu_msrs:
 case XEN_DOMCTL_getvcpucontext:
+case XEN_DOMCTL_getvcpuextstate:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETVCPUCONTEXT);
 
 case XEN_DOMCTL_getvcpuinfo:
@@ -675,20 +681,6 @@ static int flask_domctl(struct domain *d, int cmd)
 case XEN_DOMCTL_pin_mem_cacheattr:
 return current_has_perm(d, SECCLASS_HVM, HVM__CACHEATTR);
 
-case XEN_DOMCTL_set_ext_vcpucontext:
-case XEN_DOMCTL_set_vcpu_msrs:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETEXTVCPUCONTEXT);
-
-case XEN_DOMCTL_get_ext_vcpucontext:
-case XEN_DOMCTL_get_vcpu_msrs:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETEXTVCPUCONTEXT);
-
-case XEN_DOMCTL_setvcpuextstate:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETVCPUEXTSTATE);
-
-case XEN_DOMCTL_getvcpuextstate:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETVCPUEXTSTATE);
-
 case XEN_DOMCTL_sendtrigger:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__TRIGGER);
 
diff --git a/xen/xsm/flask/policy/access_vectors 
b/xen/xsm/flask/policy/access_vectors
index 3d29042..7e69ede 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -111,6 +111,9 @@ class xen2
 class domain
 {
 # XEN_DOMCTL_setvcpucontext
+# XEN_DOMCTL_setvcpuextstate
+# XEN_DOMCTL_set_ext_vcpucontext
+# XEN_DOMCTL_set_vcpu_msrs
 setvcpucontext
 # XEN_DOMCTL_pausedomain
 pause
@@ -142,6 +145,9 @@ class domain
 # XEN_DOMCTL_getvcpuinfo
 getvcpuinfo
 # XEN_DOMCTL_getvcpucontext
+# 

[Xen-devel] [PATCH 12/17] xen/xsm: remove .xsm_initcall.init section

[Daniel De Graaf]

Since FLASK is the only implementation of XSM hooks in Xen, using an
iterated initcall dispatch for setup is overly complex.  Change this to
a direct function call to a globally visible function; if additional XSM
hooks are added in the future, a switching mechanism will be needed
regardless, and that can be placed in xsm_core.c.

Signed-off-by: Daniel De Graaf 
---
 xen/arch/arm/xen.lds.S |  5 -
 xen/arch/x86/xen.lds.S |  5 -
 xen/include/xsm/xsm.h  | 16 
 xen/xsm/flask/hooks.c  |  4 +---
 xen/xsm/xsm_core.c | 13 +
 5 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 76982b2..8320381 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -162,11 +162,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
   } :text
-  .xsm_initcall.init : {
-   __xsm_initcall_start = .;
-   *(.xsm_initcall.init)
-   __xsm_initcall_end = .;
-  } :text
   __init_end_efi = .;
   . = ALIGN(STACK_SIZE);
   __init_end = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index a43b29d..dcbb8fe 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -190,11 +190,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
   } :text
-  .xsm_initcall.init : {
-   __xsm_initcall_start = .;
-   *(.xsm_initcall.init)
-   __xsm_initcall_end = .;
-  } :text
   . = ALIGN(PAGE_SIZE);
   __init_end = .;
 
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8ed8ee5..0d525ec 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -46,14 +46,6 @@ typedef enum xsm_default xsm_default_t;
 extern char *policy_buffer;
 extern u32 policy_size;
 
-typedef void (*xsm_initcall_t)(void);
-
-extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
-
-#define xsm_initcall(fn) \
-static xsm_initcall_t __initcall_##fn \
-__used_section(".xsm_initcall.init") = fn
-
 struct xsm_operations {
 void (*security_domaininfo) (struct domain *d,
 struct xen_domctl_getdomaininfo *info);
@@ -763,6 +755,14 @@ extern int unregister_xsm(struct xsm_operations *ops);
 extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
 
+#ifdef CONFIG_FLASK
+extern void flask_init(void);
+#else
+static inline void flask_init(void)
+{
+}
+#endif
+
 #else /* CONFIG_XSM */
 
 #include 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 543406b..d632b0e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1817,7 +1817,7 @@ static struct xsm_operations flask_ops = {
 .xen_version = flask_xen_version,
 };
 
-static __init void flask_init(void)
+__init void flask_init(void)
 {
 int ret = -ENOENT;
 
@@ -1860,8 +1860,6 @@ static __init void flask_init(void)
 printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
 }
 
-xsm_initcall(flask_init);
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 634ec98..3487742 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,17 +36,6 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }
 
-static void __init do_xsm_initcalls(void)
-{
-xsm_initcall_t *call;
-call = __xsm_initcall_start;
-while ( call < __xsm_initcall_end )
-{
-(*call) ();
-call++;
-}
-}
-
 static int __init xsm_core_init(void)
 {
 if ( verify(_xsm_ops) )
@@ -57,7 +46,7 @@ static int __init xsm_core_init(void)
 }
 
 xsm_ops = _xsm_ops;
-do_xsm_initcalls();
+flask_init();
 
 return 0;
 }
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 06/17] flask/policy: remove unused example

[Daniel De Graaf]

The access vectors defined here have never been used by xenstore.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Doug Goldstein 
---
 tools/flask/policy/policy/access_vectors   | 23 ++-
 tools/flask/policy/policy/security_classes |  1 -
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/tools/flask/policy/policy/access_vectors 
b/tools/flask/policy/policy/access_vectors
index 4fd61f1..d9c69c0 100644
--- a/tools/flask/policy/policy/access_vectors
+++ b/tools/flask/policy/policy/access_vectors
@@ -1,24 +1,5 @@
 # Locally defined access vectors
 #
-# Define access vectors for the security classes defined in security_classes
+# Define access vectors for the security classes defined in security_classes.
+# Access vectors defined in this file should not be used by the hypervisor.
 #
-
-# Note: this is an example; the xenstore daemon provided with Xen does
-# not yet include XSM support, and the exact permissions may be defined
-# differently if such support is added.
-class xenstore {
-   # read from keys owned by the target domain (if permissions allow)
-   read
-   # write to keys owned by the target domain (if permissions allow)
-   write
-   # change permissions of a key owned by the target domain
-   chmod
-   # change the owner of a key which was owned by the target domain
-   chown_from
-   # change the owner of a key to the target domain
-   chown_to
-   # access a key owned by the target domain without permission
-   override
-   # introduce a domain
-   introduce
-}
diff --git a/tools/flask/policy/policy/security_classes 
b/tools/flask/policy/policy/security_classes
index 56595e8..0f0f9f3 100644
--- a/tools/flask/policy/policy/security_classes
+++ b/tools/flask/policy/policy/security_classes
@@ -5,4 +5,3 @@
 # security policy.
 #
 # Access vectors for these classes must be defined in the access_vectors file.
-class xenstore
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 15/17] xsm: clean up unregistration

[Daniel De Graaf]

The only possible value of original_ops was _xsm_ops, and
unregister_xsm was never used.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Andrew Cooper 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/include/xsm/xsm.h|  1 -
 xen/xsm/flask/flask_op.c |  4 +---
 xen/xsm/flask/hooks.c|  3 ---
 xen/xsm/xsm_core.c   | 16 
 4 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 0d525ec..4b8843d 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -750,7 +750,6 @@ extern bool has_xsm_magic(paddr_t);
 #endif
 
 extern int register_xsm(struct xsm_operations *ops);
-extern int unregister_xsm(struct xsm_operations *ops);
 
 extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 3ad4bdc..719c2d7 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -58,8 +58,6 @@ static int flask_security_make_bools(void);
 
 extern int ss_initialized;
 
-extern struct xsm_operations *original_ops;
-
 static void __init parse_flask_param(char *s)
 {
 if ( !strcmp(s, "enforcing") )
@@ -243,7 +241,7 @@ static int flask_disable(void)
 flask_disabled = 1;
 
 /* Reset xsm_ops to the original module. */
-xsm_ops = original_ops;
+xsm_ops = _xsm_ops;
 
 return 0;
 }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index d632b0e..2692a6f 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -35,8 +35,6 @@
 #include 
 #include 
 
-struct xsm_operations *original_ops = NULL;
-
 static u32 domain_sid(struct domain *dom)
 {
 struct domain_security_struct *dsec = dom->ssid;
@@ -1842,7 +1840,6 @@ __init void flask_init(void)
 
 avc_init();
 
-original_ops = xsm_ops;
 if ( register_xsm(_ops) )
 panic("Flask: Unable to register with XSM");
 
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 78d881b..8df1a3c 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -144,22 +144,6 @@ int __init register_xsm(struct xsm_operations *ops)
 return 0;
 }
 
-
-int unregister_xsm(struct xsm_operations *ops)
-{
-if ( ops != xsm_ops )
-{
-printk("%s: trying to unregister "
-   "a security_opts structure that is not "
-   "registered, failing.\n", __FUNCTION__);
-return -EINVAL;
-}
-
-xsm_ops = _xsm_ops;
-
-return 0;
-}
-
 #endif
 
 long do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 04/17] flask/policy: remove unused support for binary modules

[Daniel De Graaf]

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Doug Goldstein 
---
 .../policy/policy/support/loadable_module.spt  | 166 -
 tools/flask/policy/policy/support/misc_macros.spt  |   2 +
 2 files changed, 2 insertions(+), 166 deletions(-)
 delete mode 100644 tools/flask/policy/policy/support/loadable_module.spt

diff --git a/tools/flask/policy/policy/support/loadable_module.spt 
b/tools/flask/policy/policy/support/loadable_module.spt
deleted file mode 100644
index de48b3b..000
--- a/tools/flask/policy/policy/support/loadable_module.spt
+++ /dev/null
@@ -1,166 +0,0 @@
-
-#
-# Macros for switching between source policy
-# and loadable policy module support
-#
-
-##
-#
-# For adding the module statement
-#
-define(`policy_module',`
-   ifdef(`self_contained_policy',`',`
-   module $1 $2;
-
-   require {
-   role system_r;
-   all_kernel_class_perms
-   }
-   ')
-')
-
-##
-#
-# For use in interfaces, to optionally insert a require block
-#
-define(`gen_require',`
-   ifdef(`self_contained_policy',`',`
-   define(`in_gen_require_block')
-   require {
-   $1
-   }
-   undefine(`in_gen_require_block')
-   ')
-')
-
-##
-#
-# In the future interfaces should be in loadable modules
-#
-# template(name,rules)
-#
-define(`template',`
-   `define(`$1',`
-# begin $1(dollarsstar)
-   $2
-# end $1(dollarsstar)
-   '')
-')
-
-# helper function, since m4 wont expand macros
-# if a line is a comment (#):
-define(`policy_m4_comment',`dnl
-# $2 depth: $1
-')dnl
-
-##
-#
-# In the future interfaces should be in loadable modules
-#
-# interface(name,rules)
-#
-define(`interface',`
-   `define(`$1',`
-
-   define(`policy_temp',incr(policy_call_depth))
-   pushdef(`policy_call_depth',policy_temp)
-   undefine(`policy_temp')
-
-   policy_m4_comment(policy_call_depth,begin `$1'(dollarsstar))
-
-   $2
-
-   define(`policy_temp',decr(policy_call_depth))
-   pushdef(`policy_call_depth',policy_temp)
-   undefine(`policy_temp')
-
-   policy_m4_comment(policy_call_depth,end `$1'(dollarsstar))
-
-   '')
-')
-
-define(`policy_call_depth',0)
-
-##
-#
-# Optional policy handling
-#
-define(`optional_policy',`
-   ifdef(`self_contained_policy',`
-   ifdef(`$1',`$2',`$3')
-   ',`
-   optional {
-   $2
-   ifelse(`$3',`',`',`
-   } else {
-   $3
-   ')
-   }
-   ')
-')
-
-##
-#
-# Determine if we should use the default
-# tunable value as specified by the policy
-# or if the override value should be used
-#
-define(`dflt_or_overr',`ifdef(`$1',$1,$2)')
-
-##
-#
-# Extract booleans out of an expression.
-# This needs to be reworked so expressions
-# with parentheses can work.
-
-define(`delcare_required_symbols',`
-ifelse(regexp($1, `\w'), -1, `', `dnl
-bool regexp($1, `\(\w+\)', `\1');
-delcare_required_symbols(regexp($1, `\w+\(.*\)', `\1'))dnl
-') dnl
-')
-
-##
-#
-# Tunable declaration
-#
-define(`gen_tunable',`
-   ifdef(`self_contained_policy',`
-   bool $1 dflt_or_overr(`$1'_conf,$2);
-   ',`
-   # loadable module tunable
-   # declaration will go here
-   # instead of bool when
-   # loadable modules support
-   # tunables
-   bool $1 dflt_or_overr(`$1'_conf,$2);
-   ')
-')
-
-##
-#
-# Tunable policy handling
-#
-define(`tunable_policy',`
-   ifdef(`self_contained_policy',`
-   if (`$1') {
-   $2
-   } else {
-   $3
-   }
-   ',`
-   # structure for tunables
-   # will go here instead of a
-   # conditional when loadable
-   # modules support tunables
-   gen_require(`
-   delcare_required_symbols(`$1')
-   ')
-
-   if (`$1') {
-   $2
-   } else {
-   $3
-   }
-   ')
-')
diff --git a/tools/flask/policy/policy/support/misc_macros.spt 
b/tools/flask/policy/policy/support/misc_macros.spt
index 344f5c4..3116db9 100644
--- a/tools/flask/policy/policy/support/misc_macros.spt
+++ b/tools/flask/policy/policy/support/misc_macros.spt
@@ -61,6 +61,8 @@ define(`gen_all_users',`')
 #
 define(`gen_context',`$1`'ifdef(`enable_mls',`:$2')`'')
 

Re: [Xen-devel] [PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

>>> On 20.06.16 at 15:58,  wrote:
> On 20/06/16 14:49, Jan Beulich wrote:
> On 20.06.16 at 14:54,  wrote:
>>> On 20/06/16 13:48, Jan Beulich wrote:
>>> On 20.06.16 at 14:15,  wrote:
> On 20/06/16 12:04, Jan Beulich wrote:
>> No point in aligning entry points which aren't supposed to be used
>> anyway.
>>
>> Signed-off-by: Jan Beulich 
> Reviewed-by: Andrew Cooper 
 Thanks, but - any thoughts on this part:

 TBD: Might consider simply using "andq $-15,%rsp", delivering an
 uninitialized error code (which shouldn't matter).
>>> I was still considering that part.
>>>
>>> These are entries we never expect to actually take.  At that point, the
>>> small overhead of setting up the error code to 0 is probably better than
>>> leaving it uninitialised.
>> I understand - it's really a matter of balancing the overhead on
>> these paths (which will never have an effect if these entries indeed
>> are unused, and which is of no interest if they are used by due some
>> other flaw) with the (likely negligible, but non-zero) overhead they
>> introduce on _other_ paths (due to cache and TLB consumption). I.e.
>> my goal was to make these unused entries as small as possible. And
>>
>>  andq$-15,%rsp
>>  movl$vector,4(%rsp)
>>
>> (obviously we can't use movb here) is smaller than the current
>>
>>  testb   $8,%spl
>>  jz  1f
>>  pushq   $0
>>  movb$vector,4(%rsp)
>>
>> afaict.
> 
> All of them head to do_reserved_trap() and panic().

Not sure I'm guessing right what you mean to say with that, so I
can only reiterate that I don't care at all how long it takes for
execution to get through this path. All I care about is that this
code be as small as possible, to limit its impact on surrounding
code. But for the few bytes to save here I guess there was
already way to much talk.

> An alternative would be to drop all this entry code, mark the vectors as
> not present in the IDT, and handle #NP[IDT vector] with a slightly
> different error message in do_trap().

Would likely increase overall code size (i.e. the opposite of what
I'd like to achieve here).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 00/17] XSM/FLASK updates for 4.8

[Daniel De Graaf]

Changes from v1:
 - Change c->context and c->sid from arrays to fields when shrinking
 - Keep struct xen_flask_userlist in headers, but guard it with #ifs
 - Split off Kconfig changes into their own patches
 - Add patch 16 (AVC_STATS in Kconfig)
 - Prevent free() of static data in xsm_dt_init

FLASK policy updates:
 [PATCH 01/17] flask/policy: split into modules
 [PATCH 02/17] flask/policy: split out rules for system_r
 [PATCH 03/17] flask/policy: move user definitions and constraints
 [PATCH 04/17] flask/policy: remove unused support for binary modules
 [PATCH 05/17] flask/policy: xenstore stubdom policy
 [PATCH 06/17] flask/policy: remove unused example

Hypervisor updates to the FLASK security server:
 [PATCH 07/17] flask: unify {get,set}vcpucontext permissions
 [PATCH 08/17] flask: remove unused secondary context in ocontext
 [PATCH 09/17] flask: remove unused AVC callback functions
 [PATCH 10/17] flask: remove xen_flask_userlist operation
 [PATCH 11/17] flask: improve unknown permission handling

Hypervisor updates to the XSM framework (and its config):
 [PATCH 12/17] xen/xsm: remove .xsm_initcall.init section
 [PATCH 13/17] xen: fix FLASK dependency in Kconfig
 [PATCH 14/17] xsm: annotate setup functions with __init
 [PATCH 15/17] xsm: clean up unregistration
 [PATCH 16/17] xen: Make FLASK_AVC_STATS kconfig option visible
 [PATCH 17/17] xsm: add a default policy to .init.data

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/arch/arm/xen.lds.S  |  4 
 xen/arch/x86/xen.lds.S  |  5 +
 xen/common/Kconfig  | 17 +
 xen/xsm/flask/Makefile  | 17 +
 xen/xsm/xsm_core.c  | 15 ++-
 7 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index fed732c..c85d1dc 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -704,13 +704,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -141,21 +141,21 @@ only type enforcement is used and the user and role are 
set to system_u and
 system_r for all domains.
 
 The FLASK security framework is mostly configured using a security policy file.
-This policy file is not normally generated during the Xen build process because
-it relies on the SELinux compiler "checkpolicy"; run
-
-   make -C tools/flask/policy
-
-to compile the example policy included with Xen. The policy is generated from
-definition files under this directory. Most changes to security policy will
-involve creating or modifying modules found in tools/flask/policy/modules/.  
The
-modules.conf file there defines what modules are enabled and has short
-descriptions of each module.
-
-The XSM policy file needs to be copied to /boot and loaded as a module by grub.
-The exact position of the module does not matter as long as it is after the Xen
-kernel; it is normally placed either just above the dom0 kernel or at the end.
-Once dom0 is running, the policy can be reloaded using "xl loadpolicy".
+It relies on the SELinux compiler "checkpolicy"; if this is available, the
+policy will be compiled as part of the tools build.  If hypervisor support for 
a
+built-in policy is enabled ("Compile Xen with a built-in security policy"), the
+policy will be built during the hypervisor build.
+
+The policy is generated from definition files in tools/flask/policy.  Most
+changes to security policy will involve creating or modifying modules found in
+tools/flask/policy/modules/.  The modules.conf file there defines what modules
+are enabled and has short descriptions of each module.
+
+If not using the built-in policy, the XSM policy file needs to be copied to
+/boot and loaded as a module by grub.  The exact position and filename of the
+module does not matter as long as 

[Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Daniel De Graaf]

Since enabling XSM is required to enable FLASK, place the option for
FLASK below the one for XSM.  In addition, since it does not make sense
to enable XSM without any XSM providers, and FLASK is the only XSM
provider, hide the option to disable FLASK under EXPERT.

Signed-off-by: Daniel De Graaf 
---
 xen/common/Kconfig | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index cd59574..6a51fd5 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -11,24 +11,6 @@ config COMPAT
 config CORE_PARKING
bool
 
-config FLASK
-   bool "FLux Advanced Security Kernel support"
-   default y
-   depends on XSM
-   ---help---
- Enables the FLASK (FLux Advanced Security Kernel) support which
- provides a mandatory access control framework by which security
- enforcement, isolation, and auditing can be achieved with fine
- granular control via a security policy.
-
- If unsure, say N.
-
-config FLASK_AVC_STATS
-   def_bool y
-   depends on FLASK
-   ---help---
- Maintain statistics on the access vector cache
-
 # Select HAS_DEVICE_TREE if device tree is supported
 config HAS_DEVICE_TREE
bool
@@ -137,6 +119,25 @@ config XSM
 
  If unsure, say N.
 
+config FLASK
+   def_bool y
+   bool "FLux Advanced Security Kernel support" if EXPERT = "y"
+   depends on XSM
+   ---help---
+ Enables FLASK (FLux Advanced Security Kernel) as the access control
+ mechanism used by the XSM framework.  This provides a mandatory access
+ control framework by which security enforcement, isolation, and
+ auditing can be achieved with fine granular control via a security
+ policy.
+
+ If unsure, say Y.
+
+config FLASK_AVC_STATS
+   def_bool y
+   depends on FLASK
+   ---help---
+ Maintain statistics on the access vector cache
+
 # Enable schedulers
 menu "Schedulers"
visible if EXPERT = "y"
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 14/17] xsm: annotate setup functions with __init

[Daniel De Graaf]

These functions were only called from __init functions.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Andrew Cooper 
---
 xen/xsm/dummy.c| 2 +-
 xen/xsm/xsm_core.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 9791ad4..a082b28 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -27,7 +27,7 @@ struct xsm_operations dummy_xsm_ops;
 }  \
 } while (0)
 
-void xsm_fixup_ops (struct xsm_operations *ops)
+void __init xsm_fixup_ops (struct xsm_operations *ops)
 {
 set_to_dummy_if_null(ops, security_domaininfo);
 set_to_dummy_if_null(ops, domain_create);
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 3487742..78d881b 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -127,7 +127,7 @@ bool __init has_xsm_magic(paddr_t start)
 }
 #endif
 
-int register_xsm(struct xsm_operations *ops)
+int __init register_xsm(struct xsm_operations *ops)
 {
 if ( verify(ops) )
 {
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 16/17] xen: Make FLASK_AVC_STATS kconfig option visible

[Daniel De Graaf]

Signed-off-by: Daniel De Graaf 
---
 xen/common/Kconfig | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 6a51fd5..8fb5a68 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -134,9 +134,14 @@ config FLASK
 
 config FLASK_AVC_STATS
def_bool y
+   prompt "Maintain statistics on the FLASK access vector cache" if EXPERT 
= "y"
depends on FLASK
---help---
- Maintain statistics on the access vector cache
+ Maintain counters on the access vector cache that can be viewed using
+ the FLASK_AVC_CACHESTATS sub-op of the xsm_op hypercall.  Disabling
+ this will save a tiny amount of memory and time to update the stats.
+
+ If unsure, say Y.
 
 # Enable schedulers
 menu "Schedulers"
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 01/17] flask/policy: split into modules

[Daniel De Graaf]

This makes it easier to enable or disable parts of the XSM policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Doug Goldstein 
---
 tools/flask/policy/Makefile|  22 +-
 tools/flask/policy/modules/dom0.te |  74 ++
 tools/flask/policy/modules/domU.te |  25 ++
 tools/flask/policy/modules/guest_features.te   |  31 +++
 tools/flask/policy/modules/isolated_domU.te|   7 +
 tools/flask/policy/modules/modules.conf|  34 +++
 tools/flask/policy/modules/nic_dev.te  |  14 ++
 tools/flask/policy/modules/nomigrate.te|   8 +
 tools/flask/policy/modules/prot_domU.te|  13 +
 .../policy/{policy/modules/xen => modules}/xen.if  |   0
 tools/flask/policy/modules/xen.te  |  89 +++
 tools/flask/policy/policy/modules.conf |  15 --
 tools/flask/policy/policy/modules/xen/xen.te   | 272 -
 13 files changed, 302 insertions(+), 302 deletions(-)
 create mode 100644 tools/flask/policy/modules/dom0.te
 create mode 100644 tools/flask/policy/modules/domU.te
 create mode 100644 tools/flask/policy/modules/guest_features.te
 create mode 100644 tools/flask/policy/modules/isolated_domU.te
 create mode 100644 tools/flask/policy/modules/modules.conf
 create mode 100644 tools/flask/policy/modules/nic_dev.te
 create mode 100644 tools/flask/policy/modules/nomigrate.te
 create mode 100644 tools/flask/policy/modules/prot_domU.te
 rename tools/flask/policy/{policy/modules/xen => modules}/xen.if (100%)
 create mode 100644 tools/flask/policy/modules/xen.te
 delete mode 100644 tools/flask/policy/policy/modules.conf
 delete mode 100644 tools/flask/policy/policy/modules/xen/xen.te

diff --git a/tools/flask/policy/Makefile b/tools/flask/policy/Makefile
index 4be921c..b2c2d06 100644
--- a/tools/flask/policy/Makefile
+++ b/tools/flask/policy/Makefile
@@ -37,7 +37,7 @@ POLICY_VER_LIST_HV = 24 30
 
 # policy source layout
 POLDIR := policy
-MODDIR := $(POLDIR)/modules
+MODDIR := modules
 
 # Classes and access vectors defined in the hypervisor. Changes to these 
require
 # a recompile of both the hypervisor and security policy.
@@ -60,7 +60,7 @@ DEV_OCONS := $(POLDIR)/device_contexts
 
 # config file paths
 GLOBALTUN := $(POLDIR)/global_tunables
-MOD_CONF := $(POLDIR)/modules.conf
+MOD_CONF := $(MODDIR)/modules.conf
 
 # checkpolicy can use the #line directives provided by -s for error reporting:
 M4PARAM := -D self_contained_policy -s
@@ -84,22 +84,14 @@ endif
 M4PARAM += -D mls_num_sens=$(MLS_SENS) -D mls_num_cats=$(MLS_CATS)
 
 
-# Find modules
-ALL_LAYERS := $(filter-out $(MODDIR)/CVS,$(shell find $(wildcard $(MODDIR)/*) 
-maxdepth 0 -type d))
-
-# sort here since it removes duplicates, which can happen
-# when a generated file is already generated
-DETECTED_MODS := $(sort $(foreach dir,$(ALL_LAYERS),$(wildcard $(dir)/*.te)))
-
 # modules.conf setting for policy configuration
 MODENABLED := on
 
 # extract settings from modules.conf
-ENABLED_MODS := $(foreach mod,$(shell awk '/^[ \t]*[a-z]/{ if ($$3 == 
"$(MODENABLED)") print $$1 }' $(MOD_CONF) 2> /dev/null),$(subst ./,,$(shell 
find -iname $(mod).te)))
-
-ALL_MODULES := $(filter $(ENABLED_MODS),$(DETECTED_MODS))
+ENABLED_LIST := $(shell awk '/^[ \t]*[a-z]/{ if ($$3 == "$(MODENABLED)") print 
$$1 }' $(MOD_CONF) 2> /dev/null)
 
-ALL_INTERFACES := $(ALL_MODULES:.te=.if)
+ALL_MODULES := $(foreach mod,$(ENABLED_LIST),$(MODDIR)/$(mod).te)
+ALL_INTERFACES := $(wildcard $(ALL_MODULES:.te=.if))
 
 # The order of these files is important
 POLICY_SECTIONS := $(SECCLASS) $(ISID_DECLS) $(AVS)
@@ -118,8 +110,8 @@ install: $(POLICY_FILENAME)
 $(POLICY_FILENAME): policy.conf
$(CHECKPOLICY) $(CHECKPOLICY_PARAM) $^ -o $@
 
-policy.conf: $(POLICY_SECTIONS)
-   $(M4) $(M4PARAM) $^ > $@
+policy.conf: $(POLICY_SECTIONS) $(MOD_CONF)
+   $(M4) $(M4PARAM) $(POLICY_SECTIONS) > $@
 
 clean:
$(RM) tmp policy.conf $(POLICY_FILENAME)
diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
new file mode 100644
index 000..ef6a986
--- /dev/null
+++ b/tools/flask/policy/modules/dom0.te
@@ -0,0 +1,74 @@
+
+#
+# Allow dom0 access to all sysctls, devices, and the security server.
+#
+# While this could be written more briefly using wildcards, the permissions are
+# listed out to make removing specific permissions simpler.
+#
+
+allow dom0_t xen_t:xen {
+   settime tbufcontrol readconsole clearconsole perfcontrol mtrr_add
+   mtrr_del mtrr_read microcode physinfo quirk writeconsole readapic
+   writeapic privprofile nonprivprofile kexec firmware sleep frequency
+   getidle debug getcpuinfo heap pm_op mca_op lockprof cpupool_op tmem_op
+ 

[Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Daniel De Graaf]

This operation has no known users, and is primarily useful when an MLS
policy is in use (which has never been shipped with Xen).  In addition,
the information it provides does not actually depend on hypervisor
state (only on the XSM policy), so an application that needs it could
compute the results without needing to involve the hypervisor.

Signed-off-by: Daniel De Graaf 
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/include/public/xen-compat.h |   2 +-
 xen/include/public/xsm/flask_op.h   |   6 +-
 xen/include/xlat.lst|   1 -
 xen/xsm/flask/flask_op.c|  42 --
 xen/xsm/flask/include/security.h|   2 -
 xen/xsm/flask/policy/access_vectors |   2 -
 xen/xsm/flask/ss/mls.c  |  49 
 xen/xsm/flask/ss/mls.h  |   3 -
 xen/xsm/flask/ss/services.c | 111 
 10 files changed, 7 insertions(+), 213 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index d228b24..2d982d9 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -47,7 +47,7 @@ allow dom0_t dom0_t:resource { add remove };
 # that does not have its own security server to make access decisions based on
 # Xen's security policy.
 allow dom0_t security_t:security {
-   compute_av compute_create compute_member compute_relabel compute_user
+   compute_av compute_create compute_member compute_relabel
 };
 
 # Allow string/SID conversions (for "xl list -Z" and similar)
diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h
index 590de76..dd8a5c0 100644
--- a/xen/include/public/xen-compat.h
+++ b/xen/include/public/xen-compat.h
@@ -27,7 +27,7 @@
 #ifndef __XEN_PUBLIC_XEN_COMPAT_H__
 #define __XEN_PUBLIC_XEN_COMPAT_H__
 
-#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040700
+#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040800
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 /* Xen is built with matching headers and implements the latest interface. */
diff --git a/xen/include/public/xsm/flask_op.h 
b/xen/include/public/xsm/flask_op.h
index c76359c..970ec07 100644
--- a/xen/include/public/xsm/flask_op.h
+++ b/xen/include/public/xsm/flask_op.h
@@ -70,6 +70,7 @@ struct xen_flask_transition {
 uint32_t newsid;
 };
 
+#if __XEN_INTERFACE_VERSION__ < 0x00040800
 struct xen_flask_userlist {
 /* IN: starting SID for list */
 uint32_t start_sid;
@@ -83,6 +84,7 @@ struct xen_flask_userlist {
 XEN_GUEST_HANDLE(uint32) sids;
 } u;
 };
+#endif
 
 struct xen_flask_boolean {
 /* IN/OUT: numeric identifier for boolean [GET/SET]
@@ -167,7 +169,7 @@ struct xen_flask_op {
 #define FLASK_ACCESS6
 #define FLASK_CREATE7
 #define FLASK_RELABEL   8
-#define FLASK_USER  9
+#define FLASK_USER  9  /* No longer implemented */
 #define FLASK_POLICYVERS10
 #define FLASK_GETBOOL   11
 #define FLASK_SETBOOL   12
@@ -193,7 +195,9 @@ struct xen_flask_op {
 struct xen_flask_access access;
 /* FLASK_CREATE, FLASK_RELABEL, FLASK_MEMBER */
 struct xen_flask_transition transition;
+#if __XEN_INTERFACE_VERSION__ < 0x00040800
 struct xen_flask_userlist userlist;
+#endif
 /* FLASK_GETBOOL, FLASK_SETBOOL */
 struct xen_flask_boolean boolean;
 struct xen_flask_setavc_threshold setavc_threshold;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 23befb3..801a1c1 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -129,4 +129,3 @@
 ?  flask_setenforcexsm/flask_op.h
 !  flask_sid_context   xsm/flask_op.h
 ?  flask_transitionxsm/flask_op.h
-!  flask_userlist  xsm/flask_op.h
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index ea903a7..3ad4bdc 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -86,43 +86,6 @@ static int domain_has_security(struct domain *d, u32 perms)
 perms, NULL);
 }
 
-#endif /* COMPAT */
-
-static int flask_security_user(struct xen_flask_userlist *arg)
-{
-char *user;
-u32 *sids;
-u32 nsids;
-int rv;
-
-rv = domain_has_security(current->domain, SECURITY__COMPUTE_USER);
-if ( rv )
-return rv;
-
-user = safe_copy_string_from_guest(arg->u.user, arg->size, PAGE_SIZE);
-if ( IS_ERR(user) )
-return PTR_ERR(user);
-
-rv = security_get_user_sids(arg->start_sid, user, , );
-if ( rv < 0 )
-goto out;
-
-if ( nsids * sizeof(sids[0]) > arg->size )
-nsids = arg->size / sizeof(sids[0]);
-
-arg->size = nsids;
-
-if ( _copy_to_guest(arg->u.sids, sids, nsids) )
-rv = -EFAULT;
-
-xfree(sids);
- out:
-xfree(user);
-return rv;
-}
-
-#ifndef COMPAT
-
 static int flask_security_relabel(struct xen_flask_transition *arg)

[Xen-devel] [PATCH 03/17] flask/policy: move user definitions and constraints into modules

[Daniel De Graaf]

This also renames the example users created by vm_role.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Doug Goldstein 
---
 docs/misc/xsm-flask.txt| 34 +++---
 tools/flask/policy/Makefile|  9 --
 tools/flask/policy/modules/all_system_role.te  |  5 
 tools/flask/policy/modules/modules.conf| 11 +--
 .../{policy/constraints => modules/vm_role.cons}   |  6 ++--
 tools/flask/policy/modules/vm_role.te  | 16 ++
 tools/flask/policy/modules/xen.te  |  9 --
 tools/flask/policy/policy/support/misc_macros.spt  |  6 ++--
 tools/flask/policy/policy/users| 12 +---
 9 files changed, 63 insertions(+), 45 deletions(-)
 rename tools/flask/policy/{policy/constraints => modules/vm_role.cons} (78%)
 create mode 100644 tools/flask/policy/modules/vm_role.te

diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index d3015ca..2f42585 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -147,9 +147,11 @@ it relies on the SELinux compiler "checkpolicy"; run
make -C tools/flask/policy
 
 to compile the example policy included with Xen. The policy is generated from
-definition files under this directory. When creating or modifying security
-policy, most modifications will be made to the xen type enforcement (.te) file
-tools/flask/policy/policy/modules/xen/xen.te or the macro definitions in 
xen.if.
+definition files under this directory. Most changes to security policy will
+involve creating or modifying modules found in tools/flask/policy/modules/.  
The
+modules.conf file there defines what modules are enabled and has short
+descriptions of each module.
+
 The XSM policy file needs to be copied to /boot and loaded as a module by grub.
 The exact position of the module does not matter as long as it is after the Xen
 kernel; it is normally placed either just above the dom0 kernel or at the end.
@@ -210,17 +212,16 @@ Type transitions are also used to compute the labels of 
event channels.
 Users and roles
 ---
 
-Users are defined in tools/flask/policy/policy/users. The example policy 
defines
-two users (customer_1 and customer_2) in addition to the system user system_u.
-Users are visible in the labels of domains and associated objects (event
-channels); in the example policy, "customer_1:vm_r:domU_t" is a valid label for
-the customer_1 user.
+The default user and role used for domains is system_u and system_r.  Users are
+visible in the labels of domains and associated objects (event channels); when
+the vm_role module is enabled, "user_1:vm_r:domU_t" is a valid label for a
+domain created by the user_1 user.
 
-Access control rules involving users and roles are defined in the policy
-constraints file (tools/flask/policy/policy/constraints). The example policy
-provides constraints that prevent different users from communicating using
-grants or event channels, while still allowing communication with the system_u
-user where dom0 resides.
+Access control rules involving users and roles are defined in a module's
+constraints file (for example, vm_rule.cons). The vm_role module defines one
+role (vm_r) and three users (user_1 .. user_3), along with constraints that
+prevent different users from communicating using grants or event channels, 
while
+still allowing communication with the system_u user where dom0 resides.
 
 Resource Policy
 ---
@@ -268,10 +269,9 @@ declare_domain and create_domain interfaces:
 Existing SELinux tools such as audit2allow can be applied to these denials, 
e.g.
 xl dmesg | audit2allow
 
-The generated allow rules can then be fed back into the policy by
-adding them to xen.te, although manual review is advised and will
-often lead to adding parameterized rules to the interfaces in xen.if
-to address the general case.
+The generated allow rules can then be fed back into the policy by adding them 
to
+a module, although manual review is advised and will often lead to adding
+parameterized rules to the interfaces in xen.if to address the general case.
 
 
 Device Labeling in Policy
diff --git a/tools/flask/policy/Makefile b/tools/flask/policy/Makefile
index b2c2d06..693eb10 100644
--- a/tools/flask/policy/Makefile
+++ b/tools/flask/policy/Makefile
@@ -54,7 +54,6 @@ AVS += $(POLDIR)/access_vectors
 M4SUPPORT := $(wildcard $(POLDIR)/support/*.spt)
 MLSSUPPORT := $(POLDIR)/mls
 USERS := $(POLDIR)/users
-CONSTRAINTS := $(POLDIR)/constraints
 ISID_DEFS := $(POLDIR)/initial_sids
 DEV_OCONS := $(POLDIR)/device_contexts
 
@@ -90,8 +89,12 @@ MODENABLED := on
 # extract settings from modules.conf
 ENABLED_LIST := $(shell awk '/^[ \t]*[a-z]/{ if ($$3 == "$(MODENABLED)") print 
$$1 }' $(MOD_CONF) 2> /dev/null)
 
+# Modules must provide a .te file, although it could be empty
 ALL_MODULES := $(foreach mod,$(ENABLED_LIST),$(MODDIR)/$(mod).te)
+
+# Modules may also 

[Xen-devel] [PATCH 08/17] flask: remove unused secondary context in ocontext

[Daniel De Graaf]

This field was originally used in Linux for a default message code for
network interfaces.  It has never been used in Xen, so remove it.

Signed-off-by: Daniel De Graaf 
---
 xen/xsm/flask/ss/policydb.c | 21 
 xen/xsm/flask/ss/policydb.h |  4 ++--
 xen/xsm/flask/ss/services.c | 58 ++---
 3 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/xen/xsm/flask/ss/policydb.c b/xen/xsm/flask/ss/policydb.c
index eebfe9c..00b5390 100644
--- a/xen/xsm/flask/ss/policydb.c
+++ b/xen/xsm/flask/ss/policydb.c
@@ -638,8 +638,7 @@ static int (*destroy_f[SYM_NUM]) (void *key, void *datum, 
void *datap) =
 
 static void ocontext_destroy(struct ocontext *c, int i)
 {
-context_destroy(>context[0]);
-context_destroy(>context[1]);
+context_destroy(>context);
 if ( i == OCON_ISID || i == OCON_DTREE )
 xfree(c->u.name);
 xfree(c);
@@ -747,14 +746,14 @@ int policydb_load_isids(struct policydb *p, struct sidtab 
*s)
 head = p->ocontexts[OCON_ISID];
 for ( c = head; c; c = c->next )
 {
-if ( !c->context[0].user )
+if ( !c->context.user )
 {
 printk(KERN_ERR "Flask:  SID %s was never "
"defined.\n", c->u.name);
 rc = -EINVAL;
 goto out;
 }
-if ( sidtab_insert(s, c->sid[0], >context[0]) )
+if ( sidtab_insert(s, c->sid, >context) )
 {
 printk(KERN_ERR "Flask:  unable to load initial "
"SID %s.\n", c->u.name);
@@ -2015,8 +2014,8 @@ int policydb_read(struct policydb *p, void *fp)
 rc = next_entry(buf, fp, sizeof(u32));
 if ( rc < 0 )
 goto bad;
-c->sid[0] = le32_to_cpu(buf[0]);
-rc = context_read_and_validate(>context[0], p, fp);
+c->sid = le32_to_cpu(buf[0]);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
@@ -2031,7 +2030,7 @@ int policydb_read(struct policydb *p, void *fp)
 if ( rc < 0 )
 goto bad;
 c->u.pirq = le32_to_cpu(buf[0]);
-rc = context_read_and_validate(>context[0], p, fp);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
@@ -2047,7 +2046,7 @@ int policydb_read(struct policydb *p, void *fp)
 goto bad;
 c->u.ioport.low_ioport = le32_to_cpu(buf[0]);
 c->u.ioport.high_ioport = le32_to_cpu(buf[1]);
-rc = context_read_and_validate(>context[0], p, fp);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
@@ -2075,7 +2074,7 @@ int policydb_read(struct policydb *p, void *fp)
 c->u.iomem.low_iomem = le32_to_cpu(buf[0]);
 c->u.iomem.high_iomem = le32_to_cpu(buf[1]);
 }
-rc = context_read_and_validate(>context[0], p, fp);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
@@ -2090,7 +2089,7 @@ int policydb_read(struct policydb *p, void *fp)
 if ( rc < 0 )
 goto bad;
 c->u.device = le32_to_cpu(buf[0]);
-rc = context_read_and_validate(>context[0], p, fp);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
@@ -2113,7 +2112,7 @@ int policydb_read(struct policydb *p, void *fp)
 if ( rc < 0 )
 goto bad;
 c->u.name[len] = 0;
-rc = context_read_and_validate(>context[0], p, fp);
+rc = context_read_and_validate(>context, p, fp);
 if ( rc )
 goto bad;
 break;
diff --git a/xen/xsm/flask/ss/policydb.h b/xen/xsm/flask/ss/policydb.h
index 30be71a..f158ce3 100644
--- a/xen/xsm/flask/ss/policydb.h
+++ b/xen/xsm/flask/ss/policydb.h
@@ -158,8 +158,8 @@ struct ocontext {
 u64 high_iomem;
 } iomem;
 } u;
-struct context context[2];/* security context(s) */
-u32 sid[2];/* SID(s) */
+struct context context;
+u32 sid;
 struct ocontext *next;
 };
 
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index 9da358b..c590440 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1488,13 +1488,13 @@ int security_irq_sid(int pirq, u32 *out_sid)
 
 if ( c )
 {
-if ( !c->sid[0] )
+if ( !c->sid )
 {
-rc = sidtab_context_to_sid(, >context[0], >sid[0]);
+rc = 

[Xen-devel] [PATCH 09/17] flask: remove unused AVC callback functions

[Daniel De Graaf]

These callbacks are not used in Xen.

Signed-off-by: Daniel De Graaf 
---
 xen/xsm/flask/avc.c | 97 ++---
 xen/xsm/flask/include/avc.h | 13 --
 2 files changed, 4 insertions(+), 106 deletions(-)

diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 7764379..a3e6108 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -86,18 +86,6 @@ struct avc_cache {
 u32latest_notif;/* latest revocation notification */
 };
 
-struct avc_callback_node {
-int (*callback) (u32 event, u32 ssid, u32 tsid,
- u16 tclass, u32 perms,
- u32 *out_retained);
-u32 events;
-u32 ssid;
-u32 tsid;
-u16 tclass;
-u32 perms;
-struct avc_callback_node *next;
-};
-
 /* Exported via Flask hypercall */
 unsigned int avc_cache_threshold = AVC_DEF_CACHE_THRESHOLD;
 
@@ -106,7 +94,6 @@ DEFINE_PER_CPU(struct avc_cache_stats, avc_cache_stats);
 #endif
 
 static struct avc_cache avc_cache;
-static struct avc_callback_node *avc_callbacks;
 
 static DEFINE_RCU_READ_LOCK(avc_rcu_lock);
 
@@ -616,46 +603,6 @@ void avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 
requested,
 }
 
 /**
- * avc_add_callback - Register a callback for security events.
- * @callback: callback function
- * @events: security events
- * @ssid: source security identifier or %SECSID_WILD
- * @tsid: target security identifier or %SECSID_WILD
- * @tclass: target security class
- * @perms: permissions
- *
- * Register a callback function for events in the set @events
- * related to the SID pair (@ssid, @tsid) and
- * and the permissions @perms, interpreting
- * @perms based on @tclass.  Returns %0 on success or
- * -%ENOMEM if insufficient memory exists to add the callback.
- */
-int avc_add_callback(int (*callback)(u32 event, u32 ssid, u32 tsid, u16 tclass,
- u32 perms, u32 *out_retained), u32 
events, u32 ssid, u32 tsid,
- u16 tclass, u32 perms)
-{
-struct avc_callback_node *c;
-int rc = 0;
-
-c = xmalloc(struct avc_callback_node);
-if ( !c )
-{
-rc = -ENOMEM;
-goto out;
-}
-
-c->callback = callback;
-c->events = events;
-c->ssid = ssid;
-c->tsid = tsid;
-c->perms = perms;
-c->next = avc_callbacks;
-avc_callbacks = c;
- out:
-return rc;
-}
-
-/**
  * avc_update_node Update an AVC entry
  * @event : Updating event
  * @perms : Permission mask bits
@@ -666,7 +613,7 @@ int avc_add_callback(int (*callback)(u32 event, u32 ssid, 
u32 tsid, u16 tclass,
  * otherwise, this function update the AVC entry. The original AVC-entry object
  * will release later by RCU.
  */
-static int avc_update_node(u32 event, u32 perms, u32 ssid, u32 tsid, u16 
tclass,
+static int avc_update_node(u32 perms, u32 ssid, u32 tsid, u16 tclass,
u32 seqno)
 {
 int hvalue, rc = 0;
@@ -715,28 +662,7 @@ static int avc_update_node(u32 event, u32 perms, u32 ssid, 
u32 tsid, u16 tclass,
 
 avc_node_populate(node, ssid, tsid, tclass, >ae.avd);
 
-switch ( event )
-{
-case AVC_CALLBACK_GRANT:
-node->ae.avd.allowed |= perms;
-break;
-case AVC_CALLBACK_TRY_REVOKE:
-case AVC_CALLBACK_REVOKE:
-node->ae.avd.allowed &= ~perms;
-break;
-case AVC_CALLBACK_AUDITALLOW_ENABLE:
-node->ae.avd.auditallow |= perms;
-break;
-case AVC_CALLBACK_AUDITALLOW_DISABLE:
-node->ae.avd.auditallow &= ~perms;
-break;
-case AVC_CALLBACK_AUDITDENY_ENABLE:
-node->ae.avd.auditdeny |= perms;
-break;
-case AVC_CALLBACK_AUDITDENY_DISABLE:
-node->ae.avd.auditdeny &= ~perms;
-break;
-}
+node->ae.avd.allowed |= perms;
 avc_node_replace(node, orig);
  out_unlock:
 spin_unlock_irqrestore(lock, flag);
@@ -750,8 +676,7 @@ static int avc_update_node(u32 event, u32 perms, u32 ssid, 
u32 tsid, u16 tclass,
  */
 int avc_ss_reset(u32 seqno)
 {
-struct avc_callback_node *c;
-int i, rc = 0, tmprc;
+int i, rc = 0;
 unsigned long flag;
 struct avc_node *node;
 struct hlist_head *head;
@@ -771,19 +696,6 @@ int avc_ss_reset(u32 seqno)
 spin_unlock_irqrestore(lock, flag);
 }
 
-for ( c = avc_callbacks; c; c = c->next )
-{
-if ( c->events & AVC_CALLBACK_RESET )
-{
-tmprc = c->callback(AVC_CALLBACK_RESET,
-0, 0, 0, 0, NULL);
-/* save the first error encountered for the return
-   value and continue processing the callbacks */
-if ( !rc )
-rc = tmprc;
-}
-}
-
 avc_latest_notif_update(seqno, 0);
 return rc;
 }
@@ -845,8 +757,7 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, u16 tclass, 
u32 requested,
 if ( denied )
 {
 if ( !flask_enforcing || (avd->flags & AVD_FLAGS_PERMISSIVE) )
-

[Xen-devel] [PATCH 11/17] flask: improve unknown permission handling

[Daniel De Graaf]

When an unknown domctl, sysctl, or other operation is encountered in the
FLASK security server, use the allow_unknown bit in the security policy
to decide if the permission should be allowed or denied.  This allows
new operations to be tested without needing to immediately add security
checks; however, it is not flexible enough to avoid adding the actual
permission checks.  An error message is printed to the hypervisor
console when this fallback is encountered.

This patch will allow operations that are not handled by the existing
hooks only if the policy was compiled with "checkpolicy -U allow".  In
previous releases, this bit did nothing, and the default remains to deny
the unknown operations.

Signed-off-by: Daniel De Graaf 
---
 xen/xsm/flask/hooks.c| 45 ++--
 xen/xsm/flask/include/security.h |  2 ++
 xen/xsm/flask/ss/policydb.c  |  1 +
 xen/xsm/flask/ss/policydb.h  |  6 ++
 xen/xsm/flask/ss/services.c  |  5 +
 5 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index a8d45e7..543406b 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -136,6 +136,24 @@ static int get_irq_sid(int irq, u32 *sid, struct 
avc_audit_data *ad)
 return 0;
 }
 
+static int avc_unknown_permission(const char *name, int id)
+{
+int rc;
+
+if ( !flask_enforcing || security_get_allow_unknown() )
+{
+printk(XENLOG_G_WARNING "FLASK: Allowing unknown %s: %d.\n", name, id);
+rc = 0;
+}
+else
+{
+printk(XENLOG_G_ERR "FLASK: Denying unknown %s: %d.\n", name, id);
+rc = -EPERM;
+}
+
+return rc;
+}
+
 static int flask_domain_alloc_security(struct domain *d)
 {
 struct domain_security_struct *dsec;
@@ -271,7 +289,7 @@ static int flask_evtchn_send(struct domain *d, struct 
evtchn *chn)
 rc = 0;
 break;
 default:
-rc = -EPERM;
+rc = avc_unknown_permission("event channel state", chn->state);
 }
 
 return rc;
@@ -423,7 +441,7 @@ static int flask_console_io(struct domain *d, int cmd)
 perm = XEN__WRITECONSOLE;
 break;
 default:
-return -EPERM;
+return avc_unknown_permission("console_io", cmd);
 }
 
 return domain_has_xen(d, perm);
@@ -455,7 +473,7 @@ static int flask_profile(struct domain *d, int op)
 perm = XEN__PRIVPROFILE;
 break;
 default:
-return -EPERM;
+return avc_unknown_permission("xenoprof op", op);
 }
 
 return domain_has_xen(d, perm);
@@ -521,8 +539,7 @@ static int flask_domctl_scheduler_op(struct domain *d, int 
op)
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETSCHEDULER);
 
 default:
-printk("flask_domctl_scheduler_op: Unknown op %d\n", op);
-return -EPERM;
+return avc_unknown_permission("domctl_scheduler_op", op);
 }
 }
 
@@ -537,8 +554,7 @@ static int flask_sysctl_scheduler_op(int op)
 return domain_has_xen(current->domain, XEN__GETSCHEDULER);
 
 default:
-printk("flask_sysctl_scheduler_op: Unknown op %d\n", op);
-return -EPERM;
+return avc_unknown_permission("sysctl_scheduler_op", op);
 }
 }
 
@@ -735,8 +751,7 @@ static int flask_domctl(struct domain *d, int cmd)
 return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
 
 default:
-printk("flask_domctl: Unknown op %d\n", cmd);
-return -EPERM;
+return avc_unknown_permission("domctl", cmd);
 }
 }
 
@@ -811,8 +826,7 @@ static int flask_sysctl(int cmd)
 XEN2__LIVEPATCH_OP, NULL);
 
 default:
-printk("flask_sysctl: Unknown op %d\n", cmd);
-return -EPERM;
+return avc_unknown_permission("sysctl", cmd);
 }
 }
 
@@ -1129,7 +1143,7 @@ static inline int flask_page_offline(uint32_t cmd)
 case sysctl_query_page_offline:
 return flask_resource_use_core();
 default:
-return -EPERM;
+return avc_unknown_permission("page_offline", cmd);
 }
 }
 
@@ -1402,8 +1416,7 @@ static int flask_platform_op(uint32_t op)
 SECCLASS_XEN2, XEN2__GET_SYMBOL, NULL);
 
 default:
-printk("flask_platform_op: Unknown op %d\n", op);
-return -EPERM;
+return avc_unknown_permission("platform_op", op);
 }
 }
 
@@ -1434,7 +1447,7 @@ static int flask_shadow_control(struct domain *d, 
uint32_t op)
 perm = SHADOW__LOGDIRTY;
 break;
 default:
-return -EPERM;
+return avc_unknown_permission("shadow_control", op);
 }
 
 return current_has_perm(d, SECCLASS_SHADOW, perm);
@@ -1538,7 +1551,7 @@ static int flask_apic(struct domain *d, int cmd)
 perm = XEN__WRITEAPIC;
 break;
 default:
-return -EPERM;
+return avc_unknown_permission("apic", cmd);
 }
 
 return domain_has_xen(d, perm);
diff 

Re: [Xen-devel] [PATCH] x86: compact supposedly unused entry point code

[Andrew Cooper]

On 20/06/16 14:49, Jan Beulich wrote:
 On 20.06.16 at 14:54,  wrote:
>> On 20/06/16 13:48, Jan Beulich wrote:
>> On 20.06.16 at 14:15,  wrote:
 On 20/06/16 12:04, Jan Beulich wrote:
> No point in aligning entry points which aren't supposed to be used
> anyway.
>
> Signed-off-by: Jan Beulich 
 Reviewed-by: Andrew Cooper 
>>> Thanks, but - any thoughts on this part:
>>>
>>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>>> uninitialized error code (which shouldn't matter).
>> I was still considering that part.
>>
>> These are entries we never expect to actually take.  At that point, the
>> small overhead of setting up the error code to 0 is probably better than
>> leaving it uninitialised.
> I understand - it's really a matter of balancing the overhead on
> these paths (which will never have an effect if these entries indeed
> are unused, and which is of no interest if they are used by due some
> other flaw) with the (likely negligible, but non-zero) overhead they
> introduce on _other_ paths (due to cache and TLB consumption). I.e.
> my goal was to make these unused entries as small as possible. And
>
>   andq$-15,%rsp
>   movl$vector,4(%rsp)
>
> (obviously we can't use movb here) is smaller than the current
>
>   testb   $8,%spl
>   jz  1f
>   pushq   $0
>   movb$vector,4(%rsp)
>
> afaict.

All of them head to do_reserved_trap() and panic().

An alternative would be to drop all this entry code, mark the vectors as
not present in the IDT, and handle #NP[IDT vector] with a slightly
different error message in do_trap().

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Jan Beulich]

>>> On 20.06.16 at 15:36,  wrote:

> 
> On 06/20/2016 09:30 AM, Jan Beulich wrote:
> On 20.06.16 at 14:58,  wrote:
>>> On 06/20/2016 04:56 AM, Jan Beulich wrote:
>>> On 20.06.16 at 00:03,  wrote:
> Follow up on 
> http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
> Using http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments 
> as
> reference.
>
>   New value
>   |---|
>
> f1  f5
> |---| |-|
> f2  f4
>   |-|f3   |-|
>   |-|
>
> Given a new value of the type above, Current logic will not
> allow applying parts of the new value overlapping with f3 filter.
> only f2 and f4.
 I remains unclear what f1...f5 are meant to represent here.

> This change allows all 3 types of overlapes to be included.
> More specifically for passthrough an Industrial Ethernet Interface
> (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
> Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
> given a quirk to allow read/write for that field is already in place.
> Device driver logic is such that the entire confspace  is
> written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
> arriving together in one call to xen_pcibk_config_write.
 Tweaks to make individual devices work are dubious. Any
 explanation should outline why current behavior is _generally_
 wrong. In particular with the original issue being with the
 Latency Timer field, and with that field now being allowed to
 be written by its entry in header_common[], ...

> --- a/drivers/xen/xen-pciback/conf_space.c
> +++ b/drivers/xen/xen-pciback/conf_space.c
> @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
> offset, int size, u32 value)
>   field_start = OFFSET(cfg_entry);
>   field_end = OFFSET(cfg_entry) + field->size;
>
> - if ((req_start >= field_start && req_start < field_end)
> - || (req_end > field_start && req_end <= field_end)) {
> +  if (req_end >= field_start || field_end >= req_start) {
>   tmp_val = 0;
 ... any change to the logic here which results in writes to the field
 getting issued would seem wrong without even looking at the
 particular nature of the field.

 As to the actual change - the two _end variables hold values
 pointing _past_ the region of interest, so the use of >= seems
 wrong here (ought to be >). And in the end we're talking of a
 classical overlap check here, which indeed can be simplified (to
 just two comparisons), but such simplification mustn't result in a
 change of behavior. (Such a simplification would end up being

if (req_end > field_start && field_end > req_start) {

 afaict - note the || instead of && used in your change.)
>>> Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and
>>> adding a proper comment) solve this problem?
>> How would that work? We mean to not allow writes, or else we
>> could simply add a .u.b.write handler for PCI_LATENCY_TIMER.
> 
> I thought you suggested (in another thread) to make PCI_LATENCY_TIMER 
> writable, just like PCI_CACHE_LINE_SIZE?

Well, I did put this up for discussion (as much as I questioned
whether Cache Line Size should perhaps not be writable). And if
we wanted to make it writable, then we should do so the ordinary
way, not via some hacks. (And looking at it again I don't even
understand why Cache Line Size has an entry in header_common[]
- identical behavior would result with the entry dropped afaict.)

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/2] x86/HVM: latch linear->phys translation results

[Andrew Cooper]

On 20/06/16 14:12, Tim Deegan wrote:
> At 12:54 +0100 on 09 Jun (1465476894), Andrew Cooper wrote:
>> On 08/06/16 14:09, Jan Beulich wrote:
>>> ... to avoid re-doing the same translation later again (in a retry, for
>>> example). This doesn't help very often according to my testing, but
>>> it's pretty cheap to have, and will be of further use subsequently.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -678,6 +678,19 @@ static struct hvm_mmio_cache *hvmemul_fi
>>>  return cache;
>>>  }
>>>  
>>> +static void latch_linear_to_phys(struct hvm_vcpu_io *vio, unsigned long 
>>> gla,
>>> + unsigned long gpa, bool_t write)
>>> +{
>>> +if ( vio->mmio_access.gla_valid )
>>> +return;
>>> +
>>> +vio->mmio_gva = gla & PAGE_MASK;
>> This suggest that mmio_vga is mis-named.
>>
>> Looking at the uses, handle_mmio_with_translation() is used
>> inconsistently, with virtual addresses from the shadow code, but linear
>> addresses from nested hap code.
>>
>> Clearly one of these two users are buggy for guests running in a non
>> flat way, and it looks to be the shadow side which is buggy.
> Yes, the naming in the shadow code is incorrect.  Shadow code, along
> with a lot of Xen code, uses "virtual" to refer to what the manuals
> call linear addresses, i.e. the inputs to paging.  IIRC it was only
> with the introduction of HAP hardware interfaces that we started using
> the term "linear" widely in Xen code.
>
> I will ack a mechanical renaming if you like, though beware of public
> interfaces with the old name, and common code ("linear" being an x86
> term).

I will be doing some cleanup in due course, although I don't have enough
time to do this right now.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH XTF] tests: add fep test

[Andrew Cooper]

On 17/06/16 15:21, Wei Liu wrote:
> Signed-off-by: Wei Liu 

LGTM, although a couple of comments.

> ---
>  tests/fep/Makefile | 12 
>  tests/fep/main.c   | 31 +++

Please add this to the test index in docs/all-tests.dox

>  2 files changed, 43 insertions(+)
>  create mode 100644 tests/fep/Makefile
>  create mode 100644 tests/fep/main.c
>
> diff --git a/tests/fep/Makefile b/tests/fep/Makefile
> new file mode 100644
> index 000..8702123
> --- /dev/null
> +++ b/tests/fep/Makefile
> @@ -0,0 +1,12 @@
> +MAKEFLAGS += -r
> +ROOT := $(abspath $(CURDIR)/../..)
> +
> +include $(ROOT)/build/common.mk
> +
> +NAME  := fep
> +CATEGORY  := utility
> +TEST-ENVS := $(HVM_ENVIRONMENTS)

This really doesn't need to be all HVM environments.  FEP is a property
of the HVM container, not of the running mode of the domain.  hvm32
would be fine here, and the most simple option.

> +
> +obj-perenv += main.o
> +
> +include $(ROOT)/build/gen.mk
> diff --git a/tests/fep/main.c b/tests/fep/main.c
> new file mode 100644
> index 000..34a93c0
> --- /dev/null
> +++ b/tests/fep/main.c
> @@ -0,0 +1,31 @@
> +/**
> + * @file tests/fep/main.c
> + * @ref test-fep
> + *
> + * @page test-fep FEP
> + *
> + * Returns SUCCESS if FEP is available, FAILURE if not.

This is the content one will find from the test index, and as such,
should be the most complete.  At the very least, I would add a sentence
explaining what FEP is.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

>>> On 20.06.16 at 14:54,  wrote:
> On 20/06/16 13:48, Jan Beulich wrote:
> On 20.06.16 at 14:15,  wrote:
>>> On 20/06/16 12:04, Jan Beulich wrote:
 No point in aligning entry points which aren't supposed to be used
 anyway.

 Signed-off-by: Jan Beulich 
>>> Reviewed-by: Andrew Cooper 
>> Thanks, but - any thoughts on this part:
>>
>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>> uninitialized error code (which shouldn't matter).
> 
> I was still considering that part.
> 
> These are entries we never expect to actually take.  At that point, the
> small overhead of setting up the error code to 0 is probably better than
> leaving it uninitialised.

I understand - it's really a matter of balancing the overhead on
these paths (which will never have an effect if these entries indeed
are unused, and which is of no interest if they are used by due some
other flaw) with the (likely negligible, but non-zero) overhead they
introduce on _other_ paths (due to cache and TLB consumption). I.e.
my goal was to make these unused entries as small as possible. And

andq$-15,%rsp
movl$vector,4(%rsp)

(obviously we can't use movb here) is smaller than the current

testb   $8,%spl
jz  1f
pushq   $0
movb$vector,4(%rsp)

afaict.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 4/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one

[Julien Grall]

The x86 version of the function xenmem_add_to_physmap_one contains
variable name gpfn and gfn which make the code very confusing.
I have left unchanged for now.

Also, rename gpfn to gfn in the ARM version as the latter is the correct
acronym for a guest physical frame.

Finally, remove the trailing whitespace around the changes.

Signed-off-by: Julien Grall 

---
Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/arch/arm/mm.c| 10 +-
 xen/arch/x86/mm.c| 15 +++
 xen/common/memory.c  |  6 +++---
 xen/include/xen/mm.h |  2 +-
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5ab9b75..6882d54 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1046,7 +1046,7 @@ int xenmem_add_to_physmap_one(
 unsigned int space,
 union xen_add_to_physmap_batch_extra extra,
 unsigned long idx,
-xen_pfn_t gpfn)
+gfn_t gfn)
 {
 unsigned long mfn = 0;
 int rc;
@@ -1081,8 +1081,8 @@ int xenmem_add_to_physmap_one(
 else
 return -EINVAL;
 }
-
-d->arch.grant_table_gpfn[idx] = gpfn;
+
+d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
 
 t = p2m_ram_rw;
 
@@ -1145,7 +1145,7 @@ int xenmem_add_to_physmap_one(
 if ( extra.res0 )
 return -EOPNOTSUPP;
 
-rc = map_dev_mmio_region(d, gpfn, 1, idx);
+rc = map_dev_mmio_region(d, gfn_x(gfn), 1, idx);
 return rc;
 
 default:
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
 }
 
 /* Map at new location. */
-rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
+rc = guest_physmap_add_entry(d, gfn, _mfn(mfn), 0, t);
 
 /* If we fail to add the mapping, we need to drop the reference we
  * took earlier on foreign pages */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7fbc94e..dbcf6cb 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4775,7 +4775,7 @@ int xenmem_add_to_physmap_one(
 unsigned int space,
 union xen_add_to_physmap_batch_extra extra,
 unsigned long idx,
-xen_pfn_t gpfn)
+gfn_t gpfn)
 {
 struct page_info *page = NULL;
 unsigned long gfn = 0; /* gcc ... */
@@ -4834,7 +4834,7 @@ int xenmem_add_to_physmap_one(
 break;
 }
 case XENMAPSPACE_gmfn_foreign:
-return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
+return p2m_add_foreign(d, idx, gfn_x(gpfn), extra.foreign_domid);
 default:
 break;
 }
@@ -4849,19 +4849,18 @@ int xenmem_add_to_physmap_one(
 }
 
 /* Remove previously mapped page if it was present. */
-prev_mfn = mfn_x(get_gfn(d, gpfn, ));
+prev_mfn = mfn_x(get_gfn(d, gfn_x(gpfn), ));
 if ( mfn_valid(prev_mfn) )
 {
 if ( is_xen_heap_mfn(prev_mfn) )
 /* Xen heap frames are simply unhooked from this phys slot. */
-guest_physmap_remove_page(d, _gfn(gpfn), _mfn(prev_mfn),
-  PAGE_ORDER_4K);
+guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
 else
 /* Normal domain memory is freed, to avoid leaking memory. */
-guest_remove_page(d, gpfn);
+guest_remove_page(d, gfn_x(gpfn));
 }
 /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
-put_gfn(d, gpfn);
+put_gfn(d, gfn_x(gpfn));
 
 /* Unmap from old location, if any. */
 old_gpfn = get_gpfn_from_mfn(mfn);
@@ -4872,7 +4871,7 @@ int xenmem_add_to_physmap_one(
 guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
 
 /* Map at new location. */
-rc = guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), PAGE_ORDER_4K);
+rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
 
 /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
 if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
diff --git a/xen/common/memory.c b/xen/common/memory.c
index a8a75e0..812334b 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -649,7 +649,7 @@ static int xenmem_add_to_physmap(struct domain *d,
 
 if ( xatp->space != XENMAPSPACE_gmfn_range )
 return xenmem_add_to_physmap_one(d, xatp->space, extra,
- xatp->idx, xatp->gpfn);
+ xatp->idx, _gfn(xatp->gpfn));
 
 if ( xatp->size < start )
 return -EILSEQ;
@@ -666,7 +666,7 @@ static int xenmem_add_to_physmap(struct domain *d,
 while ( xatp->size > done )
 {
 rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
-   

Re: [Xen-devel] [libvirt] gnulib and 32-bit libvirt build, rpl_canonicalize_file_name

[Ian Jackson]

Ján Tomko writes ("Re: [libvirt] gnulib and 32-bit libvirt build, 
rpl_canonicalize_file_name"):
> This has been fixed in gnulib already:
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=246b3b2
> commit 246b3b28808ee5f4664be674dce573af9497fc7a
> Author: Eric Blake 
> CommitDate: 2016-05-27 14:04:35 -0600
> 
> canonicalize: Fix broken probe for realpath.

Great, thanks.

> Pulled into libvirt by:
> commit 0ebd0b19d35b93eeea9d49318d7a69e147da
> Author: Eric Blake 
> CommitDate: 2016-05-27 14:06:45 -0600
> 
> maint: update to latest gnulib
> 
> Fix a regression in checking for realpath (which caused link
> failures regarding duplicate rpl_canonicalize_file_name), and
> fix the mingw build regarding unsetenv.
> 
> * .gnulib: Update to latest.
> 
> Signed-off-by: Eric Blake 

Right.

> The osstest service seems to be building libvirt commit:
> commit 6ec319b84f67d72bf59fe7e0fd41d88ee9c393c7
> Author: John Ferlan 
> 
> logical: Clean up allocation when building regex on the fly
> 
> git describe: v1.3.1-92-g6ec319b contains: v1.3.2-rc1~262
> 
> which used gnulib from before the commit that broke it.

OK, great.  Sorry for the noise.

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Xen 4.7 is released

[Wei Liu]

Hi all

I'm pleased to announce that Xen 4.7 is released. I would like to
thank everyone who involved in the making of 4.7 release (either in
the form of patch, bug report or packaging effort). This release
wouldn't have happened without all these contributions.

You can check out 4.7 release from xen.git with the tag "RELEASE-4.7.0".
Tarball and its signature can be obtained from:

http://bits.xensource.com/oss-xen/release/4.7.0/xen-4.7.0.tar.gz

http://bits.xensource.com/oss-xen/release/4.7.0/xen-4.7.0.tar.gz.sig

Release notes can be found at:

http://wiki.xenproject.org/wiki/Xen_Project_4.7_Release_Notes

A summary for 4.7 release can be found at:

http://wiki.xenproject.org/wiki/Category:Xen_4.7

Note that this is a xen-devel only announcement. Announcement on
xen-announce, blog post and press release etc will come later. The
official download link is going to be different from the one provided
above.

Thanks
Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 5/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn

[Julien Grall]

The correct acronym for a guest physical frame is gfn. Also use
the typesafe gfn to ensure that a guest frame is effectively used.

Signed-off-by: Julien Grall 

---
Changes in v2:
- Remove extra pair of brackets.
---
 xen/arch/arm/domain.c | 4 ++--
 xen/arch/arm/mm.c | 2 +-
 xen/include/asm-arm/domain.h  | 2 +-
 xen/include/asm-arm/grant_table.h | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 1365b4a..008747c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -444,13 +444,13 @@ struct domain *alloc_domain_struct(void)
 return NULL;
 
 clear_page(d);
-d->arch.grant_table_gpfn = xzalloc_array(xen_pfn_t, max_grant_frames);
+d->arch.grant_table_gfn = xzalloc_array(gfn_t, max_grant_frames);
 return d;
 }
 
 void free_domain_struct(struct domain *d)
 {
-xfree(d->arch.grant_table_gpfn);
+xfree(d->arch.grant_table_gfn);
 free_xenheap_page(d);
 }
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6882d54..0e408f8 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1082,7 +1082,7 @@ int xenmem_add_to_physmap_one(
 return -EINVAL;
 }
 
-d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
+d->arch.grant_table_gfn[idx] = gfn;
 
 t = p2m_ram_rw;
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 370cdeb..979f7de 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -51,7 +51,7 @@ struct arch_domain
 uint64_t vttbr;
 
 struct hvm_domain hvm_domain;
-xen_pfn_t *grant_table_gpfn;
+gfn_t *grant_table_gfn;
 
 struct vmmio vmmio;
 
diff --git a/xen/include/asm-arm/grant_table.h 
b/xen/include/asm-arm/grant_table.h
index 5e076cc..eb02423 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -30,7 +30,7 @@ static inline int replace_grant_supported(void)
 
 #define gnttab_shared_gmfn(d, t, i)  \
 ( ((i >= nr_grant_frames(d->grant_table)) && \
- (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
+ (i < max_grant_frames)) ? 0 : gfn_x(d->arch.grant_table_gfn[i]))
 
 #define gnttab_need_iommu_mapping(d)\
 (is_domain_direct_mapped(d) && need_iommu(d))
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 6/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...

[Julien Grall]

to avoid mixing machine frame with guest frame.

Signed-off-by: Julien Grall 

---
Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/arch/arm/domain_build.c  |  4 ++--
 xen/arch/arm/gic-v2.c|  4 ++--
 xen/arch/arm/p2m.c   | 22 +++---
 xen/arch/arm/platforms/exynos5.c |  8 
 xen/arch/arm/platforms/omap5.c   | 16 
 xen/arch/arm/vgic-v2.c   |  4 ++--
 xen/arch/x86/mm/p2m.c| 18 ++
 xen/common/domctl.c  |  4 ++--
 xen/include/xen/p2m-common.h |  8 
 9 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9035486..49185f0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1036,9 +1036,9 @@ static int map_range_to_domain(const struct 
dt_device_node *dev,
 if ( need_mapping )
 {
 res = map_mmio_regions(d,
-   paddr_to_pfn(addr),
+   _gfn(paddr_to_pfn(addr)),
DIV_ROUND_UP(len, PAGE_SIZE),
-   paddr_to_pfn(addr));
+   _mfn(paddr_to_pfn(addr)));
 if ( res < 0 )
 {
 printk(XENLOG_ERR "Unable to map 0x%"PRIx64
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 4e2f4c7..3893ece 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -601,9 +601,9 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
d->domain_id, v2m_data->addr, v2m_data->size,
v2m_data->spi_start, v2m_data->nr_spis);
 
-ret = map_mmio_regions(d, paddr_to_pfn(v2m_data->addr),
+ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
 DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
-paddr_to_pfn(v2m_data->addr));
+_mfn(paddr_to_pfn(v2m_data->addr)));
 if ( ret )
 {
 printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index aa4e774..47cb383 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1245,27 +1245,27 @@ int unmap_regions_rw_cache(struct domain *d,
 }
 
 int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
  unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
 {
 return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
  MATTR_DEV, 0, p2m_mmio_direct,
  d->arch.p2m.default_access);
 }
 
 int unmap_mmio_regions(struct domain *d,
-   unsigned long start_gfn,
+   gfn_t start_gfn,
unsigned long nr,
-   unsigned long mfn)
+   mfn_t mfn)
 {
 return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
  MATTR_DEV, 0, p2m_invalid,
  d->arch.p2m.default_access);
 }
@@ -1280,7 +1280,7 @@ int map_dev_mmio_region(struct domain *d,
 if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) )
 return 0;
 
-res = map_mmio_regions(d, start_gfn, nr, mfn);
+res = map_mmio_regions(d, _gfn(start_gfn), nr, _mfn(mfn));
 if ( res < 0 )
 {
 printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index bf4964d..c43934f 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -83,12 +83,12 @@ static int exynos5_init_time(void)
 static int exynos5250_specific_mapping(struct domain *d)
 {
 /* Map the chip ID */
-map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_CHIPID), 1,
- paddr_to_pfn(EXYNOS5_PA_CHIPID));
+

[Xen-devel] [PATCH v2 2/8] xen/mm: Introduce a bunch of helpers for the typesafes mfn and gfn

[Julien Grall]

Those helpers will be useful to do common operations without having to
unbox/box manually the GFNs/MFNs.

Signed-off-by: Julien Grall 

---
Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 

Changes in v2:
- Rename min_gfn/max_gfn to gfn_min/gfn_max
- Add more helpers for gfn and mfn
---
 xen/include/xen/mm.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3cf646a..58b5c75 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -61,6 +61,11 @@ TYPE_SAFE(unsigned long, mfn);
 #undef mfn_t
 #endif
 
+#define mfn_add(mfn, i) _mfn(mfn_x(mfn) + (i))
+#define mfn_max(x, y)   _mfn(max(mfn_x(x), mfn_x(y)))
+#define mfn_min(x, y)   _mfn(min(mfn_x(x), mfn_x(y)))
+#define mfn_eq(x, y)(mfn_x(x) == mfn_x(y))
+
 TYPE_SAFE(unsigned long, gfn);
 #define PRI_gfn  "05lx"
 #define INVALID_GFN  (~0UL)
@@ -70,6 +75,11 @@ TYPE_SAFE(unsigned long, gfn);
 #undef gfn_t
 #endif
 
+#define gfn_add(gfn, i) _gfn(gfn_x(gfn) + (i))
+#define gfn_max(x, y)   _gfn(max(gfn_x(x), gfn_x(y)))
+#define gfn_min(x, y)   _gfn(min(gfn_x(x), gfn_x(y)))
+#define gfn_eq(x, y)(gfn_x(x) == gfn_x(y))
+
 TYPE_SAFE(unsigned long, pfn);
 #define PRI_pfn  "05lx"
 #define INVALID_PFN  (~0UL)
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 3/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers

[Julien Grall]

Also rename some variables to gfn or mfn when it does not require much
rework.

Signed-off-by: Julien Grall 

---
Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Paul Durrant 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 

Changes in v2:
- Don't use a wrapper for x86. Instead use mfn_* to make
the change simpler.
---
 xen/arch/arm/domain_build.c|  2 +-
 xen/arch/arm/mm.c  | 10 ++---
 xen/arch/arm/p2m.c | 20 +-
 xen/arch/x86/domain.c  |  5 ++-
 xen/arch/x86/domain_build.c|  6 +--
 xen/arch/x86/hvm/ioreq.c   |  8 ++--
 xen/arch/x86/mm.c  | 12 +++---
 xen/arch/x86/mm/p2m.c  | 78 --
 xen/common/grant_table.c   |  7 ++--
 xen/common/memory.c| 32 
 xen/drivers/passthrough/arm/smmu.c |  4 +-
 xen/include/asm-arm/p2m.h  | 12 +++---
 xen/include/asm-x86/p2m.h  | 11 +++---
 xen/include/xen/mm.h   |  2 +-
 14 files changed, 110 insertions(+), 99 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 410bb4f..9035486 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -117,7 +117,7 @@ static bool_t insert_11_bank(struct domain *d,
 goto fail;
 }
 
-res = guest_physmap_add_page(d, spfn, spfn, order);
+res = guest_physmap_add_page(d, _gfn(spfn), _mfn(spfn), order);
 if ( res )
 panic("Failed map pages to DOM0: %d", res);
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 2ec211b..5ab9b75 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
 }
 
 /* Map at new location. */
-rc = guest_physmap_add_entry(d, gpfn, mfn, 0, t);
+rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
 
 /* If we fail to add the mapping, we need to drop the reference we
  * took earlier on foreign pages */
@@ -1282,8 +1282,8 @@ int create_grant_host_mapping(unsigned long addr, 
unsigned long frame,
 if ( flags & GNTMAP_readonly )
 t = p2m_grant_map_ro;
 
-rc = guest_physmap_add_entry(current->domain, addr >> PAGE_SHIFT,
- frame, 0, t);
+rc = guest_physmap_add_entry(current->domain, _gfn(addr >> PAGE_SHIFT),
+ _mfn(frame), 0, t);
 
 if ( rc )
 return GNTST_general_error;
@@ -1294,13 +1294,13 @@ int create_grant_host_mapping(unsigned long addr, 
unsigned long frame,
 int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
 unsigned long new_addr, unsigned int flags)
 {
-unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
+gfn_t gfn = _gfn(addr >> PAGE_SHIFT);
 struct domain *d = current->domain;
 
 if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
 return GNTST_general_error;
 
-guest_physmap_remove_page(d, gfn, mfn, 0);
+guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
 
 return GNTST_okay;
 }
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ab0cb41..aa4e774 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1292,26 +1292,26 @@ int map_dev_mmio_region(struct domain *d,
 }
 
 int guest_physmap_add_entry(struct domain *d,
-unsigned long gpfn,
-unsigned long mfn,
+gfn_t gfn,
+mfn_t mfn,
 unsigned long page_order,
 p2m_type_t t)
 {
 return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1 << page_order)),
- pfn_to_paddr(mfn), MATTR_MEM, 0, t,
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + (1 << page_order)),
+ pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, t,
  d->arch.p2m.default_access);
 }
 
 void guest_physmap_remove_page(struct domain *d,
-   unsigned long gpfn,
-   unsigned long mfn, unsigned int page_order)
+   gfn_t gfn,
+   mfn_t mfn, unsigned int page_order)
 {
 apply_p2m_changes(d, REMOVE,
-  pfn_to_paddr(gpfn),
-  pfn_to_paddr(gpfn + (1<

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 20.06.16 at 14:06,  wrote:
> Suppose resolve_misconfig() is modified to change all p2m_ioreq_server 
> entries(which also
> have e.recalc flag turned on) back to p2m_ram_rw. And suppose we have 
> ioreq server 1, which
> emulates gfn A, and ioreq server 2 which emulates gfn B:
> 
> 1> At the beginning, ioreq server 1 is attached to p2m_ioreq_server, and 
> gfn A is write protected
> by setting it to p2m_ioreq_server;
> 
> 2> ioreq server 1 is detached from p2m_ioreq_server, left gfn A's p2m 
> type unchanged;
> 
> 3> After the detachment of ioreq server 1, 
> p2m_change_entry_type_global() is called, all ept
> entries are invalidated;
> 
> 4> Later, ioreq server 2 is attached to p2m_ioreq_server;
> 
> 5> Gfn B is set to p2m_ioreq_server, although its corresponding ept 
> entry was invalidated,
> ept_set_entry() will trigger resolve_misconfig(), which will set the p2m 
> type of gfn B back to
> p2m_ram_rw;
> 
> 6> ept_set_entry() will set gfn B's p2m type to p2m_ioreq_server next; 
> And now, we have two
> ept entries with p2m_ioreq_server type - gfn A's and gfn B's.
> 
> With no live migration, things could work fine - later accesses to gfn A 
> will ultimately change
> its type back to p2m_ram_rw.
> 
> However, if live migration is started(all pte entries invalidated 
> again), resolve_misconfig() would
> change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means 
> the emulation of
> gfn B would fail.

Why would it? Changes to p2m_ram_logdirty won't alter
p2m_ioreq_server entries, and hence changes from it back to
p2m_ram_rw won't either.

And then - didn't we mean to disable that part of XenGT during
migration, i.e. temporarily accept the higher performance
overhead without the p2m_ioreq_server entries? In which case
flipping everything back to p2m_ram_rw after (completed or
canceled) migration would be exactly what we want. The (new
or previous) ioreq server should attach only afterwards, and
can then freely re-establish any p2m_ioreq_server entries it
deems necessary.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 0/8] xen/arm: Use the typesafes gfn and mfn

[Julien Grall]

Hello all,

Some of the ARM functions are mixing gfn vs mfn and even physical vs frame.

To avoid more confusion, this patch series makes use of the terminology
described in xen/include/xen/mm.h and the associated typesafe.

This series is based on staging + the branch next-4.8 from Stefano merge.

I have pushed a branch with the prerequisites and this series on xenbits:
git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v1

Yours sincerely,

Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Paul Durrant 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 

Julien Grall (8):
  xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
  xen/mm: Introduce a bunch of helpers for the typesafes mfn and gfn
  xen: Use typesafe gfn/mfn in guest_physmap_* helpers
  xen: Use typesafe gfn in xenmem_add_to_physmap_one
  xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the
typesafe gfn
  xen: Use the typesafe mfn and gfn in map_mmio_regions...
  xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and
mfn
  xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

 xen/arch/arm/domain.c  |  4 +-
 xen/arch/arm/domain_build.c|  6 +--
 xen/arch/arm/domctl.c  |  2 +-
 xen/arch/arm/gic-v2.c  |  4 +-
 xen/arch/arm/mm.c  | 18 +++
 xen/arch/arm/p2m.c | 91 +++-
 xen/arch/arm/platforms/exynos5.c   |  8 ++--
 xen/arch/arm/platforms/omap5.c | 16 +++
 xen/arch/arm/traps.c   | 21 +
 xen/arch/arm/vgic-v2.c |  4 +-
 xen/arch/x86/domain.c  |  5 +-
 xen/arch/x86/domain_build.c|  6 +--
 xen/arch/x86/hvm/ioreq.c   |  8 ++--
 xen/arch/x86/mm.c  | 21 +
 xen/arch/x86/mm/p2m.c  | 96 +-
 xen/common/domctl.c|  4 +-
 xen/common/grant_table.c   | 11 +++--
 xen/common/memory.c| 40 
 xen/drivers/passthrough/arm/smmu.c |  4 +-
 xen/include/asm-arm/domain.h   |  2 +-
 xen/include/asm-arm/grant_table.h  |  2 +-
 xen/include/asm-arm/p2m.h  | 23 +
 xen/include/asm-x86/p2m.h  | 11 ++---
 xen/include/xen/mm.h   | 14 +-
 xen/include/xen/p2m-common.h   |  8 ++--
 25 files changed, 227 insertions(+), 202 deletions(-)

-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn

[Julien Grall]

p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
the variable to *gfn* and use typesafe to avoid possible misusage.

Signed-off-by: Julien Grall 

---
Changes in v2:
- Drop _gfn suffix
---
 xen/arch/arm/domctl.c |  2 +-
 xen/arch/arm/p2m.c| 10 +-
 xen/include/asm-arm/p2m.h |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 30453d8..b94e97c 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -30,7 +30,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain 
*d,
 if ( e < s )
 return -EINVAL;
 
-return p2m_cache_flush(d, s, e);
+return p2m_cache_flush(d, _gfn(s), _gfn(e));
 }
 case XEN_DOMCTL_bind_pt_irq:
 {
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f3330dd..9149981 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1469,16 +1469,16 @@ int relinquish_p2m_mapping(struct domain *d)
   d->arch.p2m.default_access);
 }
 
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
+int p2m_cache_flush(struct domain *d, gfn_t start, gfn_t end)
 {
 struct p2m_domain *p2m = >arch.p2m;
 
-start_mfn = MAX(start_mfn, p2m->lowest_mapped_gfn);
-end_mfn = MIN(end_mfn, p2m->max_mapped_gfn);
+start = gfn_max(start, _gfn(p2m->lowest_mapped_gfn));
+end = gfn_min(end, _gfn(p2m->max_mapped_gfn));
 
 return apply_p2m_changes(d, CACHEFLUSH,
- pfn_to_paddr(start_mfn),
- pfn_to_paddr(end_mfn),
+ pfn_to_paddr(gfn_x(start)),
+ pfn_to_paddr(gfn_x(end)),
  pfn_to_paddr(INVALID_MFN),
  MATTR_MEM, 0, p2m_invalid,
  d->arch.p2m.default_access);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index f204482..976e51e 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -139,7 +139,7 @@ void p2m_dump_info(struct domain *d);
 mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
 
 /* Clean & invalidate caches corresponding to a region of guest address space 
*/
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
+int p2m_cache_flush(struct domain *d, gfn_t start, gfn_t end);
 
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 7/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn

[Julien Grall]

The prototype and the declaration of p2m_lookup disagree on how the
function should be used. One expect a frame number whilst the other
an address.

Thankfully, everyone is using with an address today. However, most of
the callers have to convert a guest frame to an address. Modify
the interface to take a guest physical frame in parameter and return
a machine frame.

Whilst modifying the interface, use typesafe gfn and mfn for clarity
and catching possible misusage.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/p2m.c| 37 -
 xen/arch/arm/traps.c  | 21 +++--
 xen/include/asm-arm/p2m.h |  7 +++
 3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 47cb383..f3330dd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
 }
 
 /*
- * Lookup the MFN corresponding to a domain's PFN.
+ * Lookup the MFN corresponding to a domain's GFN.
  *
  * There are no processor functions to do a stage 2 only lookup therefore we
  * do a a software walk.
  */
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
 {
 struct p2m_domain *p2m = >arch.p2m;
+const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
 const unsigned int offsets[4] = {
 zeroeth_table_offset(paddr),
 first_table_offset(paddr),
@@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t 
paddr, p2m_type_t *t)
 ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
 };
 lpae_t pte, *map;
-paddr_t maddr = INVALID_PADDR;
+mfn_t mfn = _mfn(INVALID_MFN);
 paddr_t mask = 0;
 p2m_type_t _t;
 unsigned int level, root_table;
@@ -216,21 +217,22 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t 
paddr, p2m_type_t *t)
 {
 ASSERT(mask);
 ASSERT(pte.p2m.type != p2m_invalid);
-maddr = (pte.bits & PADDR_MASK & mask) | (paddr & ~mask);
+mfn = _mfn(paddr_to_pfn((pte.bits & PADDR_MASK & mask) |
+(paddr & ~mask)));
 *t = pte.p2m.type;
 }
 
 err:
-return maddr;
+return mfn;
 }
 
-paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
 {
-paddr_t ret;
+mfn_t ret;
 struct p2m_domain *p2m = >arch.p2m;
 
 spin_lock(>lock);
-ret = __p2m_lookup(d, paddr, t);
+ret = __p2m_lookup(d, gfn, t);
 spin_unlock(>lock);
 
 return ret;
@@ -493,8 +495,9 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
  * No setting was found in the Radix tree. Check if the
  * entry exists in the page-tables.
  */
-paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
-if ( INVALID_PADDR == maddr )
+mfn_t mfn = __p2m_lookup(d, gfn, NULL);
+
+if ( mfn_x(mfn) == INVALID_MFN )
 return -ESRCH;
 
 /* If entry exists then its rwx. */
@@ -1483,8 +1486,7 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t 
start_mfn, xen_pfn_t end_mfn)
 
 mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
 {
-paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
-return _mfn(p >> PAGE_SHIFT);
+return p2m_lookup(d, gfn, NULL);
 }
 
 /*
@@ -1498,7 +1500,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned 
long flag)
 {
 long rc;
 paddr_t ipa;
-unsigned long maddr;
+gfn_t gfn;
 unsigned long mfn;
 xenmem_access_t xma;
 p2m_type_t t;
@@ -1508,11 +1510,13 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned 
long flag)
 if ( rc < 0 )
 goto err;
 
+gfn = _gfn(paddr_to_pfn(ipa));
+
 /*
  * We do this first as this is faster in the default case when no
  * permission is set on the page.
  */
-rc = __p2m_get_mem_access(current->domain, _gfn(paddr_to_pfn(ipa)), );
+rc = __p2m_get_mem_access(current->domain, gfn, );
 if ( rc < 0 )
 goto err;
 
@@ -1561,11 +1565,10 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned 
long flag)
  * We had a mem_access permission limiting the access, but the page type
  * could also be limiting, so we need to check that as well.
  */
-maddr = __p2m_lookup(current->domain, ipa, );
-if ( maddr == INVALID_PADDR )
+mfn = mfn_x(__p2m_lookup(current->domain, gfn, ));
+if ( mfn == INVALID_MFN )
 goto err;
 
-mfn = maddr >> PAGE_SHIFT;
 if ( !mfn_valid(mfn) )
 goto err;
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa3e3c2..f1737e4 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2318,14 +2318,16 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 {
 register_t ttbcr = READ_SYSREG(TCR_EL1);
 uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1);
-paddr_t 

[Xen-devel] [PATCH v2 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe

[Julien Grall]

The correct acronym for a guest physical frame is gfn. Also use
the recently introduced typesafe gfn/mfn to avoid mixing the two
different kind of frame.

Signed-off-by: Julien Grall 
Acked-by: Jan Beulich 

---
Cc: Stefano Stabellini 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 

Changes in v2:
- Add Jan's acked-by
- CCed the relevant maintainers
---
 xen/arch/arm/p2m.c| 6 +++---
 xen/common/grant_table.c  | 4 ++--
 xen/common/memory.c   | 4 ++--
 xen/include/asm-arm/p2m.h | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 65d8f1a..ab0cb41 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1481,10 +1481,10 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t 
start_mfn, xen_pfn_t end_mfn)
  d->arch.p2m.default_access);
 }
 
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
 {
-paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
-return p >> PAGE_SHIFT;
+paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
+return _mfn(p >> PAGE_SHIFT);
 }
 
 /*
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 8b22299..3c304f4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -256,7 +256,7 @@ static int __get_paged_frame(unsigned long gfn, unsigned 
long *frame, struct pag
 }
 *frame = page_to_mfn(*page);
 #else
-*frame = gmfn_to_mfn(rd, gfn);
+*frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
 *page = mfn_valid(*frame) ? mfn_to_page(*frame) : NULL;
 if ( (!(*page)) || (!get_page(*page, rd)) )
 {
@@ -1788,7 +1788,7 @@ gnttab_transfer(
 mfn = INVALID_MFN;
 }
 #else
-mfn = gmfn_to_mfn(d, gop.mfn);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gop.mfn)));
 #endif
 
 /* Check the passed page frame for basic validity. */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 46b1d9f..b54b076 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -264,7 +264,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 return 1;
 }
 #else
-mfn = gmfn_to_mfn(d, gmfn);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn)));
 #endif
 if ( unlikely(!mfn_valid(mfn)) )
 {
@@ -488,7 +488,7 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 goto fail; 
 }
 #else /* !CONFIG_X86 */
-mfn = gmfn_to_mfn(d, gmfn + k);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn + k)));
 #endif
 if ( unlikely(!mfn_valid(mfn)) )
 {
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d240d1e..75c65a8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -178,7 +178,7 @@ void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order);
 
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
 
 /*
  * Populate-on-demand
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Boris Ostrovsky]

On 06/20/2016 09:30 AM, Jan Beulich wrote:

On 20.06.16 at 14:58,  wrote:

On 06/20/2016 04:56 AM, Jan Beulich wrote:

On 20.06.16 at 00:03,  wrote:

Follow up on http://www.gossamer-threads.com/lists/xen/devel/436000#436000
Using http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
reference.

New value
|---|

f1f5
|---|   |-|
f2f4
  |-|f3   |-|
|-|

Given a new value of the type above, Current logic will not
allow applying parts of the new value overlapping with f3 filter.
only f2 and f4.

I remains unclear what f1...f5 are meant to represent here.


This change allows all 3 types of overlapes to be included.
More specifically for passthrough an Industrial Ethernet Interface
(Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
given a quirk to allow read/write for that field is already in place.
Device driver logic is such that the entire confspace  is
written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
arriving together in one call to xen_pcibk_config_write.

Tweaks to make individual devices work are dubious. Any
explanation should outline why current behavior is _generally_
wrong. In particular with the original issue being with the
Latency Timer field, and with that field now being allowed to
be written by its entry in header_common[], ...


--- a/drivers/xen/xen-pciback/conf_space.c
+++ b/drivers/xen/xen-pciback/conf_space.c
@@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
offset, int size, u32 value)
field_start = OFFSET(cfg_entry);
field_end = OFFSET(cfg_entry) + field->size;
   
-		if ((req_start >= field_start && req_start < field_end)

-   || (req_end > field_start && req_end <= field_end)) {
+if (req_end >= field_start || field_end >= req_start) {
tmp_val = 0;

... any change to the logic here which results in writes to the field
getting issued would seem wrong without even looking at the
particular nature of the field.

As to the actual change - the two _end variables hold values
pointing _past_ the region of interest, so the use of >= seems
wrong here (ought to be >). And in the end we're talking of a
classical overlap check here, which indeed can be simplified (to
just two comparisons), but such simplification mustn't result in a
change of behavior. (Such a simplification would end up being

if (req_end > field_start && field_end > req_start) {

afaict - note the || instead of && used in your change.)

Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and
adding a proper comment) solve this problem?

How would that work? We mean to not allow writes, or else we
could simply add a .u.b.write handler for PCI_LATENCY_TIMER.


I thought you suggested (in another thread) to make PCI_LATENCY_TIMER 
writable, just like PCI_CACHE_LINE_SIZE?


-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: fix an error path that uses uninitialised rc in libxl_set_memory_target

[Ian Jackson]

Wei Liu writes ("[PATCH] libxl: fix an error path that uses uninitialised rc in 
libxl_set_memory_target"):
> ecdc6fd8 ("libxl: Fix libxl_set_memory_target return value") failed to
> initialised rc in one failure path. Fix it in this path.
> 
> Also fixed an indentation issue while I was there.

Acked-by: Ian Jackson 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Jan Beulich]

>>> On 20.06.16 at 14:58,  wrote:

> 
> On 06/20/2016 04:56 AM, Jan Beulich wrote:
> On 20.06.16 at 00:03,  wrote:
>>> Follow up on http://www.gossamer-threads.com/lists/xen/devel/436000#436000 
>>> Using http://eli.thegreenplace.net/2008/08/15/intersection-of-1d-segments as
>>> reference.
>>>
>>> New value
>>> |---|
>>>
>>> f1f5
>>> |---|   |-|
>>>f2 f4
>>>  |-|f3   |-|
>>> |-|
>>>
>>> Given a new value of the type above, Current logic will not
>>> allow applying parts of the new value overlapping with f3 filter.
>>> only f2 and f4.
>> I remains unclear what f1...f5 are meant to represent here.
>>
>>> This change allows all 3 types of overlapes to be included.
>>> More specifically for passthrough an Industrial Ethernet Interface
>>> (Hilscher GmbH CIFX 50E-DP(M/S)) on a HVM DomU running the
>>> Xen 4.6 Hypervisor it allows to restore the LATENCY TIMER field
>>> given a quirk to allow read/write for that field is already in place.
>>> Device driver logic is such that the entire confspace  is
>>> written in 4 byte chunks s.t. LATENCY_TIMER AND CACHE_LINE_SIZE are
>>> arriving together in one call to xen_pcibk_config_write.
>> Tweaks to make individual devices work are dubious. Any
>> explanation should outline why current behavior is _generally_
>> wrong. In particular with the original issue being with the
>> Latency Timer field, and with that field now being allowed to
>> be written by its entry in header_common[], ...
>>
>>> --- a/drivers/xen/xen-pciback/conf_space.c
>>> +++ b/drivers/xen/xen-pciback/conf_space.c
>>> @@ -230,8 +230,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int
>>> offset, int size, u32 value)
>>> field_start = OFFSET(cfg_entry);
>>> field_end = OFFSET(cfg_entry) + field->size;
>>>   
>>> -   if ((req_start >= field_start && req_start < field_end)
>>> -   || (req_end > field_start && req_end <= field_end)) {
>>> +if (req_end >= field_start || field_end >= req_start) {
>>> tmp_val = 0;
>> ... any change to the logic here which results in writes to the field
>> getting issued would seem wrong without even looking at the
>> particular nature of the field.
>>
>> As to the actual change - the two _end variables hold values
>> pointing _past_ the region of interest, so the use of >= seems
>> wrong here (ought to be >). And in the end we're talking of a
>> classical overlap check here, which indeed can be simplified (to
>> just two comparisons), but such simplification mustn't result in a
>> change of behavior. (Such a simplification would end up being
>>
>>  if (req_end > field_start && field_end > req_start) {
>>
>> afaict - note the || instead of && used in your change.)
> 
> Will setting header_common[]'s PCI_CACHE_LINE_SIZE size field to 2 (and 
> adding a proper comment) solve this problem?

How would that work? We mean to not allow writes, or else we
could simply add a .u.b.write handler for PCI_LATENCY_TIMER.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel