[ovmf test] 167363: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z2 days
Failing since167240  2021-12-09 08:42:46 Z1 days   45 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days8 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[ovmf test] 167360: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   44 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days7 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[linux-linus test] 167351: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-shadow 22 guest-start/debian.repeat fail pass in 
167359-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167317
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167317
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167317
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167317
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167317
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167317
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167317
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167317
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxb8a98b6bf66ae35361e987333233d07241642909
baseline version:
 linuxc741e49150dbb0c0aebe234389f4aa8b47958fa8

Last test of basis   167317  2021-12-10 02:28:41 Z1 days
Testing same since   167351  2021-12-10 20:11:11 Z0 days1 attempts


People who touched revisions under test:
  Alan Young 
  Alex Deucher 
  Arnd Bergmann 
  Bas Nieuwenhuizen 
  Bjorn Helgaas 
  Charles Keepax 
  Christian König 
  Damien Le Moal 
  Dan 

[ovmf test] 167358: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   43 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days6 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[ovmf test] 167356: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   42 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days5 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[xen-unstable test] 167348: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10   fail  like 167336
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167336
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167336
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167336
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167336
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 167336
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 167336
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167336
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167336
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167336
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167336
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167336
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167336
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  

[ovmf test] 167355: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   41 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days4 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[ovmf test] 167354: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   40 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days3 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

Re: [PATCH] xen/gntdev: fix unmap notification order

[Boris Ostrovsky]

On 12/10/21 4:28 AM, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

While working with Xen's libxenvchan library I have faced an issue with
unmap notifications sent in wrong order if both UNMAP_NOTIFY_SEND_EVENT
and UNMAP_NOTIFY_CLEAR_BYTE were requested: first we send an event channel
notification and then clear the notification byte which renders in the below
inconsistency (cli_live is the byte which was requested to be cleared on unmap):

[  444.514243] gntdev_put_map UNMAP_NOTIFY_SEND_EVENT map->notify.event 6
libxenvchan_is_open cli_live 1
[  444.515239] __unmap_grant_pages UNMAP_NOTIFY_CLEAR_BYTE at 14

Thus it is not possible to reliably implement the checks like
- wait for the notification (UNMAP_NOTIFY_SEND_EVENT)
- check the variable (UNMAP_NOTIFY_CLEAR_BYTE)
because it is possible that the variable gets checked before it is cleared
by the kernel.

To fix that we need to re-order the notifications, so the variable is first
gets cleared and then the event channel notification is sent.
With this fix I can see the correct order of execution:

[   54.522611] __unmap_grant_pages UNMAP_NOTIFY_CLEAR_BYTE at 14
[   54.537966] gntdev_put_map UNMAP_NOTIFY_SEND_EVENT map->notify.event 6
libxenvchan_is_open cli_live 0

Cc: sta...@vger.kernel.org
Signed-off-by: Oleksandr Andrushchenko 




Reviewed-by: Boris Ostrovsky 

[ovmf test] 167353: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   39 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days2 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

[patch V3 19/35] genirq/msi: Consolidate MSI descriptor data

[Thomas Gleixner]

From: Thomas Gleixner 

All non PCI/MSI usage variants have data structures in struct msi_desc with
only one member: xxx_index. PCI/MSI has a entry_nr member.

Add a common msi_index member to struct msi_desc so all implementations can
share it which allows further consolidation.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 include/linux/msi.h |2 ++
 1 file changed, 2 insertions(+)

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -143,6 +143,7 @@ struct ti_sci_inta_msi_desc {
  * address or data changes
  * @write_msi_msg_data:Data parameter for the callback.
  *
+ * @msi_index: Index of the msi descriptor
  * @pci:   [PCI]   PCI speficic msi descriptor data
  * @platform:  [platform]  Platform device specific msi descriptor data
  * @fsl_mc:[fsl-mc]FSL MC device specific msi descriptor data
@@ -163,6 +164,7 @@ struct msi_desc {
void (*write_msi_msg)(struct msi_desc *entry, void *data);
void *write_msi_msg_data;
 
+   u16 msi_index;
union {
struct pci_msi_desc pci;
struct platform_msi_descplatform;

[patch V3 18/35] platform-msi: Store platform private data pointer in msi_device_data

[Thomas Gleixner]

From: Thomas Gleixner 

Storing the platform private data in a MSI descriptor is sloppy at
best. The data belongs to the device and not to the descriptor.
Add a pointer to struct msi_device_data and store the pointer there.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/base/platform-msi.c |   79 +---
 include/linux/msi.h |4 +-
 2 files changed, 34 insertions(+), 49 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -38,9 +38,7 @@ static DEFINE_IDA(platform_msi_devid_ida
  */
 static irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
 {
-   u32 devid;
-
-   devid = desc->platform.msi_priv_data->devid;
+   u32 devid = desc->dev->msi.data->platform_data->devid;
 
return (devid << (32 - DEV_ID_SHIFT)) | desc->platform.msi_index;
 }
@@ -85,11 +83,8 @@ static void platform_msi_update_dom_ops(
 static void platform_msi_write_msg(struct irq_data *data, struct msi_msg *msg)
 {
struct msi_desc *desc = irq_data_get_msi_desc(data);
-   struct platform_msi_priv_data *priv_data;
-
-   priv_data = desc->platform.msi_priv_data;
 
-   priv_data->write_msg(desc, msg);
+   desc->dev->msi.data->platform_data->write_msg(desc, msg);
 }
 
 static void platform_msi_update_chip_ops(struct msi_domain_info *info)
@@ -126,9 +121,7 @@ static void platform_msi_free_descs(stru
 }
 
 static int platform_msi_alloc_descs_with_irq(struct device *dev, int virq,
-int nvec,
-struct platform_msi_priv_data 
*data)
-
+int nvec)
 {
struct msi_desc *desc;
int i, base = 0;
@@ -144,7 +137,6 @@ static int platform_msi_alloc_descs_with
if (!desc)
break;
 
-   desc->platform.msi_priv_data = data;
desc->platform.msi_index = base + i;
desc->irq = virq ? virq + i : 0;
 
@@ -161,11 +153,9 @@ static int platform_msi_alloc_descs_with
return 0;
 }
 
-static int platform_msi_alloc_descs(struct device *dev, int nvec,
-   struct platform_msi_priv_data *data)
-
+static int platform_msi_alloc_descs(struct device *dev, int nvec)
 {
-   return platform_msi_alloc_descs_with_irq(dev, 0, nvec, data);
+   return platform_msi_alloc_descs_with_irq(dev, 0, nvec);
 }
 
 /**
@@ -199,9 +189,8 @@ struct irq_domain *platform_msi_create_i
return domain;
 }
 
-static struct platform_msi_priv_data *
-platform_msi_alloc_priv_data(struct device *dev, unsigned int nvec,
-irq_write_msi_msg_t write_msi_msg)
+static int platform_msi_alloc_priv_data(struct device *dev, unsigned int nvec,
+   irq_write_msi_msg_t write_msi_msg)
 {
struct platform_msi_priv_data *datap;
int err;
@@ -213,41 +202,44 @@ platform_msi_alloc_priv_data(struct devi
 * capable devices).
 */
if (!dev->msi.domain || !write_msi_msg || !nvec || nvec > MAX_DEV_MSIS)
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
 
if (dev->msi.domain->bus_token != DOMAIN_BUS_PLATFORM_MSI) {
dev_err(dev, "Incompatible msi_domain, giving up\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}
 
err = msi_setup_device_data(dev);
if (err)
-   return ERR_PTR(err);
+   return err;
 
-   /* Already had a helping of MSI? Greed... */
-   if (!list_empty(dev_to_msi_list(dev)))
-   return ERR_PTR(-EBUSY);
+   /* Already initialized? */
+   if (dev->msi.data->platform_data)
+   return -EBUSY;
 
datap = kzalloc(sizeof(*datap), GFP_KERNEL);
if (!datap)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;
 
datap->devid = ida_simple_get(_msi_devid_ida,
  0, 1 << DEV_ID_SHIFT, GFP_KERNEL);
if (datap->devid < 0) {
err = datap->devid;
kfree(datap);
-   return ERR_PTR(err);
+   return err;
}
 
datap->write_msg = write_msi_msg;
datap->dev = dev;
-
-   return datap;
+   dev->msi.data->platform_data = datap;
+   return 0;
 }
 
-static void platform_msi_free_priv_data(struct platform_msi_priv_data *data)
+static void platform_msi_free_priv_data(struct device *dev)
 {
+   struct platform_msi_priv_data *data = dev->msi.data->platform_data;
+
+   dev->msi.data->platform_data = NULL;
ida_simple_remove(_msi_devid_ida, data->devid);
kfree(data);
 }
@@ -264,14 +256,13 @@ static void platform_msi_free_priv_data(
 int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
   

[patch V3 31/35] iommu/arm-smmu-v3: Use msi_get_virq()

[Thomas Gleixner]

From: Thomas Gleixner 

Let the core code fiddle with the MSI descriptor retrieval.

Signed-off-by: Thomas Gleixner 
Tested-by: Robin Murphy 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Will Deacon 
Cc: Joerg Roedel 
Cc: linux-arm-ker...@lists.infradead.org
Cc: io...@lists.linux-foundation.org

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3154,7 +3154,6 @@ static void arm_smmu_write_msi_msg(struc
 
 static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
 {
-   struct msi_desc *desc;
int ret, nvec = ARM_SMMU_MAX_MSIS;
struct device *dev = smmu->dev;
 
@@ -3182,21 +3181,9 @@ static void arm_smmu_setup_msis(struct a
return;
}
 
-   for_each_msi_entry(desc, dev) {
-   switch (desc->msi_index) {
-   case EVTQ_MSI_INDEX:
-   smmu->evtq.q.irq = desc->irq;
-   break;
-   case GERROR_MSI_INDEX:
-   smmu->gerr_irq = desc->irq;
-   break;
-   case PRIQ_MSI_INDEX:
-   smmu->priq.q.irq = desc->irq;
-   break;
-   default:/* Unknown */
-   continue;
-   }
-   }
+   smmu->evtq.q.irq = msi_get_virq(dev, EVTQ_MSI_INDEX);
+   smmu->gerr_irq = msi_get_virq(dev, GERROR_MSI_INDEX);
+   smmu->priq.q.irq = msi_get_virq(dev, PRIQ_MSI_INDEX);
 
/* Add callback to free MSIs on teardown */
devm_add_action(dev, arm_smmu_free_msis, dev);

[patch V3 34/35] soc: ti: ti_sci_inta_msi: Get rid of ti_sci_inta_msi_get_virq()

[Thomas Gleixner]

From: Thomas Gleixner 

Just use the core function msi_get_virq().

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Peter Ujfalusi 
Cc: Vinod Koul 
Cc: dmaeng...@vger.kernel.org
---
 drivers/dma/ti/k3-udma-private.c   |6 ++
 drivers/dma/ti/k3-udma.c   |   10 --
 drivers/soc/ti/k3-ringacc.c|2 +-
 drivers/soc/ti/ti_sci_inta_msi.c   |   12 
 include/linux/soc/ti/ti_sci_inta_msi.h |1 -
 5 files changed, 7 insertions(+), 24 deletions(-)

--- a/drivers/dma/ti/k3-udma-private.c
+++ b/drivers/dma/ti/k3-udma-private.c
@@ -168,8 +168,7 @@ int xudma_pktdma_tflow_get_irq(struct ud
 {
const struct udma_oes_offsets *oes = >soc_data->oes;
 
-   return ti_sci_inta_msi_get_virq(ud->dev, udma_tflow_id +
-   oes->pktdma_tchan_flow);
+   return msi_get_virq(ud->dev, udma_tflow_id + oes->pktdma_tchan_flow);
 }
 EXPORT_SYMBOL(xudma_pktdma_tflow_get_irq);
 
@@ -177,7 +176,6 @@ int xudma_pktdma_rflow_get_irq(struct ud
 {
const struct udma_oes_offsets *oes = >soc_data->oes;
 
-   return ti_sci_inta_msi_get_virq(ud->dev, udma_rflow_id +
-   oes->pktdma_rchan_flow);
+   return msi_get_virq(ud->dev, udma_rflow_id + oes->pktdma_rchan_flow);
 }
 EXPORT_SYMBOL(xudma_pktdma_rflow_get_irq);
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -2313,8 +2313,7 @@ static int udma_alloc_chan_resources(str
 
/* Event from UDMA (TR events) only needed for slave TR mode channels */
if (is_slave_direction(uc->config.dir) && !uc->config.pkt_mode) {
-   uc->irq_num_udma = ti_sci_inta_msi_get_virq(ud->dev,
-   irq_udma_idx);
+   uc->irq_num_udma = msi_get_virq(ud->dev, irq_udma_idx);
if (uc->irq_num_udma <= 0) {
dev_err(ud->dev, "Failed to get udma irq (index: %u)\n",
irq_udma_idx);
@@ -2486,7 +2485,7 @@ static int bcdma_alloc_chan_resources(st
uc->psil_paired = true;
}
 
-   uc->irq_num_ring = ti_sci_inta_msi_get_virq(ud->dev, irq_ring_idx);
+   uc->irq_num_ring = msi_get_virq(ud->dev, irq_ring_idx);
if (uc->irq_num_ring <= 0) {
dev_err(ud->dev, "Failed to get ring irq (index: %u)\n",
irq_ring_idx);
@@ -2503,8 +2502,7 @@ static int bcdma_alloc_chan_resources(st
 
/* Event from BCDMA (TR events) only needed for slave channels */
if (is_slave_direction(uc->config.dir)) {
-   uc->irq_num_udma = ti_sci_inta_msi_get_virq(ud->dev,
-   irq_udma_idx);
+   uc->irq_num_udma = msi_get_virq(ud->dev, irq_udma_idx);
if (uc->irq_num_udma <= 0) {
dev_err(ud->dev, "Failed to get bcdma irq (index: 
%u)\n",
irq_udma_idx);
@@ -2672,7 +2670,7 @@ static int pktdma_alloc_chan_resources(s
 
uc->psil_paired = true;
 
-   uc->irq_num_ring = ti_sci_inta_msi_get_virq(ud->dev, irq_ring_idx);
+   uc->irq_num_ring = msi_get_virq(ud->dev, irq_ring_idx);
if (uc->irq_num_ring <= 0) {
dev_err(ud->dev, "Failed to get ring irq (index: %u)\n",
irq_ring_idx);
--- a/drivers/soc/ti/k3-ringacc.c
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -647,7 +647,7 @@ int k3_ringacc_get_ring_irq_num(struct k
if (!ring)
return -EINVAL;
 
-   irq_num = ti_sci_inta_msi_get_virq(ring->parent->dev, ring->ring_id);
+   irq_num = msi_get_virq(ring->parent->dev, ring->ring_id);
if (irq_num <= 0)
irq_num = -EINVAL;
return irq_num;
--- a/drivers/soc/ti/ti_sci_inta_msi.c
+++ b/drivers/soc/ti/ti_sci_inta_msi.c
@@ -148,15 +148,3 @@ void ti_sci_inta_msi_domain_free_irqs(st
ti_sci_inta_msi_free_descs(dev);
 }
 EXPORT_SYMBOL_GPL(ti_sci_inta_msi_domain_free_irqs);
-
-unsigned int ti_sci_inta_msi_get_virq(struct device *dev, u32 dev_index)
-{
-   struct msi_desc *desc;
-
-   for_each_msi_entry(desc, dev)
-   if (desc->msi_index == dev_index)
-   return desc->irq;
-
-   return -ENODEV;
-}
-EXPORT_SYMBOL_GPL(ti_sci_inta_msi_get_virq);
--- a/include/linux/soc/ti/ti_sci_inta_msi.h
+++ b/include/linux/soc/ti/ti_sci_inta_msi.h
@@ -18,6 +18,5 @@ struct irq_domain
   struct irq_domain *parent);
 int ti_sci_inta_msi_domain_alloc_irqs(struct device *dev,
  struct ti_sci_resource *res);
-unsigned int ti_sci_inta_msi_get_virq(struct device *dev, u32 index);
 void ti_sci_inta_msi_domain_free_irqs(struct device *dev);
 #endif /* __INCLUDE_LINUX_IRQCHIP_TI_SCI_INTA_H */

[patch V3 24/35] PCI/MSI: Provide MSI_FLAG_MSIX_CONTIGUOUS

[Thomas Gleixner]

From: Thomas Gleixner 

Provide a domain info flag which makes the core code check for a contiguous
MSI-X index on allocation. That's simpler than checking it at some other
domain callback in architecture code.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Acked-by: Bjorn Helgaas 

---
 drivers/pci/msi/irqdomain.c |   16 ++--
 include/linux/msi.h |2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -89,9 +89,21 @@ static int pci_msi_domain_check_cap(stru
if (pci_msi_desc_is_multi_msi(desc) &&
!(info->flags & MSI_FLAG_MULTI_PCI_MSI))
return 1;
-   else if (desc->pci.msi_attrib.is_msix && !(info->flags & 
MSI_FLAG_PCI_MSIX))
-   return -ENOTSUPP;
 
+   if (desc->pci.msi_attrib.is_msix) {
+   if (!(info->flags & MSI_FLAG_PCI_MSIX))
+   return -ENOTSUPP;
+
+   if (info->flags & MSI_FLAG_MSIX_CONTIGUOUS) {
+   unsigned int idx = 0;
+
+   /* Check for gaps in the entry indices */
+   for_each_msi_entry(desc, dev) {
+   if (desc->msi_index != idx++)
+   return -ENOTSUPP;
+   }
+   }
+   }
return 0;
 }
 
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -361,6 +361,8 @@ enum {
MSI_FLAG_LEVEL_CAPABLE  = (1 << 6),
/* Populate sysfs on alloc() and destroy it on free() */
MSI_FLAG_DEV_SYSFS  = (1 << 7),
+   /* MSI-X entries must be contiguous */
+   MSI_FLAG_MSIX_CONTIGUOUS= (1 << 8),
 };
 
 int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,

[patch V3 20/35] platform-msi: Use msi_desc::msi_index

[Thomas Gleixner]

From: Thomas Gleixner 

Use the common msi_index member and get rid of the pointless wrapper struct.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/base/platform-msi.c |   10 +-
 drivers/dma/qcom/hidma.c|4 ++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |4 ++--
 drivers/mailbox/bcm-flexrm-mailbox.c|4 ++--
 include/linux/msi.h |   10 --
 5 files changed, 11 insertions(+), 21 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -40,7 +40,7 @@ static irq_hw_number_t platform_msi_calc
 {
u32 devid = desc->dev->msi.data->platform_data->devid;
 
-   return (devid << (32 - DEV_ID_SHIFT)) | desc->platform.msi_index;
+   return (devid << (32 - DEV_ID_SHIFT)) | desc->msi_index;
 }
 
 static void platform_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
@@ -112,8 +112,8 @@ static void platform_msi_free_descs(stru
struct msi_desc *desc, *tmp;
 
list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
-   if (desc->platform.msi_index >= base &&
-   desc->platform.msi_index < (base + nvec)) {
+   if (desc->msi_index >= base &&
+   desc->msi_index < (base + nvec)) {
list_del(>list);
free_msi_entry(desc);
}
@@ -129,7 +129,7 @@ static int platform_msi_alloc_descs_with
if (!list_empty(dev_to_msi_list(dev))) {
desc = list_last_entry(dev_to_msi_list(dev),
   struct msi_desc, list);
-   base = desc->platform.msi_index + 1;
+   base = desc->msi_index + 1;
}
 
for (i = 0; i < nvec; i++) {
@@ -137,7 +137,7 @@ static int platform_msi_alloc_descs_with
if (!desc)
break;
 
-   desc->platform.msi_index = base + i;
+   desc->msi_index = base + i;
desc->irq = virq ? virq + i : 0;
 
list_add_tail(>list, dev_to_msi_list(dev));
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -666,7 +666,7 @@ static void hidma_write_msi_msg(struct m
struct device *dev = msi_desc_to_dev(desc);
struct hidma_dev *dmadev = dev_get_drvdata(dev);
 
-   if (!desc->platform.msi_index) {
+   if (!desc->msi_index) {
writel(msg->address_lo, dmadev->dev_evca + 0x118);
writel(msg->address_hi, dmadev->dev_evca + 0x11C);
writel(msg->data, dmadev->dev_evca + 0x120);
@@ -702,7 +702,7 @@ static int hidma_request_msi(struct hidm
return rc;
 
for_each_msi_entry(desc, >dev) {
-   if (!desc->platform.msi_index)
+   if (!desc->msi_index)
dmadev->msi_virqbase = desc->irq;
 
rc = devm_request_irq(>dev, desc->irq,
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3142,7 +3142,7 @@ static void arm_smmu_write_msi_msg(struc
phys_addr_t doorbell;
struct device *dev = msi_desc_to_dev(desc);
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
-   phys_addr_t *cfg = arm_smmu_msi_cfg[desc->platform.msi_index];
+   phys_addr_t *cfg = arm_smmu_msi_cfg[desc->msi_index];
 
doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
doorbell &= MSI_CFG0_ADDR_MASK;
@@ -3183,7 +3183,7 @@ static void arm_smmu_setup_msis(struct a
}
 
for_each_msi_entry(desc, dev) {
-   switch (desc->platform.msi_index) {
+   switch (desc->msi_index) {
case EVTQ_MSI_INDEX:
smmu->evtq.q.irq = desc->irq;
break;
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1484,7 +1484,7 @@ static void flexrm_mbox_msi_write(struct
 {
struct device *dev = msi_desc_to_dev(desc);
struct flexrm_mbox *mbox = dev_get_drvdata(dev);
-   struct flexrm_ring *ring = >rings[desc->platform.msi_index];
+   struct flexrm_ring *ring = >rings[desc->msi_index];
 
/* Configure per-Ring MSI registers */
writel_relaxed(msg->address_lo, ring->regs + RING_MSI_ADDR_LS);
@@ -1609,7 +1609,7 @@ static int flexrm_mbox_probe(struct plat
 
/* Save alloced IRQ numbers for each ring */
for_each_msi_entry(desc, dev) {
-   ring = >rings[desc->platform.msi_index];
+   ring = >rings[desc->msi_index];
ring->irq = desc->irq;
}
 
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -107,14 +107,6 @@ struct pci_msi_desc {
 };
 
 /**
- * platform_msi_desc - Platform device specific msi descriptor data
- * @msi_index: The index of the MSI descriptor for multi MSI
- */
-struct platform_msi_desc {

[patch V3 25/35] powerpc/pseries/msi: Let core code check for contiguous entries

[Thomas Gleixner]

From: Thomas Gleixner 

Set the domain info flag and remove the check.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: "Cédric Le Goater" 
Cc: linuxppc-...@lists.ozlabs.org

---
V2: Remove it completely - Cedric
---
 arch/powerpc/platforms/pseries/msi.c |   33 -
 1 file changed, 8 insertions(+), 25 deletions(-)

--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -321,27 +321,6 @@ static int msi_quota_for_device(struct p
return request;
 }
 
-static int check_msix_entries(struct pci_dev *pdev)
-{
-   struct msi_desc *entry;
-   int expected;
-
-   /* There's no way for us to express to firmware that we want
-* a discontiguous, or non-zero based, range of MSI-X entries.
-* So we must reject such requests. */
-
-   expected = 0;
-   for_each_pci_msi_entry(entry, pdev) {
-   if (entry->msi_index != expected) {
-   pr_debug("rtas_msi: bad MSI-X entries.\n");
-   return -EINVAL;
-   }
-   expected++;
-   }
-
-   return 0;
-}
-
 static void rtas_hack_32bit_msi_gen2(struct pci_dev *pdev)
 {
u32 addr_hi, addr_lo;
@@ -380,9 +359,6 @@ static int rtas_prepare_msi_irqs(struct
if (quota && quota < nvec)
return quota;
 
-   if (type == PCI_CAP_ID_MSIX && check_msix_entries(pdev))
-   return -EINVAL;
-
/*
 * Firmware currently refuse any non power of two allocation
 * so we round up if the quota will allow it.
@@ -529,9 +505,16 @@ static struct irq_chip pseries_pci_msi_i
.irq_write_msi_msg  = pseries_msi_write_msg,
 };
 
+
+/*
+ * Set MSI_FLAG_MSIX_CONTIGUOUS as there is no way to express to
+ * firmware to request a discontiguous or non-zero based range of
+ * MSI-X entries. Core code will reject such setup attempts.
+ */
 static struct msi_domain_info pseries_msi_domain_info = {
.flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_MULTI_PCI_MSI  | MSI_FLAG_PCI_MSIX),
+ MSI_FLAG_MULTI_PCI_MSI  | MSI_FLAG_PCI_MSIX |
+ MSI_FLAG_MSIX_CONTIGUOUS),
.ops   = _pci_msi_domain_ops,
.chip  = _pci_msi_irq_chip,
 };

[patch V3 21/35] bus: fsl-mc-msi: Use msi_desc::msi_index

[Thomas Gleixner]

From: Thomas Gleixner 

Use the common msi_index member and get rid of the pointless wrapper struct.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Stuart Yoder 
Cc: Laurentiu Tudor 
---
 drivers/bus/fsl-mc/fsl-mc-allocator.c |2 +-
 drivers/bus/fsl-mc/fsl-mc-msi.c   |6 +++---
 include/linux/msi.h   |   10 --
 3 files changed, 4 insertions(+), 14 deletions(-)

--- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
+++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
@@ -393,7 +393,7 @@ int fsl_mc_populate_irq_pool(struct fsl_
}
 
for_each_msi_entry(msi_desc, _bus_dev->dev) {
-   mc_dev_irq = _resources[msi_desc->fsl_mc.msi_index];
+   mc_dev_irq = _resources[msi_desc->msi_index];
mc_dev_irq->msi_desc = msi_desc;
mc_dev_irq->resource.id = msi_desc->irq;
}
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -29,7 +29,7 @@ static irq_hw_number_t fsl_mc_domain_cal
 * Make the base hwirq value for ICID*1 so it is readable
 * as a decimal value in /proc/interrupts.
 */
-   return (irq_hw_number_t)(desc->fsl_mc.msi_index + (dev->icid * 1));
+   return (irq_hw_number_t)(desc->msi_index + (dev->icid * 1));
 }
 
 static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg,
@@ -122,7 +122,7 @@ static void fsl_mc_msi_write_msg(struct
struct fsl_mc_device *mc_bus_dev = to_fsl_mc_device(msi_desc->dev);
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
struct fsl_mc_device_irq *mc_dev_irq =
-   _bus->irq_resources[msi_desc->fsl_mc.msi_index];
+   _bus->irq_resources[msi_desc->msi_index];
 
msi_desc->msg = *msg;
 
@@ -235,7 +235,7 @@ static int fsl_mc_msi_alloc_descs(struct
goto cleanup_msi_descs;
}
 
-   msi_desc->fsl_mc.msi_index = i;
+   msi_desc->msi_index = i;
INIT_LIST_HEAD(_desc->list);
list_add_tail(_desc->list, dev_to_msi_list(dev));
}
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -107,14 +107,6 @@ struct pci_msi_desc {
 };
 
 /**
- * fsl_mc_msi_desc - FSL-MC device specific msi descriptor data
- * @msi_index: The index of the MSI descriptor
- */
-struct fsl_mc_msi_desc {
-   u16 msi_index;
-};
-
-/**
  * ti_sci_inta_msi_desc - TISCI based INTA specific msi descriptor data
  * @dev_index: TISCI device index
  */
@@ -137,7 +129,6 @@ struct ti_sci_inta_msi_desc {
  *
  * @msi_index: Index of the msi descriptor
  * @pci:   [PCI]   PCI speficic msi descriptor data
- * @fsl_mc:[fsl-mc]FSL MC device specific msi descriptor data
  * @inta:  [INTA]  TISCI based INTA specific msi descriptor data
  */
 struct msi_desc {
@@ -158,7 +149,6 @@ struct msi_desc {
u16 msi_index;
union {
struct pci_msi_desc pci;
-   struct fsl_mc_msi_desc  fsl_mc;
struct ti_sci_inta_msi_desc inta;
};
 };

[patch V3 17/35] platform-msi: Rename functions and clarify comments

[Thomas Gleixner]

From: Thomas Gleixner 

It's hard to distinguish what platform_msi_domain_alloc() and
platform_msi_domain_alloc_irqs() are about. Make the distinction more
explicit and add comments which explain the use cases properly.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/base/platform-msi.c |   36 +---
 drivers/irqchip/irq-mbigen.c|4 ++--
 drivers/irqchip/irq-mvebu-icu.c |6 +++---
 include/linux/msi.h |8 
 4 files changed, 30 insertions(+), 24 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -313,17 +313,18 @@ EXPORT_SYMBOL_GPL(platform_msi_domain_fr
  *  a platform-msi domain
  * @domain:The platform-msi domain
  *
- * Returns the private data provided when calling
- * platform_msi_create_device_domain.
+ * Return: The private data provided when calling
+ * platform_msi_create_device_domain().
  */
 void *platform_msi_get_host_data(struct irq_domain *domain)
 {
struct platform_msi_priv_data *data = domain->host_data;
+
return data->host_data;
 }
 
 /**
- * __platform_msi_create_device_domain - Create a platform-msi domain
+ * __platform_msi_create_device_domain - Create a platform-msi device domain
  *
  * @dev:   The device generating the MSIs
  * @nvec:  The number of MSIs that need to be allocated
@@ -332,7 +333,11 @@ void *platform_msi_get_host_data(struct
  * @ops:   The hierarchy domain operations to use
  * @host_data: Private data associated to this domain
  *
- * Returns an irqdomain for @nvec interrupts
+ * Return: An irqdomain for @nvec interrupts on success, NULL in case of error.
+ *
+ * This is for interrupt domains which stack on a platform-msi domain
+ * created by platform_msi_create_irq_domain(). @dev->msi.domain points to
+ * that platform-msi domain which is the parent for the new domain.
  */
 struct irq_domain *
 __platform_msi_create_device_domain(struct device *dev,
@@ -372,18 +377,19 @@ struct irq_domain *
 }
 
 /**
- * platform_msi_domain_free - Free interrupts associated with a platform-msi
- *domain
+ * platform_msi_device_domain_free - Free interrupts associated with a 
platform-msi
+ *  device domain
  *
- * @domain:The platform-msi domain
+ * @domain:The platform-msi device domain
  * @virq:  The base irq from which to perform the free operation
  * @nvec:  How many interrupts to free from @virq
  */
-void platform_msi_domain_free(struct irq_domain *domain, unsigned int virq,
- unsigned int nvec)
+void platform_msi_device_domain_free(struct irq_domain *domain, unsigned int 
virq,
+unsigned int nvec)
 {
struct platform_msi_priv_data *data = domain->host_data;
struct msi_desc *desc, *tmp;
+
for_each_msi_entry_safe(desc, tmp, data->dev) {
if (WARN_ON(!desc->irq || desc->nvec_used != 1))
return;
@@ -397,10 +403,10 @@ void platform_msi_domain_free(struct irq
 }
 
 /**
- * platform_msi_domain_alloc - Allocate interrupts associated with
- *a platform-msi domain
+ * platform_msi_device_domain_alloc - Allocate interrupts associated with
+ *   a platform-msi device domain
  *
- * @domain:The platform-msi domain
+ * @domain:The platform-msi device domain
  * @virq:  The base irq from which to perform the allocate operation
  * @nr_irqs:   How many interrupts to free from @virq
  *
@@ -408,8 +414,8 @@ void platform_msi_domain_free(struct irq
  * with irq_domain_mutex held (which can only be done as part of a
  * top-level interrupt allocation).
  */
-int platform_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
- unsigned int nr_irqs)
+int platform_msi_device_domain_alloc(struct irq_domain *domain, unsigned int 
virq,
+unsigned int nr_irqs)
 {
struct platform_msi_priv_data *data = domain->host_data;
int err;
@@ -421,7 +427,7 @@ int platform_msi_domain_alloc(struct irq
err = msi_domain_populate_irqs(domain->parent, data->dev,
   virq, nr_irqs, >arg);
if (err)
-   platform_msi_domain_free(domain, virq, nr_irqs);
+   platform_msi_device_domain_free(domain, virq, nr_irqs);
 
return err;
 }
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -207,7 +207,7 @@ static int mbigen_irq_domain_alloc(struc
if (err)
return err;
 
-   err = platform_msi_domain_alloc(domain, virq, nr_irqs);
+   err = platform_msi_device_domain_alloc(domain, virq, nr_irqs);
if (err)
return err;
 
@@ -223,7 +223,7 @@ static int 

[patch V3 27/35] PCI/MSI: Use __msi_get_virq() in pci_get_vector()

[Thomas Gleixner]

From: Thomas Gleixner 

Use msi_get_vector() and handle the return value to be compatible.

No functional change intended.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
---
V2: Handle the INTx case directly instead of trying to be overly smart - Marc
---
 drivers/pci/msi/msi.c |   25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -1037,28 +1037,13 @@ EXPORT_SYMBOL(pci_free_irq_vectors);
  */
 int pci_irq_vector(struct pci_dev *dev, unsigned int nr)
 {
-   if (dev->msix_enabled) {
-   struct msi_desc *entry;
+   unsigned int irq;
 
-   for_each_pci_msi_entry(entry, dev) {
-   if (entry->msi_index == nr)
-   return entry->irq;
-   }
-   WARN_ON_ONCE(1);
-   return -EINVAL;
-   }
+   if (!dev->msi_enabled && !dev->msix_enabled)
+   return !nr ? dev->irq : -EINVAL;
 
-   if (dev->msi_enabled) {
-   struct msi_desc *entry = first_pci_msi_entry(dev);
-
-   if (WARN_ON_ONCE(nr >= entry->nvec_used))
-   return -EINVAL;
-   } else {
-   if (WARN_ON_ONCE(nr > 0))
-   return -EINVAL;
-   }
-
-   return dev->irq + nr;
+   irq = msi_get_virq(>dev, nr);
+   return irq ? irq : -EINVAL;
 }
 EXPORT_SYMBOL(pci_irq_vector);
 

[patch V3 14/35] PCI/MSI: Let the irq code handle sysfs groups

[Thomas Gleixner]

From: Thomas Gleixner 

Set the domain info flag which makes the core code handle sysfs groups and
put an explicit invocation into the legacy code.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Acked-by: Bjorn Helgaas 
---
 drivers/pci/msi/irqdomain.c |2 +-
 drivers/pci/msi/legacy.c|6 +-
 drivers/pci/msi/msi.c   |   23 ---
 include/linux/pci.h |1 -
 4 files changed, 6 insertions(+), 26 deletions(-)

--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -159,7 +159,7 @@ struct irq_domain *pci_msi_create_irq_do
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
pci_msi_domain_update_chip_ops(info);
 
-   info->flags |= MSI_FLAG_ACTIVATE_EARLY;
+   info->flags |= MSI_FLAG_ACTIVATE_EARLY | MSI_FLAG_DEV_SYSFS;
if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
info->flags |= MSI_FLAG_MUST_REACTIVATE;
 
--- a/drivers/pci/msi/legacy.c
+++ b/drivers/pci/msi/legacy.c
@@ -70,10 +70,14 @@ int pci_msi_legacy_setup_msi_irqs(struct
 {
int ret = arch_setup_msi_irqs(dev, nvec, type);
 
-   return pci_msi_setup_check_result(dev, type, ret);
+   ret = pci_msi_setup_check_result(dev, type, ret);
+   if (!ret)
+   ret = msi_device_populate_sysfs(>dev);
+   return ret;
 }
 
 void pci_msi_legacy_teardown_msi_irqs(struct pci_dev *dev)
 {
+   msi_device_destroy_sysfs(>dev);
arch_teardown_msi_irqs(dev);
 }
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -233,11 +233,6 @@ static void free_msi_irqs(struct pci_dev
for (i = 0; i < entry->nvec_used; i++)
BUG_ON(irq_has_action(entry->irq + i));
 
-   if (dev->msi_irq_groups) {
-   msi_destroy_sysfs(>dev, dev->msi_irq_groups);
-   dev->msi_irq_groups = NULL;
-   }
-
pci_msi_teardown_msi_irqs(dev);
 
list_for_each_entry_safe(entry, tmp, msi_list, list) {
@@ -417,7 +412,6 @@ static int msi_verify_entries(struct pci
 static int msi_capability_init(struct pci_dev *dev, int nvec,
   struct irq_affinity *affd)
 {
-   const struct attribute_group **groups;
struct msi_desc *entry;
int ret;
 
@@ -448,14 +442,6 @@ static int msi_capability_init(struct pc
if (ret)
goto err;
 
-   groups = msi_populate_sysfs(>dev);
-   if (IS_ERR(groups)) {
-   ret = PTR_ERR(groups);
-   goto err;
-   }
-
-   dev->msi_irq_groups = groups;
-
/* Set MSI enabled bits */
pci_intx_for_msi(dev, 0);
pci_msi_set_enable(dev, 1);
@@ -584,7 +570,6 @@ static void msix_mask_all(void __iomem *
 static int msix_capability_init(struct pci_dev *dev, struct msix_entry 
*entries,
int nvec, struct irq_affinity *affd)
 {
-   const struct attribute_group **groups;
void __iomem *base;
int ret, tsize;
u16 control;
@@ -629,14 +614,6 @@ static int msix_capability_init(struct p
 
msix_update_entries(dev, entries);
 
-   groups = msi_populate_sysfs(>dev);
-   if (IS_ERR(groups)) {
-   ret = PTR_ERR(groups);
-   goto out_free;
-   }
-
-   dev->msi_irq_groups = groups;
-
/* Disable INTX and unmask MSI-X */
pci_intx_for_msi(dev, 0);
pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -475,7 +475,6 @@ struct pci_dev {
 #ifdef CONFIG_PCI_MSI
void __iomem*msix_base;
raw_spinlock_t  msi_lock;
-   const struct attribute_group **msi_irq_groups;
 #endif
struct pci_vpd  vpd;
 #ifdef CONFIG_PCIE_DPC

[patch V3 30/35] perf/smmuv3: Use msi_get_virq()

[Thomas Gleixner]

From: Thomas Gleixner 

Let the core code fiddle with the MSI descriptor retrieval.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Mark Rutland 
Cc: Will Deacon 
Cc: linux-arm-ker...@lists.infradead.org
---
 drivers/perf/arm_smmuv3_pmu.c |5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -684,7 +684,6 @@ static void smmu_pmu_write_msi_msg(struc
 
 static void smmu_pmu_setup_msi(struct smmu_pmu *pmu)
 {
-   struct msi_desc *desc;
struct device *dev = pmu->dev;
int ret;
 
@@ -701,9 +700,7 @@ static void smmu_pmu_setup_msi(struct sm
return;
}
 
-   desc = first_msi_entry(dev);
-   if (desc)
-   pmu->irq = desc->irq;
+   pmu->irq = msi_get_virq(dev, 0);
 
/* Add callback to free MSIs on teardown */
devm_add_action(dev, smmu_pmu_free_msis, dev);

[patch V3 23/35] PCI/MSI: Use msi_desc::msi_index

[Thomas Gleixner]

From: Thomas Gleixner 

The usage of msi_desc::pci::entry_nr is confusing at best. It's the index
into the MSI[X] descriptor table.

Use msi_desc::msi_index which is shared between all MSI incarnations
instead of having a PCI specific storage for no value.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Acked-by: Bjorn Helgaas 

---
 arch/powerpc/platforms/pseries/msi.c |4 ++--
 arch/x86/pci/xen.c   |2 +-
 drivers/pci/msi/irqdomain.c  |2 +-
 drivers/pci/msi/msi.c|   20 
 drivers/pci/xen-pcifront.c   |2 +-
 include/linux/msi.h  |2 --
 6 files changed, 13 insertions(+), 19 deletions(-)

--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -332,7 +332,7 @@ static int check_msix_entries(struct pci
 
expected = 0;
for_each_pci_msi_entry(entry, pdev) {
-   if (entry->pci.msi_attrib.entry_nr != expected) {
+   if (entry->msi_index != expected) {
pr_debug("rtas_msi: bad MSI-X entries.\n");
return -EINVAL;
}
@@ -579,7 +579,7 @@ static int pseries_irq_domain_alloc(stru
int hwirq;
int i, ret;
 
-   hwirq = rtas_query_irq_number(pci_get_pdn(pdev), 
desc->pci.msi_attrib.entry_nr);
+   hwirq = rtas_query_irq_number(pci_get_pdn(pdev), desc->msi_index);
if (hwirq < 0) {
dev_err(>dev, "Failed to query HW IRQ: %d\n", hwirq);
return hwirq;
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -306,7 +306,7 @@ static int xen_initdom_setup_msi_irqs(st
return -EINVAL;
 
map_irq.table_base = pci_resource_start(dev, bir);
-   map_irq.entry_nr = msidesc->pci.msi_attrib.entry_nr;
+   map_irq.entry_nr = msidesc->msi_index;
}
 
ret = -EINVAL;
--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -57,7 +57,7 @@ static irq_hw_number_t pci_msi_domain_ca
 {
struct pci_dev *dev = msi_desc_to_pci_dev(desc);
 
-   return (irq_hw_number_t)desc->pci.msi_attrib.entry_nr |
+   return (irq_hw_number_t)desc->msi_index |
pci_dev_id(dev) << 11 |
(pci_domain_nr(dev->bus) & 0x) << 27;
 }
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -44,7 +44,7 @@ static inline void pci_msi_unmask(struct
 
 static inline void __iomem *pci_msix_desc_addr(struct msi_desc *desc)
 {
-   return desc->pci.mask_base + desc->pci.msi_attrib.entry_nr * 
PCI_MSIX_ENTRY_SIZE;
+   return desc->pci.mask_base + desc->msi_index * PCI_MSIX_ENTRY_SIZE;
 }
 
 /*
@@ -356,13 +356,10 @@ msi_setup_entry(struct pci_dev *dev, int
if (dev->dev_flags & PCI_DEV_FLAGS_HAS_MSI_MASKING)
control |= PCI_MSI_FLAGS_MASKBIT;
 
-   entry->pci.msi_attrib.is_msix   = 0;
-   entry->pci.msi_attrib.is_64 = !!(control & 
PCI_MSI_FLAGS_64BIT);
-   entry->pci.msi_attrib.is_virtual= 0;
-   entry->pci.msi_attrib.entry_nr  = 0;
+   entry->pci.msi_attrib.is_64 = !!(control & PCI_MSI_FLAGS_64BIT);
entry->pci.msi_attrib.can_mask  = !pci_msi_ignore_mask &&
  !!(control & PCI_MSI_FLAGS_MASKBIT);
-   entry->pci.msi_attrib.default_irq   = dev->irq; /* Save IOAPIC 
IRQ */
+   entry->pci.msi_attrib.default_irq = dev->irq;
entry->pci.msi_attrib.multi_cap = (control & PCI_MSI_FLAGS_QMASK) >> 1;
entry->pci.msi_attrib.multiple  = ilog2(__roundup_pow_of_two(nvec));
 
@@ -504,12 +501,11 @@ static int msix_setup_entries(struct pci
entry->pci.msi_attrib.is_64 = 1;
 
if (entries)
-   entry->pci.msi_attrib.entry_nr = entries[i].entry;
+   entry->msi_index = entries[i].entry;
else
-   entry->pci.msi_attrib.entry_nr = i;
+   entry->msi_index = i;
 
-   entry->pci.msi_attrib.is_virtual =
-   entry->pci.msi_attrib.entry_nr >= vec_count;
+   entry->pci.msi_attrib.is_virtual = entry->msi_index >= 
vec_count;
 
entry->pci.msi_attrib.can_mask  = !pci_msi_ignore_mask &&
  
!entry->pci.msi_attrib.is_virtual;
@@ -1045,7 +1041,7 @@ int pci_irq_vector(struct pci_dev *dev,
struct msi_desc *entry;
 
for_each_pci_msi_entry(entry, dev) {
-   if (entry->pci.msi_attrib.entry_nr == nr)
+   if (entry->msi_index == nr)
return entry->irq;
}
WARN_ON_ONCE(1);
@@ -1084,7 +1080,7 @@ const struct cpumask *pci_irq_get_affini
struct msi_desc *entry;
 

[patch V3 26/35] genirq/msi: Provide interface to retrieve Linux interrupt number

[Thomas Gleixner]

From: Thomas Gleixner 

This allows drivers to retrieve the Linux interrupt number instead of
fiddling with MSI descriptors.

msi_get_virq() returns the Linux interrupt number or 0 in case that there
is no entry for the given MSI index.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
---
V2: Simplify the implementation and let PCI deal with the PCI specialities - 
Marc
---
 include/linux/msi.h |2 ++
 kernel/irq/msi.c|   36 
 2 files changed, 38 insertions(+)

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -153,6 +153,8 @@ struct msi_device_data {
 
 int msi_setup_device_data(struct device *dev);
 
+unsigned int msi_get_virq(struct device *dev, unsigned int index);
+
 /* Helpers to hide struct msi_desc implementation details */
 #define msi_desc_to_dev(desc)  ((desc)->dev)
 #define dev_to_msi_list(dev)   (&(dev)->msi_list)
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -105,6 +105,42 @@ int msi_setup_device_data(struct device
return 0;
 }
 
+/**
+ * msi_get_virq - Return Linux interrupt number of a MSI interrupt
+ * @dev:   Device to operate on
+ * @index: MSI interrupt index to look for (0-based)
+ *
+ * Return: The Linux interrupt number on success (> 0), 0 if not found
+ */
+unsigned int msi_get_virq(struct device *dev, unsigned int index)
+{
+   struct msi_desc *desc;
+   bool pcimsi;
+
+   if (!dev->msi.data)
+   return 0;
+
+   pcimsi = dev_is_pci(dev) ? to_pci_dev(dev)->msi_enabled : false;
+
+   for_each_msi_entry(desc, dev) {
+   /* PCI-MSI has only one descriptor for multiple interrupts. */
+   if (pcimsi) {
+   if (desc->irq && index < desc->nvec_used)
+   return desc->irq + index;
+   break;
+   }
+
+   /*
+* PCI-MSIX and platform MSI use a descriptor per
+* interrupt.
+*/
+   if (desc->msi_index == index)
+   return desc->irq;
+   }
+   return 0;
+}
+EXPORT_SYMBOL_GPL(msi_get_virq);
+
 #ifdef CONFIG_SYSFS
 static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
 char *buf)

[patch V3 22/35] soc: ti: ti_sci_inta_msi: Use msi_desc::msi_index

[Thomas Gleixner]

From: Thomas Gleixner 

Use the common msi_index member and get rid of the pointless wrapper struct.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Nishanth Menon 
Cc: Tero Kristo 
Cc: Santosh Shilimkar 
Cc: Thomas Gleixner 
Cc: linux-arm-ker...@lists.infradead.org
---
 drivers/irqchip/irq-ti-sci-inta.c |2 +-
 drivers/soc/ti/ti_sci_inta_msi.c  |6 +++---
 include/linux/msi.h   |   16 ++--
 3 files changed, 6 insertions(+), 18 deletions(-)

--- a/drivers/irqchip/irq-ti-sci-inta.c
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -595,7 +595,7 @@ static void ti_sci_inta_msi_set_desc(msi
struct platform_device *pdev = to_platform_device(desc->dev);
 
arg->desc = desc;
-   arg->hwirq = TO_HWIRQ(pdev->id, desc->inta.dev_index);
+   arg->hwirq = TO_HWIRQ(pdev->id, desc->msi_index);
 }
 
 static struct msi_domain_ops ti_sci_inta_msi_ops = {
--- a/drivers/soc/ti/ti_sci_inta_msi.c
+++ b/drivers/soc/ti/ti_sci_inta_msi.c
@@ -84,7 +84,7 @@ static int ti_sci_inta_msi_alloc_descs(s
return -ENOMEM;
}
 
-   msi_desc->inta.dev_index = res->desc[set].start + i;
+   msi_desc->msi_index = res->desc[set].start + i;
INIT_LIST_HEAD(_desc->list);
list_add_tail(_desc->list, dev_to_msi_list(dev));
count++;
@@ -96,7 +96,7 @@ static int ti_sci_inta_msi_alloc_descs(s
return -ENOMEM;
}
 
-   msi_desc->inta.dev_index = res->desc[set].start_sec + i;
+   msi_desc->msi_index = res->desc[set].start_sec + i;
INIT_LIST_HEAD(_desc->list);
list_add_tail(_desc->list, dev_to_msi_list(dev));
count++;
@@ -154,7 +154,7 @@ unsigned int ti_sci_inta_msi_get_virq(st
struct msi_desc *desc;
 
for_each_msi_entry(desc, dev)
-   if (desc->inta.dev_index == dev_index)
+   if (desc->msi_index == dev_index)
return desc->irq;
 
return -ENODEV;
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -107,14 +107,6 @@ struct pci_msi_desc {
 };
 
 /**
- * ti_sci_inta_msi_desc - TISCI based INTA specific msi descriptor data
- * @dev_index: TISCI device index
- */
-struct ti_sci_inta_msi_desc {
-   u16 dev_index;
-};
-
-/**
  * struct msi_desc - Descriptor structure for MSI based interrupts
  * @list:  List head for management
  * @irq:   The base interrupt number
@@ -128,8 +120,7 @@ struct ti_sci_inta_msi_desc {
  * @write_msi_msg_data:Data parameter for the callback.
  *
  * @msi_index: Index of the msi descriptor
- * @pci:   [PCI]   PCI speficic msi descriptor data
- * @inta:  [INTA]  TISCI based INTA specific msi descriptor data
+ * @pci:   PCI specific msi descriptor data
  */
 struct msi_desc {
/* Shared device/bus type independent data */
@@ -147,10 +138,7 @@ struct msi_desc {
void *write_msi_msg_data;
 
u16 msi_index;
-   union {
-   struct pci_msi_desc pci;
-   struct ti_sci_inta_msi_desc inta;
-   };
+   struct pci_msi_desc pci;
 };
 
 /**

[patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()

[Thomas Gleixner]

From: Thomas Gleixner 

Replace open coded MSI descriptor chasing and use the proper accessor
functions instead.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/pci/msi/msi.c |   26 ++
 1 file changed, 10 insertions(+), 16 deletions(-)

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -1061,26 +1061,20 @@ EXPORT_SYMBOL(pci_irq_vector);
  */
 const struct cpumask *pci_irq_get_affinity(struct pci_dev *dev, int nr)
 {
-   if (dev->msix_enabled) {
-   struct msi_desc *entry;
+   int irq = pci_irq_vector(dev, nr);
+   struct msi_desc *desc;
 
-   for_each_pci_msi_entry(entry, dev) {
-   if (entry->msi_index == nr)
-   return >affinity->mask;
-   }
-   WARN_ON_ONCE(1);
+   if (WARN_ON_ONCE(irq <= 0))
return NULL;
-   } else if (dev->msi_enabled) {
-   struct msi_desc *entry = first_pci_msi_entry(dev);
 
-   if (WARN_ON_ONCE(!entry || !entry->affinity ||
-nr >= entry->nvec_used))
-   return NULL;
-
-   return >affinity[nr].mask;
-   } else {
+   desc = irq_get_msi_desc(irq);
+   /* Non-MSI does not have the information handy */
+   if (!desc)
return cpu_possible_mask;
-   }
+
+   if (WARN_ON_ONCE(!desc->affinity))
+   return NULL;
+   return >affinity[nr].mask;
 }
 EXPORT_SYMBOL(pci_irq_get_affinity);
 

[patch V3 33/35] bus: fsl-mc: fsl-mc-allocator: Rework MSI handling

[Thomas Gleixner]

From: Thomas Gleixner 

Storing a pointer to the MSI descriptor just to track the Linux interrupt
number is daft. Just store the interrupt number and be done with it.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Stuart Yoder 
Cc: Laurentiu Tudor 
---
 drivers/bus/fsl-mc/dprc-driver.c|8 
 drivers/bus/fsl-mc/fsl-mc-allocator.c   |9 ++---
 drivers/bus/fsl-mc/fsl-mc-msi.c |6 +++---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c|4 ++--
 drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c|4 +---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c |5 ++---
 drivers/soc/fsl/dpio/dpio-driver.c  |8 
 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c  |4 ++--
 include/linux/fsl/mc.h  |4 ++--
 9 files changed, 22 insertions(+), 30 deletions(-)

--- a/drivers/bus/fsl-mc/dprc-driver.c
+++ b/drivers/bus/fsl-mc/dprc-driver.c
@@ -400,7 +400,7 @@ static irqreturn_t dprc_irq0_handler_thr
struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_dev);
struct fsl_mc_io *mc_io = mc_dev->mc_io;
-   struct msi_desc *msi_desc = mc_dev->irqs[0]->msi_desc;
+   int irq = mc_dev->irqs[0]->virq;
 
dev_dbg(dev, "DPRC IRQ %d triggered on CPU %u\n",
irq_num, smp_processor_id());
@@ -409,7 +409,7 @@ static irqreturn_t dprc_irq0_handler_thr
return IRQ_HANDLED;
 
mutex_lock(_bus->scan_mutex);
-   if (!msi_desc || msi_desc->irq != (u32)irq_num)
+   if (irq != (u32)irq_num)
goto out;
 
status = 0;
@@ -521,7 +521,7 @@ static int register_dprc_irq_handler(str
 * function that programs the MSI physically in the device
 */
error = devm_request_threaded_irq(_dev->dev,
- irq->msi_desc->irq,
+ irq->virq,
  dprc_irq0_handler,
  dprc_irq0_handler_thread,
  IRQF_NO_SUSPEND | IRQF_ONESHOT,
@@ -771,7 +771,7 @@ static void dprc_teardown_irq(struct fsl
 
(void)disable_dprc_irq(mc_dev);
 
-   devm_free_irq(_dev->dev, irq->msi_desc->irq, _dev->dev);
+   devm_free_irq(_dev->dev, irq->virq, _dev->dev);
 
fsl_mc_free_irqs(mc_dev);
 }
--- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
+++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
@@ -350,7 +350,6 @@ int fsl_mc_populate_irq_pool(struct fsl_
 unsigned int irq_count)
 {
unsigned int i;
-   struct msi_desc *msi_desc;
struct fsl_mc_device_irq *irq_resources;
struct fsl_mc_device_irq *mc_dev_irq;
int error;
@@ -388,16 +387,12 @@ int fsl_mc_populate_irq_pool(struct fsl_
mc_dev_irq->resource.type = res_pool->type;
mc_dev_irq->resource.data = mc_dev_irq;
mc_dev_irq->resource.parent_pool = res_pool;
+   mc_dev_irq->virq = msi_get_virq(_bus_dev->dev, i);
+   mc_dev_irq->resource.id = mc_dev_irq->virq;
INIT_LIST_HEAD(_dev_irq->resource.node);
list_add_tail(_dev_irq->resource.node, _pool->free_list);
}
 
-   for_each_msi_entry(msi_desc, _bus_dev->dev) {
-   mc_dev_irq = _resources[msi_desc->msi_index];
-   mc_dev_irq->msi_desc = msi_desc;
-   mc_dev_irq->resource.id = msi_desc->irq;
-   }
-
res_pool->max_count = irq_count;
res_pool->free_count = irq_count;
mc_bus->irq_resources = irq_resources;
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -58,11 +58,11 @@ static void fsl_mc_msi_update_dom_ops(st
 }
 
 static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
-  struct fsl_mc_device_irq *mc_dev_irq)
+  struct fsl_mc_device_irq *mc_dev_irq,
+  struct msi_desc *msi_desc)
 {
int error;
struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
-   struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
struct dprc_irq_cfg irq_cfg;
 
/*
@@ -129,7 +129,7 @@ static void fsl_mc_msi_write_msg(struct
/*
 * Program the MSI (paddr, value) pair in the device:
 */
-   __fsl_mc_msi_write_msg(mc_bus_dev, mc_dev_irq);
+   __fsl_mc_msi_write_msg(mc_bus_dev, mc_dev_irq, msi_desc);
 }
 
 static void fsl_mc_msi_update_chip_ops(struct msi_domain_info *info)
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -4246,7 +4246,7 @@ static int dpaa2_eth_setup_irqs(struct f
}
 
irq = ls_dev->irqs[0];
-   err = 

[patch V3 29/35] dmaengine: mv_xor_v2: Get rid of msi_desc abuse

[Thomas Gleixner]

From: Thomas Gleixner 

Storing a pointer to the MSI descriptor just to keep track of the Linux
interrupt number is daft. Use msi_get_virq() instead.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Vinod Koul 
Cc: dmaeng...@vger.kernel.org
---
 drivers/dma/mv_xor_v2.c |   16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

--- a/drivers/dma/mv_xor_v2.c
+++ b/drivers/dma/mv_xor_v2.c
@@ -149,7 +149,7 @@ struct mv_xor_v2_descriptor {
  * @desc_size: HW descriptor size
  * @npendings: number of pending descriptors (for which tx_submit has
  * @hw_queue_idx: HW queue index
- * @msi_desc: local interrupt descriptor information
+ * @irq: The Linux interrupt number
  * been called, but not yet issue_pending)
  */
 struct mv_xor_v2_device {
@@ -168,7 +168,7 @@ struct mv_xor_v2_device {
int desc_size;
unsigned int npendings;
unsigned int hw_queue_idx;
-   struct msi_desc *msi_desc;
+   unsigned int irq;
 };
 
 /**
@@ -718,7 +718,6 @@ static int mv_xor_v2_probe(struct platfo
int i, ret = 0;
struct dma_device *dma_dev;
struct mv_xor_v2_sw_desc *sw_desc;
-   struct msi_desc *msi_desc;
 
BUILD_BUG_ON(sizeof(struct mv_xor_v2_descriptor) !=
 MV_XOR_V2_EXT_DESC_SIZE);
@@ -770,14 +769,9 @@ static int mv_xor_v2_probe(struct platfo
if (ret)
goto disable_clk;
 
-   msi_desc = first_msi_entry(>dev);
-   if (!msi_desc) {
-   ret = -ENODEV;
-   goto free_msi_irqs;
-   }
-   xor_dev->msi_desc = msi_desc;
+   xor_dev->irq = msi_get_virq(>dev, 0);
 
-   ret = devm_request_irq(>dev, msi_desc->irq,
+   ret = devm_request_irq(>dev, xor_dev->irq,
   mv_xor_v2_interrupt_handler, 0,
   dev_name(>dev), xor_dev);
if (ret)
@@ -892,7 +886,7 @@ static int mv_xor_v2_remove(struct platf
  xor_dev->desc_size * MV_XOR_V2_DESC_NUM,
  xor_dev->hw_desq_virt, xor_dev->hw_desq);
 
-   devm_free_irq(>dev, xor_dev->msi_desc->irq, xor_dev);
+   devm_free_irq(>dev, xor_dev->irq, xor_dev);
 
platform_msi_domain_free_irqs(>dev);
 

[patch V3 32/35] mailbox: bcm-flexrm-mailbox: Rework MSI interrupt handling

[Thomas Gleixner]

From: Thomas Gleixner 

No point in retrieving the MSI descriptors. Just query the Linux interrupt
number.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Jassi Brar 

---
 drivers/mailbox/bcm-flexrm-mailbox.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1497,7 +1497,6 @@ static int flexrm_mbox_probe(struct plat
int index, ret = 0;
void __iomem *regs;
void __iomem *regs_end;
-   struct msi_desc *desc;
struct resource *iomem;
struct flexrm_ring *ring;
struct flexrm_mbox *mbox;
@@ -1608,10 +1607,8 @@ static int flexrm_mbox_probe(struct plat
goto fail_destroy_cmpl_pool;
 
/* Save alloced IRQ numbers for each ring */
-   for_each_msi_entry(desc, dev) {
-   ring = >rings[desc->msi_index];
-   ring->irq = desc->irq;
-   }
+   for (index = 0; index < mbox->num_rings; index++)
+   mbox->rings[index].irq = msi_get_virq(dev, index);
 
/* Check availability of debugfs */
if (!debugfs_initialized())

[patch V3 35/35] dmaengine: qcom_hidma: Cleanup MSI handling

[Thomas Gleixner]

From: Thomas Gleixner 

There is no reason to walk the MSI descriptors to retrieve the interrupt
number for a device. Use msi_get_virq() instead.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Acked-by: Sinan Kaya 
Cc: dmaeng...@vger.kernel.org
---
 drivers/dma/qcom/hidma.c |   42 ++
 1 file changed, 18 insertions(+), 24 deletions(-)

--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -678,11 +678,13 @@ static void hidma_free_msis(struct hidma
 {
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
struct device *dev = dmadev->ddev.dev;
-   struct msi_desc *desc;
+   int i, virq;
 
-   /* free allocated MSI interrupts above */
-   for_each_msi_entry(desc, dev)
-   devm_free_irq(dev, desc->irq, >lldev);
+   for (i = 0; i < HIDMA_MSI_INTS; i++) {
+   virq = msi_get_virq(dev, i);
+   if (virq)
+   devm_free_irq(dev, virq, >lldev);
+   }
 
platform_msi_domain_free_irqs(dev);
 #endif
@@ -692,45 +694,37 @@ static int hidma_request_msi(struct hidm
 struct platform_device *pdev)
 {
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
-   int rc;
-   struct msi_desc *desc;
-   struct msi_desc *failed_desc = NULL;
+   int rc, i, virq;
 
rc = platform_msi_domain_alloc_irqs(>dev, HIDMA_MSI_INTS,
hidma_write_msi_msg);
if (rc)
return rc;
 
-   for_each_msi_entry(desc, >dev) {
-   if (!desc->msi_index)
-   dmadev->msi_virqbase = desc->irq;
-
-   rc = devm_request_irq(>dev, desc->irq,
+   for (i = 0; i < HIDMA_MSI_INTS; i++) {
+   virq = msi_get_virq(>dev, i);
+   rc = devm_request_irq(>dev, virq,
   hidma_chirq_handler_msi,
   0, "qcom-hidma-msi",
   >lldev);
-   if (rc) {
-   failed_desc = desc;
+   if (rc)
break;
-   }
+   if (!i)
+   dmadev->msi_virqbase = virq;
}
 
if (rc) {
/* free allocated MSI interrupts above */
-   for_each_msi_entry(desc, >dev) {
-   if (desc == failed_desc)
-   break;
-   devm_free_irq(>dev, desc->irq,
- >lldev);
+   for (--i; i >= 0; i--) {
+   virq = msi_get_virq(>dev, i);
+   devm_free_irq(>dev, virq, >lldev);
}
+   dev_warn(>dev,
+"failed to request MSI irq, falling back to wired 
IRQ\n");
} else {
/* Add callback to free MSIs on teardown */
hidma_ll_setup_irq(dmadev->lldev, true);
-
}
-   if (rc)
-   dev_warn(>dev,
-"failed to request MSI irq, falling back to wired 
IRQ\n");
return rc;
 #else
return -EINVAL;

[patch V3 15/35] platform-msi: Let the core code handle sysfs groups

[Thomas Gleixner]

From: Thomas Gleixner 

Set the domain info flag and remove the local sysfs code.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/base/platform-msi.c |   11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -23,7 +23,6 @@
 struct platform_msi_priv_data {
struct device   *dev;
void*host_data;
-   const struct attribute_group**msi_irq_groups;
msi_alloc_info_targ;
irq_write_msi_msg_t write_msg;
int devid;
@@ -191,6 +190,7 @@ struct irq_domain *platform_msi_create_i
platform_msi_update_dom_ops(info);
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
platform_msi_update_chip_ops(info);
+   info->flags |= MSI_FLAG_DEV_SYSFS;
 
domain = msi_create_irq_domain(fwnode, info, parent);
if (domain)
@@ -279,16 +279,8 @@ int platform_msi_domain_alloc_irqs(struc
if (err)
goto out_free_desc;
 
-   priv_data->msi_irq_groups = msi_populate_sysfs(dev);
-   if (IS_ERR(priv_data->msi_irq_groups)) {
-   err = PTR_ERR(priv_data->msi_irq_groups);
-   goto out_free_irqs;
-   }
-
return 0;
 
-out_free_irqs:
-   msi_domain_free_irqs(dev->msi.domain, dev);
 out_free_desc:
platform_msi_free_descs(dev, 0, nvec);
 out_free_priv_data:
@@ -308,7 +300,6 @@ void platform_msi_domain_free_irqs(struc
struct msi_desc *desc;
 
desc = first_msi_entry(dev);
-   msi_destroy_sysfs(dev, 
desc->platform.msi_priv_data->msi_irq_groups);
platform_msi_free_priv_data(desc->platform.msi_priv_data);
}
 

[patch V3 16/35] genirq/msi: Remove the original sysfs interfaces

[Thomas Gleixner]

From: Thomas Gleixner 

No more users. Refactor the core code accordingly and move the global
interface under CONFIG_PCI_MSI_ARCH_FALLBACKS.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 include/linux/msi.h |   29 +++-
 kernel/irq/msi.c|   53 +++-
 2 files changed, 28 insertions(+), 54 deletions(-)

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -246,26 +246,6 @@ void __pci_write_msi_msg(struct msi_desc
 void pci_msi_mask_irq(struct irq_data *data);
 void pci_msi_unmask_irq(struct irq_data *data);
 
-#ifdef CONFIG_SYSFS
-int msi_device_populate_sysfs(struct device *dev);
-void msi_device_destroy_sysfs(struct device *dev);
-
-const struct attribute_group **msi_populate_sysfs(struct device *dev);
-void msi_destroy_sysfs(struct device *dev,
-  const struct attribute_group **msi_irq_groups);
-#else
-static inline int msi_device_populate_sysfs(struct device *dev) { return 0; }
-static inline void msi_device_destroy_sysfs(struct device *dev) { }
-
-static inline const struct attribute_group **msi_populate_sysfs(struct device 
*dev)
-{
-   return NULL;
-}
-static inline void msi_destroy_sysfs(struct device *dev, const struct 
attribute_group **msi_irq_groups)
-{
-}
-#endif
-
 /*
  * The arch hooks to setup up msi irqs. Default functions are implemented
  * as weak symbols so that they /can/ be overriden by architecture specific
@@ -279,7 +259,14 @@ int arch_setup_msi_irq(struct pci_dev *d
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-#endif
+#ifdef CONFIG_SYSFS
+int msi_device_populate_sysfs(struct device *dev);
+void msi_device_destroy_sysfs(struct device *dev);
+#else /* CONFIG_SYSFS */
+static inline int msi_device_populate_sysfs(struct device *dev) { return 0; }
+static inline void msi_device_destroy_sysfs(struct device *dev) { }
+#endif /* !CONFIG_SYSFS */
+#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
 
 /*
  * The restore hook is still available even for fully irq domain based
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -118,12 +118,8 @@ static ssize_t msi_mode_show(struct devi
 /**
  * msi_populate_sysfs - Populate msi_irqs sysfs entries for devices
  * @dev:   The device(PCI, platform etc) who will get sysfs entries
- *
- * Return attribute_group ** so that specific bus MSI can save it to
- * somewhere during initilizing msi irqs. If devices has no MSI irq,
- * return NULL; if it fails to populate sysfs, return ERR_PTR
  */
-const struct attribute_group **msi_populate_sysfs(struct device *dev)
+static const struct attribute_group **msi_populate_sysfs(struct device *dev)
 {
const struct attribute_group **msi_irq_groups;
struct attribute **msi_attrs, *msi_attr;
@@ -214,41 +210,32 @@ int msi_device_populate_sysfs(struct dev
 }
 
 /**
- * msi_destroy_sysfs - Destroy msi_irqs sysfs entries for devices
- * @dev:   The device(PCI, platform etc) who will remove sysfs 
entries
- * @msi_irq_groups:attribute_group for device msi_irqs entries
- */
-void msi_destroy_sysfs(struct device *dev, const struct attribute_group 
**msi_irq_groups)
-{
-   struct device_attribute *dev_attr;
-   struct attribute **msi_attrs;
-   int count = 0;
-
-   if (msi_irq_groups) {
-   sysfs_remove_groups(>kobj, msi_irq_groups);
-   msi_attrs = msi_irq_groups[0]->attrs;
-   while (msi_attrs[count]) {
-   dev_attr = container_of(msi_attrs[count],
-   struct device_attribute, attr);
-   kfree(dev_attr->attr.name);
-   kfree(dev_attr);
-   ++count;
-   }
-   kfree(msi_attrs);
-   kfree(msi_irq_groups[0]);
-   kfree(msi_irq_groups);
-   }
-}
-
-/**
  * msi_device_destroy_sysfs - Destroy msi_irqs sysfs entries for a device
  * @dev:   The device (PCI, platform etc) for which to remove
  * sysfs entries
  */
 void msi_device_destroy_sysfs(struct device *dev)
 {
-   msi_destroy_sysfs(dev, dev->msi.data->attrs);
+   const struct attribute_group **msi_irq_groups = dev->msi.data->attrs;
+   struct device_attribute *dev_attr;
+   struct attribute **msi_attrs;
+   int count = 0;
+
dev->msi.data->attrs = NULL;
+   if (!msi_irq_groups)
+   return;
+
+   sysfs_remove_groups(>kobj, msi_irq_groups);
+   msi_attrs = msi_irq_groups[0]->attrs;
+   while (msi_attrs[count]) {
+   dev_attr = container_of(msi_attrs[count], struct 
device_attribute, attr);
+   kfree(dev_attr->attr.name);
+   kfree(dev_attr);
+   ++count;
+   }
+   kfree(msi_attrs);
+   kfree(msi_irq_groups[0]);
+   

[patch V3 13/35] genirq/msi: Provide msi_device_populate/destroy_sysfs()

[Thomas Gleixner]

From: Thomas Gleixner 

Add new allocation functions which can be activated by domain info
flags. They store the groups pointer in struct msi_device_data.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 include/linux/msi.h |4 
 kernel/irq/msi.c|   42 --
 2 files changed, 44 insertions(+), 2 deletions(-)

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -56,6 +56,8 @@ struct irq_data;
 struct msi_desc;
 struct pci_dev;
 struct platform_msi_priv_data;
+struct attribute_group;
+
 void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 #ifdef CONFIG_GENERIC_MSI_IRQ
 void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
@@ -174,9 +176,11 @@ struct msi_desc {
 /**
  * msi_device_data - MSI per device data
  * @properties:MSI properties which are interesting to drivers
+ * @attrs: Pointer to the sysfs attribute group
  */
 struct msi_device_data {
unsigned long   properties;
+   const struct attribute_group**attrs;
 };
 
 int msi_setup_device_data(struct device *dev);
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -200,6 +200,20 @@ const struct attribute_group **msi_popul
 }
 
 /**
+ * msi_device_populate_sysfs - Populate msi_irqs sysfs entries for a device
+ * @dev:   The device (PCI, platform etc) which will get sysfs entries
+ */
+int msi_device_populate_sysfs(struct device *dev)
+{
+   const struct attribute_group **group = msi_populate_sysfs(dev);
+
+   if (IS_ERR(group))
+   return PTR_ERR(group);
+   dev->msi.data->attrs = group;
+   return 0;
+}
+
+/**
  * msi_destroy_sysfs - Destroy msi_irqs sysfs entries for devices
  * @dev:   The device(PCI, platform etc) who will remove sysfs 
entries
  * @msi_irq_groups:attribute_group for device msi_irqs entries
@@ -225,6 +239,17 @@ void msi_destroy_sysfs(struct device *de
kfree(msi_irq_groups);
}
 }
+
+/**
+ * msi_device_destroy_sysfs - Destroy msi_irqs sysfs entries for a device
+ * @dev:   The device (PCI, platform etc) for which to remove
+ * sysfs entries
+ */
+void msi_device_destroy_sysfs(struct device *dev)
+{
+   msi_destroy_sysfs(dev, dev->msi.data->attrs);
+   dev->msi.data->attrs = NULL;
+}
 #endif
 
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
@@ -672,8 +697,19 @@ int msi_domain_alloc_irqs(struct irq_dom
 {
struct msi_domain_info *info = domain->host_data;
struct msi_domain_ops *ops = info->ops;
+   int ret;
 
-   return ops->domain_alloc_irqs(domain, dev, nvec);
+   ret = ops->domain_alloc_irqs(domain, dev, nvec);
+   if (ret)
+   return ret;
+
+   if (!(info->flags & MSI_FLAG_DEV_SYSFS))
+   return 0;
+
+   ret = msi_device_populate_sysfs(dev);
+   if (ret)
+   msi_domain_free_irqs(domain, dev);
+   return ret;
 }
 
 void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
@@ -712,7 +748,9 @@ void msi_domain_free_irqs(struct irq_dom
struct msi_domain_info *info = domain->host_data;
struct msi_domain_ops *ops = info->ops;
 
-   return ops->domain_free_irqs(domain, dev);
+   if (info->flags & MSI_FLAG_DEV_SYSFS)
+   msi_device_destroy_sysfs(dev);
+   ops->domain_free_irqs(domain, dev);
 }
 
 /**

[patch V3 12/35] soc: ti: ti_sci_inta_msi: Allocate MSI device data on first use

[Thomas Gleixner]

From: Thomas Gleixner 

Allocate the MSI device data on first invocation of the allocation function.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Nishanth Menon 
Cc: Tero Kristo 
Cc: Santosh Shilimkar 
Cc: linux-arm-ker...@lists.infradead.org
---
 drivers/soc/ti/ti_sci_inta_msi.c |4 
 1 file changed, 4 insertions(+)

--- a/drivers/soc/ti/ti_sci_inta_msi.c
+++ b/drivers/soc/ti/ti_sci_inta_msi.c
@@ -120,6 +120,10 @@ int ti_sci_inta_msi_domain_alloc_irqs(st
if (pdev->id < 0)
return -ENODEV;
 
+   ret = msi_setup_device_data(dev);
+   if (ret)
+   return ret;
+
nvec = ti_sci_inta_msi_alloc_descs(dev, res);
if (nvec <= 0)
return nvec;

[patch V3 11/35] bus: fsl-mc-msi: Allocate MSI device data on first use

[Thomas Gleixner]

From: Thomas Gleixner 

Allocate the MSI device data on first invocation of the allocation function.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Cc: Stuart Yoder 
Cc: Laurentiu Tudor 
---
 drivers/bus/fsl-mc/fsl-mc-msi.c |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -253,6 +253,14 @@ int fsl_mc_msi_domain_alloc_irqs(struct
struct irq_domain *msi_domain;
int error;
 
+   msi_domain = dev_get_msi_domain(dev);
+   if (!msi_domain)
+   return -EINVAL;
+
+   error = msi_setup_device_data(dev);
+   if (error)
+   return error;
+
if (!list_empty(dev_to_msi_list(dev)))
return -EINVAL;
 
@@ -260,12 +268,6 @@ int fsl_mc_msi_domain_alloc_irqs(struct
if (error < 0)
return error;
 
-   msi_domain = dev_get_msi_domain(dev);
-   if (!msi_domain) {
-   error = -EINVAL;
-   goto cleanup_msi_descs;
-   }
-
/*
 * NOTE: Calling this function will trigger the invocation of the
 * its_fsl_mc_msi_prepare() callback

[patch V3 10/35] platform-msi: Allocate MSI device data on first use

[Thomas Gleixner]

From: Thomas Gleixner 

Allocate the MSI device data on first invocation of the allocation function
for platform MSI private data.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 drivers/base/platform-msi.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -204,6 +204,8 @@ platform_msi_alloc_priv_data(struct devi
 irq_write_msi_msg_t write_msi_msg)
 {
struct platform_msi_priv_data *datap;
+   int err;
+
/*
 * Limit the number of interrupts to 2048 per device. Should we
 * need to bump this up, DEV_ID_SHIFT should be adjusted
@@ -218,6 +220,10 @@ platform_msi_alloc_priv_data(struct devi
return ERR_PTR(-EINVAL);
}
 
+   err = msi_setup_device_data(dev);
+   if (err)
+   return ERR_PTR(err);
+
/* Already had a helping of MSI? Greed... */
if (!list_empty(dev_to_msi_list(dev)))
return ERR_PTR(-EBUSY);
@@ -229,7 +235,7 @@ platform_msi_alloc_priv_data(struct devi
datap->devid = ida_simple_get(_msi_devid_ida,
  0, 1 << DEV_ID_SHIFT, GFP_KERNEL);
if (datap->devid < 0) {
-   int err = datap->devid;
+   err = datap->devid;
kfree(datap);
return ERR_PTR(err);
}

[patch V3 09/35] PCI/MSI: Allocate MSI device data on first use

[Thomas Gleixner]

From: Thomas Gleixner 

Allocate MSI device data on first use, i.e. when a PCI driver invokes one
of the PCI/MSI enablement functions.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
Acked-by: Bjorn Helgaas 
---
 drivers/pci/msi/msi.c |   20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -900,10 +900,12 @@ static int __pci_enable_msi_range(struct
 /* deprecated, don't use */
 int pci_enable_msi(struct pci_dev *dev)
 {
-   int rc = __pci_enable_msi_range(dev, 1, 1, NULL);
-   if (rc < 0)
-   return rc;
-   return 0;
+   int rc = msi_setup_device_data(>dev);
+
+   if (!rc)
+   rc = __pci_enable_msi_range(dev, 1, 1, NULL);
+
+   return rc < 0 ? rc : 0;
 }
 EXPORT_SYMBOL(pci_enable_msi);
 
@@ -958,7 +960,11 @@ static int __pci_enable_msix_range(struc
 int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
int minvec, int maxvec)
 {
-   return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL, 0);
+   int ret = msi_setup_device_data(>dev);
+
+   if (!ret)
+   ret = __pci_enable_msix_range(dev, entries, minvec, maxvec, 
NULL, 0);
+   return ret;
 }
 EXPORT_SYMBOL(pci_enable_msix_range);
 
@@ -985,8 +991,12 @@ int pci_alloc_irq_vectors_affinity(struc
   struct irq_affinity *affd)
 {
struct irq_affinity msi_default_affd = {0};
+   int ret = msi_setup_device_data(>dev);
int nvecs = -ENOSPC;
 
+   if (ret)
+   return ret;
+
if (flags & PCI_IRQ_AFFINITY) {
if (!affd)
affd = _default_affd;

[patch V3 08/35] device: Add device:: Msi_data pointer and struct msi_device_data

[Thomas Gleixner]

From: Thomas Gleixner 

Create struct msi_device_data and add a pointer of that type to struct
dev_msi_info, which is part of struct device. Provide an allocator function
which can be invoked from the MSI interrupt allocation code pathes.

Add a properties field to the data structure as a first member so the
allocation size is not zero bytes. The field will be uses later on.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 
---
 include/linux/device.h |5 +
 include/linux/msi.h|   18 ++
 kernel/irq/msi.c   |   32 
 3 files changed, 55 insertions(+)

--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -45,6 +45,7 @@ struct iommu_ops;
 struct iommu_group;
 struct dev_pin_info;
 struct dev_iommu;
+struct msi_device_data;
 
 /**
  * struct subsys_interface - interfaces to device functions
@@ -374,11 +375,15 @@ struct dev_links_info {
 /**
  * struct dev_msi_info - Device data related to MSI
  * @domain:The MSI interrupt domain associated to the device
+ * @data:  Pointer to MSI device data
  */
 struct dev_msi_info {
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
struct irq_domain   *domain;
 #endif
+#ifdef CONFIG_GENERIC_MSI_IRQ
+   struct msi_device_data  *data;
+#endif
 };
 
 /**
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -171,6 +171,16 @@ struct msi_desc {
};
 };
 
+/**
+ * msi_device_data - MSI per device data
+ * @properties:MSI properties which are interesting to drivers
+ */
+struct msi_device_data {
+   unsigned long   properties;
+};
+
+int msi_setup_device_data(struct device *dev);
+
 /* Helpers to hide struct msi_desc implementation details */
 #define msi_desc_to_dev(desc)  ((desc)->dev)
 #define dev_to_msi_list(dev)   (&(dev)->msi_list)
@@ -233,10 +243,16 @@ void pci_msi_mask_irq(struct irq_data *d
 void pci_msi_unmask_irq(struct irq_data *data);
 
 #ifdef CONFIG_SYSFS
+int msi_device_populate_sysfs(struct device *dev);
+void msi_device_destroy_sysfs(struct device *dev);
+
 const struct attribute_group **msi_populate_sysfs(struct device *dev);
 void msi_destroy_sysfs(struct device *dev,
   const struct attribute_group **msi_irq_groups);
 #else
+static inline int msi_device_populate_sysfs(struct device *dev) { return 0; }
+static inline void msi_device_destroy_sysfs(struct device *dev) { }
+
 static inline const struct attribute_group **msi_populate_sysfs(struct device 
*dev)
 {
return NULL;
@@ -384,6 +400,8 @@ enum {
MSI_FLAG_MUST_REACTIVATE= (1 << 5),
/* Is level-triggered capable, using two messages */
MSI_FLAG_LEVEL_CAPABLE  = (1 << 6),
+   /* Populate sysfs on alloc() and destroy it on free() */
+   MSI_FLAG_DEV_SYSFS  = (1 << 7),
 };
 
 int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -73,6 +73,38 @@ void get_cached_msi_msg(unsigned int irq
 }
 EXPORT_SYMBOL_GPL(get_cached_msi_msg);
 
+static void msi_device_data_release(struct device *dev, void *res)
+{
+   WARN_ON_ONCE(!list_empty(>msi_list));
+   dev->msi.data = NULL;
+}
+
+/**
+ * msi_setup_device_data - Setup MSI device data
+ * @dev:   Device for which MSI device data should be set up
+ *
+ * Return: 0 on success, appropriate error code otherwise
+ *
+ * This can be called more than once for @dev. If the MSI device data is
+ * already allocated the call succeeds. The allocated memory is
+ * automatically released when the device is destroyed.
+ */
+int msi_setup_device_data(struct device *dev)
+{
+   struct msi_device_data *md;
+
+   if (dev->msi.data)
+   return 0;
+
+   md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL);
+   if (!md)
+   return -ENOMEM;
+
+   dev->msi.data = md;
+   devres_add(dev, md);
+   return 0;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
 char *buf)

[patch V3 07/35] device: Move MSI related data into a struct

[Thomas Gleixner]

From: Thomas Gleixner 

The only unconditional part of MSI data in struct device is the irqdomain
pointer. Everything else can be allocated on demand. Create a data
structure and move the irqdomain pointer into it. The other MSI specific
parts are going to be removed from struct device in later steps.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Jason Gunthorpe 

---
 drivers/base/platform-msi.c |   12 ++--
 drivers/dma/ti/k3-udma.c|4 ++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |2 +-
 drivers/irqchip/irq-mvebu-icu.c |6 +++---
 drivers/soc/ti/k3-ringacc.c |4 ++--
 drivers/soc/ti/ti_sci_inta_msi.c|2 +-
 include/linux/device.h  |   20 ++--
 7 files changed, 29 insertions(+), 21 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -210,10 +210,10 @@ platform_msi_alloc_priv_data(struct devi
 * accordingly (which would impact the max number of MSI
 * capable devices).
 */
-   if (!dev->msi_domain || !write_msi_msg || !nvec || nvec > MAX_DEV_MSIS)
+   if (!dev->msi.domain || !write_msi_msg || !nvec || nvec > MAX_DEV_MSIS)
return ERR_PTR(-EINVAL);
 
-   if (dev->msi_domain->bus_token != DOMAIN_BUS_PLATFORM_MSI) {
+   if (dev->msi.domain->bus_token != DOMAIN_BUS_PLATFORM_MSI) {
dev_err(dev, "Incompatible msi_domain, giving up\n");
return ERR_PTR(-EINVAL);
}
@@ -269,7 +269,7 @@ int platform_msi_domain_alloc_irqs(struc
if (err)
goto out_free_priv_data;
 
-   err = msi_domain_alloc_irqs(dev->msi_domain, dev, nvec);
+   err = msi_domain_alloc_irqs(dev->msi.domain, dev, nvec);
if (err)
goto out_free_desc;
 
@@ -282,7 +282,7 @@ int platform_msi_domain_alloc_irqs(struc
return 0;
 
 out_free_irqs:
-   msi_domain_free_irqs(dev->msi_domain, dev);
+   msi_domain_free_irqs(dev->msi.domain, dev);
 out_free_desc:
platform_msi_free_descs(dev, 0, nvec);
 out_free_priv_data:
@@ -306,7 +306,7 @@ void platform_msi_domain_free_irqs(struc
platform_msi_free_priv_data(desc->platform.msi_priv_data);
}
 
-   msi_domain_free_irqs(dev->msi_domain, dev);
+   msi_domain_free_irqs(dev->msi.domain, dev);
platform_msi_free_descs(dev, 0, MAX_DEV_MSIS);
 }
 EXPORT_SYMBOL_GPL(platform_msi_domain_free_irqs);
@@ -354,7 +354,7 @@ struct irq_domain *
return NULL;
 
data->host_data = host_data;
-   domain = irq_domain_create_hierarchy(dev->msi_domain, 0,
+   domain = irq_domain_create_hierarchy(dev->msi.domain, 0,
 is_tree ? 0 : nvec,
 dev->fwnode, ops, data);
if (!domain)
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -5279,9 +5279,9 @@ static int udma_probe(struct platform_de
if (IS_ERR(ud->ringacc))
return PTR_ERR(ud->ringacc);
 
-   dev->msi_domain = of_msi_get_domain(dev, dev->of_node,
+   dev->msi.domain = of_msi_get_domain(dev, dev->of_node,
DOMAIN_BUS_TI_SCI_INTA_MSI);
-   if (!dev->msi_domain) {
+   if (!dev->msi.domain) {
dev_err(dev, "Failed to get MSI domain\n");
return -EPROBE_DEFER;
}
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3170,7 +3170,7 @@ static void arm_smmu_setup_msis(struct a
if (!(smmu->features & ARM_SMMU_FEAT_MSI))
return;
 
-   if (!dev->msi_domain) {
+   if (!dev->msi.domain) {
dev_info(smmu->dev, "msi_domain absent - falling back to wired 
irqs\n");
return;
}
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -314,12 +314,12 @@ static int mvebu_icu_subset_probe(struct
msi_data->subset_data = of_device_get_match_data(dev);
}
 
-   dev->msi_domain = of_msi_get_domain(dev, dev->of_node,
+   dev->msi.domain = of_msi_get_domain(dev, dev->of_node,
DOMAIN_BUS_PLATFORM_MSI);
-   if (!dev->msi_domain)
+   if (!dev->msi.domain)
return -EPROBE_DEFER;
 
-   msi_parent_dn = irq_domain_get_of_node(dev->msi_domain);
+   msi_parent_dn = irq_domain_get_of_node(dev->msi.domain);
if (!msi_parent_dn)
return -ENODEV;
 
--- a/drivers/soc/ti/k3-ringacc.c
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -1356,9 +1356,9 @@ static int k3_ringacc_init(struct platfo
struct resource *res;
int ret, i;
 
-   dev->msi_domain = of_msi_get_domain(dev, dev->of_node,
+   dev->msi.domain = of_msi_get_domain(dev, dev->of_node,

[patch V3 06/35] powerpc/pseries/msi: Use PCI device properties

[Thomas Gleixner]

From: Thomas Gleixner 

instead of fiddling with MSI descriptors.

Signed-off-by: Thomas Gleixner 
Cc: Michael Ellerman 
Cc: linuxppc-...@lists.ozlabs.org
---
V3: Use pci_dev->msix_enabled - Jason
---
 arch/powerpc/platforms/pseries/msi.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -448,8 +448,7 @@ static int pseries_msi_ops_prepare(struc
   int nvec, msi_alloc_info_t *arg)
 {
struct pci_dev *pdev = to_pci_dev(dev);
-   struct msi_desc *desc = first_pci_msi_entry(pdev);
-   int type = desc->pci.msi_attrib.is_msix ? PCI_CAP_ID_MSIX : 
PCI_CAP_ID_MSI;
+   int type = pdev->msix_enabled ? PCI_CAP_ID_MSIX : PCI_CAP_ID_MSI;
 
return rtas_prepare_msi_irqs(pdev, nvec, type, arg);
 }

[patch V3 05/35] powerpc/cell/axon_msi: Use PCI device property

[Thomas Gleixner]

From: Thomas Gleixner 

instead of fiddling with MSI descriptors.

Signed-off-by: Thomas Gleixner 
Cc: Arnd Bergmann 
Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: linuxppc-...@lists.ozlabs.org
---
V3: Use pci_dev property - Jason
V2: Invoke the function with the correct number of arguments - Andy
---
 arch/powerpc/platforms/cell/axon_msi.c |5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -199,7 +199,6 @@ static struct axon_msic *find_msi_transl
 static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
 {
struct device_node *dn;
-   struct msi_desc *entry;
int len;
const u32 *prop;
 
@@ -209,10 +208,8 @@ static int setup_msi_msg_address(struct
return -ENODEV;
}
 
-   entry = first_pci_msi_entry(dev);
-
for (; dn; dn = of_get_next_parent(dn)) {
-   if (entry->pci.msi_attrib.is_64) {
+   if (!dev->no_64bit_msi) {
prop = of_get_property(dn, "msi-address-64", );
if (prop)
break;

[patch V3 04/35] genirq/msi: Use PCI device property

[Thomas Gleixner]

From: Thomas Gleixner 

to determine whether this is MSI or MSIX instead of consulting MSI
descriptors.

Signed-off-by: Thomas Gleixner 
---
V2: Use PCI device property - Jason
---
 kernel/irq/msi.c |   17 ++---
 1 file changed, 2 insertions(+), 15 deletions(-)

--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -77,21 +77,8 @@ EXPORT_SYMBOL_GPL(get_cached_msi_msg);
 static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
 char *buf)
 {
-   struct msi_desc *entry;
-   bool is_msix = false;
-   unsigned long irq;
-   int retval;
-
-   retval = kstrtoul(attr->attr.name, 10, );
-   if (retval)
-   return retval;
-
-   entry = irq_get_msi_desc(irq);
-   if (!entry)
-   return -ENODEV;
-
-   if (dev_is_pci(dev))
-   is_msix = entry->pci.msi_attrib.is_msix;
+   /* MSI vs. MSIX is per device not per interrupt */
+   bool is_msix = dev_is_pci(dev) ? to_pci_dev(dev)->msix_enabled : false;
 
return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi");
 }

[patch V3 02/35] x86/pci/XEN: Use PCI device property

[Thomas Gleixner]

From: Thomas Gleixner 

instead of fiddling with MSI descriptors.

Signed-off-by: Thomas Gleixner 
Cc: Juergen Gross 
Cc: xen-devel@lists.xenproject.org
---
V3: Use pci_dev->msix_enabled.
---
 arch/x86/pci/xen.c |9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -399,9 +399,7 @@ static void xen_teardown_msi_irqs(struct
 
 static void xen_pv_teardown_msi_irqs(struct pci_dev *dev)
 {
-   struct msi_desc *msidesc = first_pci_msi_entry(dev);
-
-   if (msidesc->pci.msi_attrib.is_msix)
+   if (dev->msix_enabled)
xen_pci_frontend_disable_msix(dev);
else
xen_pci_frontend_disable_msi(dev);
@@ -417,10 +415,7 @@ static int xen_msi_domain_alloc_irqs(str
if (WARN_ON_ONCE(!dev_is_pci(dev)))
return -EINVAL;
 
-   if (first_msi_entry(dev)->pci.msi_attrib.is_msix)
-   type = PCI_CAP_ID_MSIX;
-   else
-   type = PCI_CAP_ID_MSI;
+   type = to_pci_dev(dev)->msix_enabled ? PCI_CAP_ID_MSIX : PCI_CAP_ID_MSI;
 
return xen_msi_ops.setup_msi_irqs(to_pci_dev(dev), nvec, type);
 }

[patch V3 03/35] x86/apic/msi: Use PCI device MSI property

[Thomas Gleixner]

From: Thomas Gleixner 

instead of fiddling with MSI descriptors.

Signed-off-by: Thomas Gleixner 
---
V3: Use pci_dev->msix_enabled - Jason
---
 arch/x86/kernel/apic/msi.c |5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -160,11 +160,8 @@ static struct irq_chip pci_msi_controlle
 int pci_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
msi_alloc_info_t *arg)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
-   struct msi_desc *desc = first_pci_msi_entry(pdev);
-
init_irq_alloc_info(arg, NULL);
-   if (desc->pci.msi_attrib.is_msix) {
+   if (to_pci_dev(dev)->msix_enabled) {
arg->type = X86_IRQ_ALLOC_TYPE_PCI_MSIX;
} else {
arg->type = X86_IRQ_ALLOC_TYPE_PCI_MSI;

[patch V3 01/35] PCI/MSI: Set pci_dev::msi[x]_enabled early

[Thomas Gleixner]

There are quite some places which retrieve the first MSI descriptor to
evaluate whether the setup is for MSI or MSI-X. That's required because
pci_dev::msi[x]_enabled is only set when the setup completed successfully.

There is no real reason why msi[x]_enabled can't be set at the beginning of
the setup sequence and cleared in case of a failure.

Implement that so the MSI descriptor evaluations can be converted to simple
property queries.

Signed-off-by: Thomas Gleixner 
---
V3: New patch
---
 drivers/pci/msi/msi.c |   23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -421,11 +421,18 @@ static int msi_capability_init(struct pc
struct msi_desc *entry;
int ret;
 
-   pci_msi_set_enable(dev, 0); /* Disable MSI during set up */
+   /*
+* Disable MSI during setup in the hardware, but mark it enabled
+* so that setup code can evaluate it.
+*/
+   pci_msi_set_enable(dev, 0);
+   dev->msi_enabled = 1;
 
entry = msi_setup_entry(dev, nvec, affd);
-   if (!entry)
-   return -ENOMEM;
+   if (!entry) {
+   ret = -ENOMEM;
+   goto fail;
+   }
 
/* All MSIs are unmasked by default; mask them all */
pci_msi_mask(entry, msi_multi_mask(entry));
@@ -452,7 +459,6 @@ static int msi_capability_init(struct pc
/* Set MSI enabled bits */
pci_intx_for_msi(dev, 0);
pci_msi_set_enable(dev, 1);
-   dev->msi_enabled = 1;
 
pcibios_free_irq(dev);
dev->irq = entry->irq;
@@ -461,6 +467,8 @@ static int msi_capability_init(struct pc
 err:
pci_msi_unmask(entry, msi_multi_mask(entry));
free_msi_irqs(dev);
+fail:
+   dev->msi_enabled = 0;
return ret;
 }
 
@@ -589,6 +597,9 @@ static int msix_capability_init(struct p
pci_msix_clear_and_set_ctrl(dev, 0, PCI_MSIX_FLAGS_MASKALL |
PCI_MSIX_FLAGS_ENABLE);
 
+   /* Mark it enabled so setup functions can query it */
+   dev->msix_enabled = 1;
+
pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, );
/* Request & Map MSI-X table region */
tsize = msix_table_size(control);
@@ -626,9 +637,8 @@ static int msix_capability_init(struct p
 
dev->msi_irq_groups = groups;
 
-   /* Set MSI-X enabled bits and unmask the function */
+   /* Disable INTX and unmask MSI-X */
pci_intx_for_msi(dev, 0);
-   dev->msix_enabled = 1;
pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
pcibios_free_irq(dev);
@@ -638,6 +648,7 @@ static int msix_capability_init(struct p
free_msi_irqs(dev);
 
 out_disable:
+   dev->msix_enabled = 0;
pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 
return ret;

[patch V3 00/35] genirq/msi, PCI/MSI: Spring cleaning - Part 2

[Thomas Gleixner]

This is the second part of [PCI]MSI refactoring which aims to provide the
ability of expanding MSI-X vectors after enabling MSI-X.

This is based on the first part of this work which can be found here:

https://lore.kernel.org/r/20211206210147.872865...@linutronix.de

and has been applied to:

 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/msi


This second part has the following important changes:

   1) Cleanup of the MSI related data in struct device

  struct device contains at the moment various MSI related parts. Some
  of them (the irq domain pointer) cannot be moved out, but the rest
  can be allocated on first use. This is in preparation of adding more
  per device MSI data later on.

   2) Consolidation of sysfs handling

  As a first step this moves the sysfs pointer from struct msi_desc
  into the new per device MSI data structure where it belongs.

  Later changes will cleanup this code further, but that's not possible
  at this point.

   3) Use PCI device properties instead of looking up MSI descriptors and
  analysing their data.

   4) Provide a function to retrieve the Linux interrupt number for a given
  MSI index similar to pci_irq_vector() and cleanup all open coded
  variants.

It's also available from git:

 git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git msi-v3-part-2

Part 3 of this effort is available on top

 git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git msi-v3-part-3

 Part 3 is not going to be reposted as there is no change vs. V2.

V2 of part 2 can be found here:

https://lore.kernel.org/r/20211206210307.625116...@linutronix.de

Changes versus V2:

  - Use PCI device properties instead of creating a new set - Jason

  - Picked up Reviewed/Tested/Acked-by tags as appropriate

Thanks,

tglx
---
 arch/powerpc/platforms/cell/axon_msi.c  |5 
 arch/powerpc/platforms/pseries/msi.c|   38 +---
 arch/x86/kernel/apic/msi.c  |5 
 arch/x86/pci/xen.c  |   11 -
 drivers/base/platform-msi.c |  152 ---
 drivers/bus/fsl-mc/dprc-driver.c|8 -
 drivers/bus/fsl-mc/fsl-mc-allocator.c   |9 -
 drivers/bus/fsl-mc/fsl-mc-msi.c |   26 +--
 drivers/dma/mv_xor_v2.c |   16 --
 drivers/dma/qcom/hidma.c|   44 ++---
 drivers/dma/ti/k3-udma-private.c|6 
 drivers/dma/ti/k3-udma.c|   14 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   23 --
 drivers/irqchip/irq-mbigen.c|4 
 drivers/irqchip/irq-mvebu-icu.c |   12 -
 drivers/irqchip/irq-ti-sci-inta.c   |2 
 drivers/mailbox/bcm-flexrm-mailbox.c|9 -
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c|4 
 drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c|4 
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c |5 
 drivers/pci/msi/irqdomain.c |   20 ++
 drivers/pci/msi/legacy.c|6 
 drivers/pci/msi/msi.c   |  133 ++--
 drivers/pci/xen-pcifront.c  |2 
 drivers/perf/arm_smmuv3_pmu.c   |5 
 drivers/soc/fsl/dpio/dpio-driver.c  |8 -
 drivers/soc/ti/k3-ringacc.c |6 
 drivers/soc/ti/ti_sci_inta_msi.c|   22 --
 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c  |4 
 include/linux/device.h  |   25 ++-
 include/linux/fsl/mc.h  |4 
 include/linux/msi.h |   95 
 include/linux/pci.h |1 
 include/linux/soc/ti/ti_sci_inta_msi.h  |1 
 kernel/irq/msi.c|  158 +++-
 35 files changed, 429 insertions(+), 458 deletions(-)

[ovmf test] 167352: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf e81a81e5846edcc4c2e91cf3a39d0ba8c31b687a
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   38 attempts
Testing same since   167352  2021-12-10 20:11:48 Z0 days1 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Chris Jones 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1185 lines long.)

Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

[Dmitry Osipenko]

10.12.2021 22:33, Dmitry Osipenko пишет:
>> Not really, they only prevent the race from occurring while
>> notifier_has_unique_priority() is running.
>>
>> If anyone depends on this check for correctness, they need to lock the
>> rwsem, do the check, do the thing depending on the check while holding
>> the rwsem and then release the rwsem.  Otherwise it is racy.
>>
> It's fine that it's a bit "racy" since in the context of this series. We
> always do the check after adding new entry, so it's not a problem.
> 
> There are two options:
> 
> 1. Use blocking_notifier_has_unique_priority() like it's done in this
> patchset. Remove it after all drivers are converted to the new API and
> add blocking_notifier_chain_register_unique().
> 
> 2. Add blocking_notifier_chain_register_unique(), but don't let it fail
> the registration of non-unique entries until all drivers are converted
> to the new API.

There is third, perhaps the best option:

3. Add blocking_notifier_chain_register_unique() and fall back to
blocking_notifier_chain_register() if unique fails, do it until all
drivers are converted to the new API.

[ovmf test] 167350: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   37 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days   11 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Dmitry Osipenko]

10.12.2021 22:44, Dmitry Osipenko пишет:
> 10.12.2021 22:42, Dmitry Osipenko пишет:
> ...
 There is no strong requirement for priorities to be unique, the reboot.c
 code will work properly.
>>>
>>> In which case adding the WARN() is not appropriate IMV.
>>>
>>> Also I've looked at the existing code and at least in some cases the
>>> order in which the notifiers run doesn't matter.  I'm not sure what
>>> the purpose of this patch is TBH.
>>
>> The purpose is to let developer know that driver needs to be corrected.
>>
 The potential problem is on the user's side and the warning is intended
 to aid the user.
>>>
>>> Unless somebody has the panic_on_warn mentioned previously set and
>>> really the user need not understand what the WARN() is about.  IOW,
>>> WARN() helps developers, not users.
>>>
 We can make it a strong requirement, but only after converting and
 testing all kernel drivers.
>>>
>>> Right.
>>>
 I'll consider to add patches for that.
>>>
>>> But can you avoid adding more patches to this series?
>>
>> I won't add more patches since such patches can be added only after
>> completion of transition to the new API of the whole kernel.
>>
> 
> Thank you for the review.
> 

I meant you, Rafael, and Michał, just in case :)

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Dmitry Osipenko]

10.12.2021 22:42, Dmitry Osipenko пишет:
...
>>> There is no strong requirement for priorities to be unique, the reboot.c
>>> code will work properly.
>>
>> In which case adding the WARN() is not appropriate IMV.
>>
>> Also I've looked at the existing code and at least in some cases the
>> order in which the notifiers run doesn't matter.  I'm not sure what
>> the purpose of this patch is TBH.
> 
> The purpose is to let developer know that driver needs to be corrected.
> 
>>> The potential problem is on the user's side and the warning is intended
>>> to aid the user.
>>
>> Unless somebody has the panic_on_warn mentioned previously set and
>> really the user need not understand what the WARN() is about.  IOW,
>> WARN() helps developers, not users.
>>
>>> We can make it a strong requirement, but only after converting and
>>> testing all kernel drivers.
>>
>> Right.
>>
>>> I'll consider to add patches for that.
>>
>> But can you avoid adding more patches to this series?
> 
> I won't add more patches since such patches can be added only after
> completion of transition to the new API of the whole kernel.
> 

Thank you for the review.

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Dmitry Osipenko]

10.12.2021 22:14, Rafael J. Wysocki пишет:
> On Fri, Dec 10, 2021 at 8:04 PM Dmitry Osipenko  wrote:
>>
>> 10.12.2021 21:27, Rafael J. Wysocki пишет:
>>> On Mon, Nov 29, 2021 at 12:34 PM Dmitry Osipenko  wrote:

 29.11.2021 03:26, Michał Mirosław пишет:
> On Mon, Nov 29, 2021 at 12:06:19AM +0300, Dmitry Osipenko wrote:
>> 28.11.2021 03:28, Michał Mirosław пишет:
>>> On Fri, Nov 26, 2021 at 09:00:41PM +0300, Dmitry Osipenko wrote:
 Add sanity check which ensures that there are no two restart handlers
 registered with the same priority. Normally it's a direct sign of a
 problem if two handlers use the same priority.
>>>
>>> The patch doesn't ensure the property that there are no 
>>> duplicated-priority
>>> entries on the chain.
>>
>> It's not the exact point of this patch.
>>
>>> I'd rather see a atomic_notifier_chain_register_unique() that returns
>>> -EBUSY or something istead of adding an entry with duplicate priority.
>>> That way it would need only one list traversal unless you want to
>>> register the duplicate anyway (then you would call the older
>>> atomic_notifier_chain_register() after reporting the error).
>>
>> The point of this patch is to warn developers about the problem that
>> needs to be fixed. We already have such troubling drivers in mainline.
>>
>> It's not critical to register different handlers with a duplicated
>> priorities, but such cases really need to be corrected. We shouldn't
>> break users' machines during transition to the new API, meanwhile
>> developers should take action of fixing theirs drivers.
>>
>>> (Or you could return > 0 when a duplicate is registered in
>>> atomic_notifier_chain_register() if the callers are prepared
>>> for that. I don't really like this way, though.)
>>
>> I had a similar thought at some point before and decided that I'm not in
>> favor of this approach. It's nicer to have a dedicated function that
>> verifies the uniqueness, IMO.
>
> I don't like the part that it traverses the list second time to check
> the uniqueness. But actually you could avoid that if
> notifier_chain_register() would always add equal-priority entries in
> reverse order:
>
>  static int notifier_chain_register(struct notifier_block **nl,
>   struct notifier_block *n)
>  {
>   while ((*nl) != NULL) {
>   if (unlikely((*nl) == n)) {
>   WARN(1, "double register detected");
>   return 0;
>   }
> - if (n->priority > (*nl)->priority)
> + if (n->priority >= (*nl)->priority)
>   break;
>   nl = &((*nl)->next);
>   }
>   n->next = *nl;
>   rcu_assign_pointer(*nl, n);
>   return 0;
>  }
>
> Then the check for uniqueness after adding would be:
>
>  WARN(nb->next && nb->priority == nb->next->priority);

 We can't just change the registration order because invocation order of
 the call chain depends on the registration order
>>>
>>> It doesn't if unique priorities are required and isn't that what you want?
>>>
 and some of current
 users may rely on that order. I'm pretty sure that changing the order
 will have unfortunate consequences.
>>>
>>> Well, the WARN() doesn't help much then.
>>>
>>> Either you can make all of the users register with unique priorities,
>>> and then you can make the registration reject non-unique ones, or you
>>> cannot assume them to be unique.
>>
>> There is no strong requirement for priorities to be unique, the reboot.c
>> code will work properly.
> 
> In which case adding the WARN() is not appropriate IMV.
> 
> Also I've looked at the existing code and at least in some cases the
> order in which the notifiers run doesn't matter.  I'm not sure what
> the purpose of this patch is TBH.

The purpose is to let developer know that driver needs to be corrected.

>> The potential problem is on the user's side and the warning is intended
>> to aid the user.
> 
> Unless somebody has the panic_on_warn mentioned previously set and
> really the user need not understand what the WARN() is about.  IOW,
> WARN() helps developers, not users.
> 
>> We can make it a strong requirement, but only after converting and
>> testing all kernel drivers.
> 
> Right.
> 
>> I'll consider to add patches for that.
> 
> But can you avoid adding more patches to this series?

I won't add more patches since such patches can be added only after
completion of transition to the new API of the whole kernel.

Re: [PATCH v4 06/25] reboot: Warn if unregister_restart_handler() fails

[Dmitry Osipenko]

10.12.2021 22:08, Rafael J. Wysocki пишет:
> On Fri, Dec 10, 2021 at 7:54 PM Dmitry Osipenko  wrote:
>>
>> 10.12.2021 21:32, Rafael J. Wysocki пишет:
>>> On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:

 Emit warning if unregister_restart_handler() fails since it never should
 fail. This will ease further API development by catching mistakes early.

 Signed-off-by: Dmitry Osipenko 
 ---
  kernel/reboot.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/kernel/reboot.c b/kernel/reboot.c
 index e6659ae329f1..f0e7b9c13f6b 100644
 --- a/kernel/reboot.c
 +++ b/kernel/reboot.c
 @@ -210,7 +210,7 @@ EXPORT_SYMBOL(register_restart_handler);
   */
  int unregister_restart_handler(struct notifier_block *nb)
  {
 -   return atomic_notifier_chain_unregister(_handler_list, nb);
 +   return 
 WARN_ON(atomic_notifier_chain_unregister(_handler_list, nb));
>>>
>>> The only reason why it can fail is if the object pointed to by nb is
>>> not in the chain.
>>
>> I had exactly this case where object wasn't in the chain due to a bug
>> and this warning was very helpful.
> 
> During the development.  In production it would be rather annoying.
> 
>>>  Why WARN() about this?  And what about systems with
>>> panic_on_warn set?
>>
>> That warning condition will never happen normally, only when something
>> is seriously wrong.
>>
>> Those systems with panic_on_warn will get what was they asked for.
> 
> They may not be asking for panicking on bugs in the reboot notifier
> code, though.  That's what your change is making them panic on.
> 

Alright, I'll drop the warnings and turn the warning about uniqueness
into error or warning message.

[PATCH] xen-hvm: Allow disabling buffer_io_timer

[Jason Andryuk]

commit f37f29d31488 "xen: slightly simplify bufioreq handling" hard
coded setting req.count = 1 during initial field setup before the main
loop.  This missed a subtlety that an early exit from the loop when
there are no ioreqs to process, would have req.count == 0 for the return
value.  handle_buffered_io() would then remove state->buffered_io_timer.
Instead handle_buffered_iopage() is basically always returning true and
handle_buffered_io() always re-setting the timer.

Restore the disabling of the timer by introducing a new handled_ioreq
boolean and use as the return value.  The named variable will more
clearly show the intent of the code.

Signed-off-by: Jason Andryuk 
---
 hw/i386/xen/xen-hvm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 482be95415..cf8e500514 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1087,10 +1087,11 @@ static void handle_ioreq(XenIOState *state, ioreq_t 
*req)
 }
 }
 
-static int handle_buffered_iopage(XenIOState *state)
+static bool handle_buffered_iopage(XenIOState *state)
 {
 buffered_iopage_t *buf_page = state->buffered_io_page;
 buf_ioreq_t *buf_req = NULL;
+bool handled_ioreq = false;
 ioreq_t req;
 int qw;
 
@@ -1144,9 +1145,10 @@ static int handle_buffered_iopage(XenIOState *state)
 assert(!req.data_is_ptr);
 
 qatomic_add(_page->read_pointer, qw + 1);
+handled_ioreq = true;
 }
 
-return req.count;
+return handled_ioreq;
 }
 
 static void handle_buffered_io(void *opaque)
-- 
2.33.1

Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

[Dmitry Osipenko]

10.12.2021 22:05, Rafael J. Wysocki пишет:
> On Fri, Dec 10, 2021 at 7:52 PM Dmitry Osipenko  wrote:
>>
>> 10.12.2021 21:19, Rafael J. Wysocki пишет:
>> ...
 +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
 +   struct notifier_block *n)
 +{
 +   unsigned long flags;
 +   bool ret;
 +
 +   spin_lock_irqsave(>lock, flags);
 +   ret = notifier_has_unique_priority(>head, n);
 +   spin_unlock_irqrestore(>lock, flags);
>>>
>>> This only works if the caller can prevent new entries from being added
>>> to the list at this point or if the caller knows that they cannot be
>>> added for some reason, but the kerneldoc doesn't mention this
>>> limitation.
>>
>> I'll update the comment.
>>
>> ..
 +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head 
 *nh,
 +   struct notifier_block *n)
 +{
 +   bool ret;
 +
 +   /*
 +* This code gets used during boot-up, when task switching is
 +* not yet working and interrupts must remain disabled. At such
 +* times we must not call down_read().
 +*/
 +   if (system_state != SYSTEM_BOOTING)
>>>
>>> No, please don't do this, it makes the whole thing error-prone.
>>
>> What should I do then?
> 
> First of all, do you know of any users who may want to call this
> during early initialization?  If so, then why may they want to do
> that?

I'll need to carefully review all those dozens of platform restart
handlers to answer this question.

> Depending on the above, I would consider adding a special mechanism for them.

Please notice that every blocking_notifier_*() function has this
SYSTEM_BOOTING check, it's not my invention. Notifier API needs to be
generic.

 +   down_read(>rwsem);
 +
 +   ret = notifier_has_unique_priority(>head, n);
 +
 +   if (system_state != SYSTEM_BOOTING)
 +   up_read(>rwsem);
>>>
>>> And still what if a new entry with a non-unique priority is added to
>>> the chain at this point?
>>
>> If entry with a non-unique priority is added after the check, then
>> obviously it won't be detected.
> 
> Why isn't this a problem?>> I don't understand the question. These
>> down/up_read() are the locks that prevent the race, if that's the question.
> 
> Not really, they only prevent the race from occurring while
> notifier_has_unique_priority() is running.
> 
> If anyone depends on this check for correctness, they need to lock the
> rwsem, do the check, do the thing depending on the check while holding
> the rwsem and then release the rwsem.  Otherwise it is racy.
> 

It's fine that it's a bit "racy" since in the context of this series. We
always do the check after adding new entry, so it's not a problem.

There are two options:

1. Use blocking_notifier_has_unique_priority() like it's done in this
patchset. Remove it after all drivers are converted to the new API and
add blocking_notifier_chain_register_unique().

2. Add blocking_notifier_chain_register_unique(), but don't let it fail
the registration of non-unique entries until all drivers are converted
to the new API.

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Rafael J. Wysocki]

On Fri, Dec 10, 2021 at 8:04 PM Dmitry Osipenko  wrote:
>
> 10.12.2021 21:27, Rafael J. Wysocki пишет:
> > On Mon, Nov 29, 2021 at 12:34 PM Dmitry Osipenko  wrote:
> >>
> >> 29.11.2021 03:26, Michał Mirosław пишет:
> >>> On Mon, Nov 29, 2021 at 12:06:19AM +0300, Dmitry Osipenko wrote:
>  28.11.2021 03:28, Michał Mirosław пишет:
> > On Fri, Nov 26, 2021 at 09:00:41PM +0300, Dmitry Osipenko wrote:
> >> Add sanity check which ensures that there are no two restart handlers
> >> registered with the same priority. Normally it's a direct sign of a
> >> problem if two handlers use the same priority.
> >
> > The patch doesn't ensure the property that there are no 
> > duplicated-priority
> > entries on the chain.
> 
>  It's not the exact point of this patch.
> 
> > I'd rather see a atomic_notifier_chain_register_unique() that returns
> > -EBUSY or something istead of adding an entry with duplicate priority.
> > That way it would need only one list traversal unless you want to
> > register the duplicate anyway (then you would call the older
> > atomic_notifier_chain_register() after reporting the error).
> 
>  The point of this patch is to warn developers about the problem that
>  needs to be fixed. We already have such troubling drivers in mainline.
> 
>  It's not critical to register different handlers with a duplicated
>  priorities, but such cases really need to be corrected. We shouldn't
>  break users' machines during transition to the new API, meanwhile
>  developers should take action of fixing theirs drivers.
> 
> > (Or you could return > 0 when a duplicate is registered in
> > atomic_notifier_chain_register() if the callers are prepared
> > for that. I don't really like this way, though.)
> 
>  I had a similar thought at some point before and decided that I'm not in
>  favor of this approach. It's nicer to have a dedicated function that
>  verifies the uniqueness, IMO.
> >>>
> >>> I don't like the part that it traverses the list second time to check
> >>> the uniqueness. But actually you could avoid that if
> >>> notifier_chain_register() would always add equal-priority entries in
> >>> reverse order:
> >>>
> >>>  static int notifier_chain_register(struct notifier_block **nl,
> >>>   struct notifier_block *n)
> >>>  {
> >>>   while ((*nl) != NULL) {
> >>>   if (unlikely((*nl) == n)) {
> >>>   WARN(1, "double register detected");
> >>>   return 0;
> >>>   }
> >>> - if (n->priority > (*nl)->priority)
> >>> + if (n->priority >= (*nl)->priority)
> >>>   break;
> >>>   nl = &((*nl)->next);
> >>>   }
> >>>   n->next = *nl;
> >>>   rcu_assign_pointer(*nl, n);
> >>>   return 0;
> >>>  }
> >>>
> >>> Then the check for uniqueness after adding would be:
> >>>
> >>>  WARN(nb->next && nb->priority == nb->next->priority);
> >>
> >> We can't just change the registration order because invocation order of
> >> the call chain depends on the registration order
> >
> > It doesn't if unique priorities are required and isn't that what you want?
> >
> >> and some of current
> >> users may rely on that order. I'm pretty sure that changing the order
> >> will have unfortunate consequences.
> >
> > Well, the WARN() doesn't help much then.
> >
> > Either you can make all of the users register with unique priorities,
> > and then you can make the registration reject non-unique ones, or you
> > cannot assume them to be unique.
>
> There is no strong requirement for priorities to be unique, the reboot.c
> code will work properly.

In which case adding the WARN() is not appropriate IMV.

Also I've looked at the existing code and at least in some cases the
order in which the notifiers run doesn't matter.  I'm not sure what
the purpose of this patch is TBH.

> The potential problem is on the user's side and the warning is intended
> to aid the user.

Unless somebody has the panic_on_warn mentioned previously set and
really the user need not understand what the WARN() is about.  IOW,
WARN() helps developers, not users.

> We can make it a strong requirement, but only after converting and
> testing all kernel drivers.

Right.

> I'll consider to add patches for that.

But can you avoid adding more patches to this series?

Re: [PATCH v4 06/25] reboot: Warn if unregister_restart_handler() fails

[Rafael J. Wysocki]

On Fri, Dec 10, 2021 at 7:54 PM Dmitry Osipenko  wrote:
>
> 10.12.2021 21:32, Rafael J. Wysocki пишет:
> > On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
> >>
> >> Emit warning if unregister_restart_handler() fails since it never should
> >> fail. This will ease further API development by catching mistakes early.
> >>
> >> Signed-off-by: Dmitry Osipenko 
> >> ---
> >>  kernel/reboot.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/reboot.c b/kernel/reboot.c
> >> index e6659ae329f1..f0e7b9c13f6b 100644
> >> --- a/kernel/reboot.c
> >> +++ b/kernel/reboot.c
> >> @@ -210,7 +210,7 @@ EXPORT_SYMBOL(register_restart_handler);
> >>   */
> >>  int unregister_restart_handler(struct notifier_block *nb)
> >>  {
> >> -   return atomic_notifier_chain_unregister(_handler_list, nb);
> >> +   return 
> >> WARN_ON(atomic_notifier_chain_unregister(_handler_list, nb));
> >
> > The only reason why it can fail is if the object pointed to by nb is
> > not in the chain.
>
> I had exactly this case where object wasn't in the chain due to a bug
> and this warning was very helpful.

During the development.  In production it would be rather annoying.

> >  Why WARN() about this?  And what about systems with
> > panic_on_warn set?
>
> That warning condition will never happen normally, only when something
> is seriously wrong.
>
> Those systems with panic_on_warn will get what was they asked for.

They may not be asking for panicking on bugs in the reboot notifier
code, though.  That's what your change is making them panic on.

Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

[Rafael J. Wysocki]

On Fri, Dec 10, 2021 at 7:52 PM Dmitry Osipenko  wrote:
>
> 10.12.2021 21:19, Rafael J. Wysocki пишет:
> ...
> >> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
> >> +   struct notifier_block *n)
> >> +{
> >> +   unsigned long flags;
> >> +   bool ret;
> >> +
> >> +   spin_lock_irqsave(>lock, flags);
> >> +   ret = notifier_has_unique_priority(>head, n);
> >> +   spin_unlock_irqrestore(>lock, flags);
> >
> > This only works if the caller can prevent new entries from being added
> > to the list at this point or if the caller knows that they cannot be
> > added for some reason, but the kerneldoc doesn't mention this
> > limitation.
>
> I'll update the comment.
>
> ..
> >> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head 
> >> *nh,
> >> +   struct notifier_block *n)
> >> +{
> >> +   bool ret;
> >> +
> >> +   /*
> >> +* This code gets used during boot-up, when task switching is
> >> +* not yet working and interrupts must remain disabled. At such
> >> +* times we must not call down_read().
> >> +*/
> >> +   if (system_state != SYSTEM_BOOTING)
> >
> > No, please don't do this, it makes the whole thing error-prone.
>
> What should I do then?

First of all, do you know of any users who may want to call this
during early initialization?  If so, then why may they want to do
that?

Depending on the above, I would consider adding a special mechanism for them.

> >> +   down_read(>rwsem);
> >> +
> >> +   ret = notifier_has_unique_priority(>head, n);
> >> +
> >> +   if (system_state != SYSTEM_BOOTING)
> >> +   up_read(>rwsem);
> >
> > And still what if a new entry with a non-unique priority is added to
> > the chain at this point?
>
> If entry with a non-unique priority is added after the check, then
> obviously it won't be detected.

Why isn't this a problem?

> I don't understand the question. These
> down/up_read() are the locks that prevent the race, if that's the question.

Not really, they only prevent the race from occurring while
notifier_has_unique_priority() is running.

If anyone depends on this check for correctness, they need to lock the
rwsem, do the check, do the thing depending on the check while holding
the rwsem and then release the rwsem.  Otherwise it is racy.

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Dmitry Osipenko]

10.12.2021 21:27, Rafael J. Wysocki пишет:
> On Mon, Nov 29, 2021 at 12:34 PM Dmitry Osipenko  wrote:
>>
>> 29.11.2021 03:26, Michał Mirosław пишет:
>>> On Mon, Nov 29, 2021 at 12:06:19AM +0300, Dmitry Osipenko wrote:
 28.11.2021 03:28, Michał Mirosław пишет:
> On Fri, Nov 26, 2021 at 09:00:41PM +0300, Dmitry Osipenko wrote:
>> Add sanity check which ensures that there are no two restart handlers
>> registered with the same priority. Normally it's a direct sign of a
>> problem if two handlers use the same priority.
>
> The patch doesn't ensure the property that there are no 
> duplicated-priority
> entries on the chain.

 It's not the exact point of this patch.

> I'd rather see a atomic_notifier_chain_register_unique() that returns
> -EBUSY or something istead of adding an entry with duplicate priority.
> That way it would need only one list traversal unless you want to
> register the duplicate anyway (then you would call the older
> atomic_notifier_chain_register() after reporting the error).

 The point of this patch is to warn developers about the problem that
 needs to be fixed. We already have such troubling drivers in mainline.

 It's not critical to register different handlers with a duplicated
 priorities, but such cases really need to be corrected. We shouldn't
 break users' machines during transition to the new API, meanwhile
 developers should take action of fixing theirs drivers.

> (Or you could return > 0 when a duplicate is registered in
> atomic_notifier_chain_register() if the callers are prepared
> for that. I don't really like this way, though.)

 I had a similar thought at some point before and decided that I'm not in
 favor of this approach. It's nicer to have a dedicated function that
 verifies the uniqueness, IMO.
>>>
>>> I don't like the part that it traverses the list second time to check
>>> the uniqueness. But actually you could avoid that if
>>> notifier_chain_register() would always add equal-priority entries in
>>> reverse order:
>>>
>>>  static int notifier_chain_register(struct notifier_block **nl,
>>>   struct notifier_block *n)
>>>  {
>>>   while ((*nl) != NULL) {
>>>   if (unlikely((*nl) == n)) {
>>>   WARN(1, "double register detected");
>>>   return 0;
>>>   }
>>> - if (n->priority > (*nl)->priority)
>>> + if (n->priority >= (*nl)->priority)
>>>   break;
>>>   nl = &((*nl)->next);
>>>   }
>>>   n->next = *nl;
>>>   rcu_assign_pointer(*nl, n);
>>>   return 0;
>>>  }
>>>
>>> Then the check for uniqueness after adding would be:
>>>
>>>  WARN(nb->next && nb->priority == nb->next->priority);
>>
>> We can't just change the registration order because invocation order of
>> the call chain depends on the registration order
> 
> It doesn't if unique priorities are required and isn't that what you want?
> 
>> and some of current
>> users may rely on that order. I'm pretty sure that changing the order
>> will have unfortunate consequences.
> 
> Well, the WARN() doesn't help much then.
> 
> Either you can make all of the users register with unique priorities,
> and then you can make the registration reject non-unique ones, or you
> cannot assume them to be unique.

There is no strong requirement for priorities to be unique, the reboot.c
code will work properly.

The potential problem is on the user's side and the warning is intended
to aid the user.

We can make it a strong requirement, but only after converting and
testing all kernel drivers. I'll consider to add patches for that.

[ovmf test] 167349: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   36 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days   10 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH v4 07/25] reboot: Remove extern annotation from function prototypes

[Dmitry Osipenko]

10.12.2021 21:35, Rafael J. Wysocki пишет:
> On Fri, Dec 10, 2021 at 7:16 PM Dmitry Osipenko  wrote:
>>
>> 10.12.2021 21:09, Rafael J. Wysocki пишет:
>>> On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:

 There is no need to annotate function prototypes with 'extern', it makes
 code less readable. Remove unnecessary annotations from .

 Signed-off-by: Dmitry Osipenko 
>>>
>>> I'm not sure that this is really useful.
>>>
>>> Personally, I tend to respect the existing conventions like this.
>>>
>>> Surely, this change is not required for the rest of the series to work.
>>
>> Problem that such things start to spread all over the kernel with a
>> copy-paste approach if there is nobody to clean up the code.
>>
>> This is not a common convention and sometimes it's getting corrected [1].
>>
>> [1] https://git.kernel.org/linus/6d7434931
> 
> In separate patches outside of series adding new features, if one is
> so inclined.
> 

Alright, I'll drop this patch then because it can't be done in parallel
without creating the merge conflict. I'll try not to forget to come back
to this later on.

Re: [PATCH v4 06/25] reboot: Warn if unregister_restart_handler() fails

[Dmitry Osipenko]

10.12.2021 21:32, Rafael J. Wysocki пишет:
> On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>>
>> Emit warning if unregister_restart_handler() fails since it never should
>> fail. This will ease further API development by catching mistakes early.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  kernel/reboot.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>> index e6659ae329f1..f0e7b9c13f6b 100644
>> --- a/kernel/reboot.c
>> +++ b/kernel/reboot.c
>> @@ -210,7 +210,7 @@ EXPORT_SYMBOL(register_restart_handler);
>>   */
>>  int unregister_restart_handler(struct notifier_block *nb)
>>  {
>> -   return atomic_notifier_chain_unregister(_handler_list, nb);
>> +   return 
>> WARN_ON(atomic_notifier_chain_unregister(_handler_list, nb));
> 
> The only reason why it can fail is if the object pointed to by nb is
> not in the chain.

I had exactly this case where object wasn't in the chain due to a bug
and this warning was very helpful.

>  Why WARN() about this?  And what about systems with
> panic_on_warn set?

That warning condition will never happen normally, only when something
is seriously wrong.

Those systems with panic_on_warn will get what was they asked for.

Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

[Dmitry Osipenko]

10.12.2021 21:19, Rafael J. Wysocki пишет:
...
>> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
>> +   struct notifier_block *n)
>> +{
>> +   unsigned long flags;
>> +   bool ret;
>> +
>> +   spin_lock_irqsave(>lock, flags);
>> +   ret = notifier_has_unique_priority(>head, n);
>> +   spin_unlock_irqrestore(>lock, flags);
> 
> This only works if the caller can prevent new entries from being added
> to the list at this point or if the caller knows that they cannot be
> added for some reason, but the kerneldoc doesn't mention this
> limitation.

I'll update the comment.

..
>> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head 
>> *nh,
>> +   struct notifier_block *n)
>> +{
>> +   bool ret;
>> +
>> +   /*
>> +* This code gets used during boot-up, when task switching is
>> +* not yet working and interrupts must remain disabled. At such
>> +* times we must not call down_read().
>> +*/
>> +   if (system_state != SYSTEM_BOOTING)
> 
> No, please don't do this, it makes the whole thing error-prone.

What should I do then?

>> +   down_read(>rwsem);
>> +
>> +   ret = notifier_has_unique_priority(>head, n);
>> +
>> +   if (system_state != SYSTEM_BOOTING)
>> +   up_read(>rwsem);
> 
> And still what if a new entry with a non-unique priority is added to
> the chain at this point?

If entry with a non-unique priority is added after the check, then
obviously it won't be detected. I don't understand the question. These
down/up_read() are the locks that prevent the race, if that's the question.

Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain

[Oleksandr Andrushchenko]

Hi, Julien!

On 10.12.21 19:52, Julien Grall wrote:
> Hi Oleksandr,
>
> On 09/12/2021 07:29, Oleksandr Andrushchenko wrote:
>> +unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
>> +{
>> +    if ( !has_vpci(d) )
>> +    return 0;
>> +
>> +    if ( is_hardware_domain(d) )
>> +    {
>> +    int ret = pci_host_iterate_bridges_and_count(d, 
>> vpci_get_num_handlers_cb);
>> +
>> +    return ret < 0 ? 0 : ret;
>
> Sorry I only spotted this now. AFAICT, ret is not meant to return ret < 0 in 
> this case. But if it were then I think it would be wrong to continue as 
> nothing happened because the code will likely fall over/crash when 
> registering the I/O handlers.
>
> I would document this oddity with
>
> if ( ret < 0 )
> {
>    ASSERT_UNREACHABLE();
>    return 0;
> }
>
> I can do the change on commit if the others are happy with it.
Yes, please, do me a favor
>
> Cheers,
>
Thank you,
Oleksandr

Re: [PATCH v4 07/25] reboot: Remove extern annotation from function prototypes

[Rafael J. Wysocki]

On Fri, Dec 10, 2021 at 7:16 PM Dmitry Osipenko  wrote:
>
> 10.12.2021 21:09, Rafael J. Wysocki пишет:
> > On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
> >>
> >> There is no need to annotate function prototypes with 'extern', it makes
> >> code less readable. Remove unnecessary annotations from .
> >>
> >> Signed-off-by: Dmitry Osipenko 
> >
> > I'm not sure that this is really useful.
> >
> > Personally, I tend to respect the existing conventions like this.
> >
> > Surely, this change is not required for the rest of the series to work.
>
> Problem that such things start to spread all over the kernel with a
> copy-paste approach if there is nobody to clean up the code.
>
> This is not a common convention and sometimes it's getting corrected [1].
>
> [1] https://git.kernel.org/linus/6d7434931

In separate patches outside of series adding new features, if one is
so inclined.

Re: [PATCH v4 06/25] reboot: Warn if unregister_restart_handler() fails

[Rafael J. Wysocki]

On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>
> Emit warning if unregister_restart_handler() fails since it never should
> fail. This will ease further API development by catching mistakes early.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  kernel/reboot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index e6659ae329f1..f0e7b9c13f6b 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -210,7 +210,7 @@ EXPORT_SYMBOL(register_restart_handler);
>   */
>  int unregister_restart_handler(struct notifier_block *nb)
>  {
> -   return atomic_notifier_chain_unregister(_handler_list, nb);
> +   return 
> WARN_ON(atomic_notifier_chain_unregister(_handler_list, nb));

The only reason why it can fail is if the object pointed to by nb is
not in the chain.  Why WARN() about this?  And what about systems with
panic_on_warn set?

>  }
>  EXPORT_SYMBOL(unregister_restart_handler);
>
> --
> 2.33.1
>

Re: [PATCH v4 05/25] reboot: Warn if restart handler has duplicated priority

[Rafael J. Wysocki]

On Mon, Nov 29, 2021 at 12:34 PM Dmitry Osipenko  wrote:
>
> 29.11.2021 03:26, Michał Mirosław пишет:
> > On Mon, Nov 29, 2021 at 12:06:19AM +0300, Dmitry Osipenko wrote:
> >> 28.11.2021 03:28, Michał Mirosław пишет:
> >>> On Fri, Nov 26, 2021 at 09:00:41PM +0300, Dmitry Osipenko wrote:
>  Add sanity check which ensures that there are no two restart handlers
>  registered with the same priority. Normally it's a direct sign of a
>  problem if two handlers use the same priority.
> >>>
> >>> The patch doesn't ensure the property that there are no 
> >>> duplicated-priority
> >>> entries on the chain.
> >>
> >> It's not the exact point of this patch.
> >>
> >>> I'd rather see a atomic_notifier_chain_register_unique() that returns
> >>> -EBUSY or something istead of adding an entry with duplicate priority.
> >>> That way it would need only one list traversal unless you want to
> >>> register the duplicate anyway (then you would call the older
> >>> atomic_notifier_chain_register() after reporting the error).
> >>
> >> The point of this patch is to warn developers about the problem that
> >> needs to be fixed. We already have such troubling drivers in mainline.
> >>
> >> It's not critical to register different handlers with a duplicated
> >> priorities, but such cases really need to be corrected. We shouldn't
> >> break users' machines during transition to the new API, meanwhile
> >> developers should take action of fixing theirs drivers.
> >>
> >>> (Or you could return > 0 when a duplicate is registered in
> >>> atomic_notifier_chain_register() if the callers are prepared
> >>> for that. I don't really like this way, though.)
> >>
> >> I had a similar thought at some point before and decided that I'm not in
> >> favor of this approach. It's nicer to have a dedicated function that
> >> verifies the uniqueness, IMO.
> >
> > I don't like the part that it traverses the list second time to check
> > the uniqueness. But actually you could avoid that if
> > notifier_chain_register() would always add equal-priority entries in
> > reverse order:
> >
> >  static int notifier_chain_register(struct notifier_block **nl,
> >   struct notifier_block *n)
> >  {
> >   while ((*nl) != NULL) {
> >   if (unlikely((*nl) == n)) {
> >   WARN(1, "double register detected");
> >   return 0;
> >   }
> > - if (n->priority > (*nl)->priority)
> > + if (n->priority >= (*nl)->priority)
> >   break;
> >   nl = &((*nl)->next);
> >   }
> >   n->next = *nl;
> >   rcu_assign_pointer(*nl, n);
> >   return 0;
> >  }
> >
> > Then the check for uniqueness after adding would be:
> >
> >  WARN(nb->next && nb->priority == nb->next->priority);
>
> We can't just change the registration order because invocation order of
> the call chain depends on the registration order

It doesn't if unique priorities are required and isn't that what you want?

> and some of current
> users may rely on that order. I'm pretty sure that changing the order
> will have unfortunate consequences.

Well, the WARN() doesn't help much then.

Either you can make all of the users register with unique priorities,
and then you can make the registration reject non-unique ones, or you
cannot assume them to be unique.

Re: [PATCH v4 04/25] reboot: Correct typo in a comment

[Rafael J. Wysocki]

On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>
> Correct s/implemenations/implementations/ in .
>
> Signed-off-by: Dmitry Osipenko 

This patch clearly need not be part of this series.

> ---
>  include/linux/reboot.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index af907a3d68d1..7c288013a3ca 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -63,7 +63,7 @@ struct pt_regs;
>  extern void machine_crash_shutdown(struct pt_regs *);
>
>  /*
> - * Architecture independent implemenations of sys_reboot commands.
> + * Architecture independent implementations of sys_reboot commands.
>   */
>
>  extern void kernel_restart_prepare(char *cmd);
> --
> 2.33.1
>

Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

[Rafael J. Wysocki]

On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>
> Add atomic/blocking_notifier_has_unique_priority() helpers which return
> true if given handler has unique priority.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  include/linux/notifier.h |  5 +++
>  kernel/notifier.c| 69 
>  2 files changed, 74 insertions(+)
>
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index 924c9d7c8e73..2c4036f225e1 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -175,6 +175,11 @@ int raw_notifier_call_chain_robust(struct 
> raw_notifier_head *nh,
>
>  bool blocking_notifier_call_chain_is_empty(struct blocking_notifier_head 
> *nh);
>
> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
> +   struct notifier_block *nb);
> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head *nh,
> +   struct notifier_block *nb);
> +
>  #define NOTIFY_DONE0x  /* Don't care */
>  #define NOTIFY_OK  0x0001  /* Suits me */
>  #define NOTIFY_STOP_MASK   0x8000  /* Don't call further */
> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index b20cb7b9b1f0..7a325b742104 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -122,6 +122,19 @@ static int notifier_call_chain_robust(struct 
> notifier_block **nl,
> return ret;
>  }
>
> +static int notifier_has_unique_priority(struct notifier_block **nl,
> +   struct notifier_block *n)
> +{
> +   while (*nl && (*nl)->priority >= n->priority) {
> +   if ((*nl)->priority == n->priority && *nl != n)
> +   return false;
> +
> +   nl = &((*nl)->next);
> +   }
> +
> +   return true;
> +}
> +
>  /*
>   * Atomic notifier chain routines.  Registration and unregistration
>   * use a spinlock, and call_chain is synchronized by RCU (no locks).
> @@ -203,6 +216,30 @@ int atomic_notifier_call_chain(struct 
> atomic_notifier_head *nh,
>  EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
>  NOKPROBE_SYMBOL(atomic_notifier_call_chain);
>
> +/**
> + * atomic_notifier_has_unique_priority - Checks whether notifier's 
> priority is unique
> + * @nh: Pointer to head of the atomic notifier chain
> + * @n: Entry in notifier chain to check
> + *
> + * Checks whether there is another notifier in the chain with the same 
> priority.
> + * Must be called in process context.
> + *
> + * Returns true if priority is unique, false otherwise.
> + */
> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
> +   struct notifier_block *n)
> +{
> +   unsigned long flags;
> +   bool ret;
> +
> +   spin_lock_irqsave(>lock, flags);
> +   ret = notifier_has_unique_priority(>head, n);
> +   spin_unlock_irqrestore(>lock, flags);

This only works if the caller can prevent new entries from being added
to the list at this point or if the caller knows that they cannot be
added for some reason, but the kerneldoc doesn't mention this
limitation.

> +
> +   return ret;
> +}
> +EXPORT_SYMBOL_GPL(atomic_notifier_has_unique_priority);
> +
>  /*
>   * Blocking notifier chain routines.  All access to the chain is
>   * synchronized by an rwsem.
> @@ -336,6 +373,38 @@ bool blocking_notifier_call_chain_is_empty(struct 
> blocking_notifier_head *nh)
>  }
>  EXPORT_SYMBOL_GPL(blocking_notifier_call_chain_is_empty);
>
> +/**
> + * blocking_notifier_has_unique_priority - Checks whether notifier's 
> priority is unique
> + * @nh: Pointer to head of the blocking notifier chain
> + * @n: Entry in notifier chain to check
> + *
> + * Checks whether there is another notifier in the chain with the same 
> priority.
> + * Must be called in process context.
> + *
> + * Returns true if priority is unique, false otherwise.
> + */
> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head *nh,
> +   struct notifier_block *n)
> +{
> +   bool ret;
> +
> +   /*
> +* This code gets used during boot-up, when task switching is
> +* not yet working and interrupts must remain disabled. At such
> +* times we must not call down_read().
> +*/
> +   if (system_state != SYSTEM_BOOTING)

No, please don't do this, it makes the whole thing error-prone.

> +   down_read(>rwsem);
> +
> +   ret = notifier_has_unique_priority(>head, n);
> +
> +   if (system_state != SYSTEM_BOOTING)
> +   up_read(>rwsem);

And still what if a new entry with a non-unique priority is added to
the chain at this point?

> +
> +   return ret;
> +}
> +EXPORT_SYMBOL_GPL(blocking_notifier_has_unique_priority);
> +
>  /*
>   * Raw notifier chain routines.  There is no protection;
>   * the caller must provide it.  Use at your own risk!
> --
> 2.33.1
>

Re: [PATCH v4 02/25] notifier: Add blocking_notifier_call_chain_is_empty()

[Dmitry Osipenko]

10.12.2021 21:14, Rafael J. Wysocki пишет:
> On Fri, Nov 26, 2021 at 7:01 PM Dmitry Osipenko  wrote:
>>
>> Add blocking_notifier_call_chain_is_empty() that returns true if call
>> chain is empty.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  include/linux/notifier.h |  2 ++
>>  kernel/notifier.c| 14 ++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
>> index 4b80a815b666..924c9d7c8e73 100644
>> --- a/include/linux/notifier.h
>> +++ b/include/linux/notifier.h
>> @@ -173,6 +173,8 @@ int blocking_notifier_call_chain_robust(struct 
>> blocking_notifier_head *nh,
>>  int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
>> unsigned long val_up, unsigned long val_down, void *v);
>>
>> +bool blocking_notifier_call_chain_is_empty(struct blocking_notifier_head 
>> *nh);
>> +
>>  #define NOTIFY_DONE0x  /* Don't care */
>>  #define NOTIFY_OK  0x0001  /* Suits me */
>>  #define NOTIFY_STOP_MASK   0x8000  /* Don't call further */
>> diff --git a/kernel/notifier.c b/kernel/notifier.c
>> index b8251dc0bc0f..b20cb7b9b1f0 100644
>> --- a/kernel/notifier.c
>> +++ b/kernel/notifier.c
>> @@ -322,6 +322,20 @@ int blocking_notifier_call_chain(struct 
>> blocking_notifier_head *nh,
>>  }
>>  EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
>>
>> +/**
>> + * blocking_notifier_call_chain_is_empty - Check whether notifier chain 
>> is empty
>> + * @nh: Pointer to head of the blocking notifier chain
>> + *
>> + * Checks whether notifier chain is empty.
>> + *
>> + * Returns true is notifier chain is empty, false otherwise.
>> + */
>> +bool blocking_notifier_call_chain_is_empty(struct blocking_notifier_head 
>> *nh)
>> +{
>> +   return !rcu_access_pointer(nh->head);
>> +}
>> +EXPORT_SYMBOL_GPL(blocking_notifier_call_chain_is_empty);
> 
> The check is not reliable (racy) without locking, so I wouldn't export
> anything like this to modules.
> 
> At least IMO it should be added along with a user.
> 

I'll remove the export since it's indeed not obvious how other users may
want to use this function.

Re: [PATCH v4 07/25] reboot: Remove extern annotation from function prototypes

[Dmitry Osipenko]

10.12.2021 21:09, Rafael J. Wysocki пишет:
> On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>>
>> There is no need to annotate function prototypes with 'extern', it makes
>> code less readable. Remove unnecessary annotations from .
>>
>> Signed-off-by: Dmitry Osipenko 
> 
> I'm not sure that this is really useful.
> 
> Personally, I tend to respect the existing conventions like this.
> 
> Surely, this change is not required for the rest of the series to work.

Problem that such things start to spread all over the kernel with a
copy-paste approach if there is nobody to clean up the code.

This is not a common convention and sometimes it's getting corrected [1].

[1] https://git.kernel.org/linus/6d7434931

Re: [PATCH v4 02/25] notifier: Add blocking_notifier_call_chain_is_empty()

[Rafael J. Wysocki]

On Fri, Nov 26, 2021 at 7:01 PM Dmitry Osipenko  wrote:
>
> Add blocking_notifier_call_chain_is_empty() that returns true if call
> chain is empty.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  include/linux/notifier.h |  2 ++
>  kernel/notifier.c| 14 ++
>  2 files changed, 16 insertions(+)
>
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index 4b80a815b666..924c9d7c8e73 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -173,6 +173,8 @@ int blocking_notifier_call_chain_robust(struct 
> blocking_notifier_head *nh,
>  int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
> unsigned long val_up, unsigned long val_down, void *v);
>
> +bool blocking_notifier_call_chain_is_empty(struct blocking_notifier_head 
> *nh);
> +
>  #define NOTIFY_DONE0x  /* Don't care */
>  #define NOTIFY_OK  0x0001  /* Suits me */
>  #define NOTIFY_STOP_MASK   0x8000  /* Don't call further */
> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index b8251dc0bc0f..b20cb7b9b1f0 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -322,6 +322,20 @@ int blocking_notifier_call_chain(struct 
> blocking_notifier_head *nh,
>  }
>  EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
>
> +/**
> + * blocking_notifier_call_chain_is_empty - Check whether notifier chain 
> is empty
> + * @nh: Pointer to head of the blocking notifier chain
> + *
> + * Checks whether notifier chain is empty.
> + *
> + * Returns true is notifier chain is empty, false otherwise.
> + */
> +bool blocking_notifier_call_chain_is_empty(struct blocking_notifier_head *nh)
> +{
> +   return !rcu_access_pointer(nh->head);
> +}
> +EXPORT_SYMBOL_GPL(blocking_notifier_call_chain_is_empty);

The check is not reliable (racy) without locking, so I wouldn't export
anything like this to modules.

At least IMO it should be added along with a user.

Re: [PATCH v4 07/25] reboot: Remove extern annotation from function prototypes

[Rafael J. Wysocki]

On Fri, Nov 26, 2021 at 7:02 PM Dmitry Osipenko  wrote:
>
> There is no need to annotate function prototypes with 'extern', it makes
> code less readable. Remove unnecessary annotations from .
>
> Signed-off-by: Dmitry Osipenko 

I'm not sure that this is really useful.

Personally, I tend to respect the existing conventions like this.

Surely, this change is not required for the rest of the series to work.

> ---
>  include/linux/reboot.h | 38 +++---
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 7c288013a3ca..b7fa25726323 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -40,36 +40,36 @@ extern int reboot_cpu;
>  extern int reboot_force;
>
>
> -extern int register_reboot_notifier(struct notifier_block *);
> -extern int unregister_reboot_notifier(struct notifier_block *);
> +int register_reboot_notifier(struct notifier_block *);
> +int unregister_reboot_notifier(struct notifier_block *);
>
> -extern int devm_register_reboot_notifier(struct device *, struct 
> notifier_block *);
> +int devm_register_reboot_notifier(struct device *, struct notifier_block *);
>
> -extern int register_restart_handler(struct notifier_block *);
> -extern int unregister_restart_handler(struct notifier_block *);
> -extern void do_kernel_restart(char *cmd);
> +int register_restart_handler(struct notifier_block *);
> +int unregister_restart_handler(struct notifier_block *);
> +void do_kernel_restart(char *cmd);
>
>  /*
>   * Architecture-specific implementations of sys_reboot commands.
>   */
>
> -extern void migrate_to_reboot_cpu(void);
> -extern void machine_restart(char *cmd);
> -extern void machine_halt(void);
> -extern void machine_power_off(void);
> +void migrate_to_reboot_cpu(void);
> +void machine_restart(char *cmd);
> +void machine_halt(void);
> +void machine_power_off(void);
>
> -extern void machine_shutdown(void);
> +void machine_shutdown(void);
>  struct pt_regs;
> -extern void machine_crash_shutdown(struct pt_regs *);
> +void machine_crash_shutdown(struct pt_regs *);
>
>  /*
>   * Architecture independent implementations of sys_reboot commands.
>   */
>
> -extern void kernel_restart_prepare(char *cmd);
> -extern void kernel_restart(char *cmd);
> -extern void kernel_halt(void);
> -extern void kernel_power_off(void);
> +void kernel_restart_prepare(char *cmd);
> +void kernel_restart(char *cmd);
> +void kernel_halt(void);
> +void kernel_power_off(void);
>
>  extern int C_A_D; /* for sysctl */
>  void ctrl_alt_del(void);
> @@ -77,15 +77,15 @@ void ctrl_alt_del(void);
>  #define POWEROFF_CMD_PATH_LEN  256
>  extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
>
> -extern void orderly_poweroff(bool force);
> -extern void orderly_reboot(void);
> +void orderly_poweroff(bool force);
> +void orderly_reboot(void);
>  void hw_protection_shutdown(const char *reason, int ms_until_forced);
>
>  /*
>   * Emergency restart, callable from an interrupt handler.
>   */
>
> -extern void emergency_restart(void);
> +void emergency_restart(void);
>  #include 
>
>  #endif /* _LINUX_REBOOT_H */
> --
> 2.33.1
>

[xen-unstable-smoke test] 167343: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  df3e1a5efe700a9f59eced801cac73f9fd02a0e2
baseline version:
 xen  29e31aaf5d81e57679c2abfe8ffd3851a87042b5

Last test of basis   167337  2021-12-10 10:00:26 Z0 days
Testing same since   167343  2021-12-10 15:01:44 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Julien Grall 
  Paul Durrant 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   29e31aaf5d..df3e1a5efe  df3e1a5efe700a9f59eced801cac73f9fd02a0e2 -> smoke

[ovmf test] 167347: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   35 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days9 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH v5 02/14] vpci: fix function attributes for vpci_process_pending

[Julien Grall]

Hi Oleksandr,

On 25/11/2021 11:02, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

vpci_process_pending is defined with different attributes, e.g.
with __must_check if CONFIG_HAS_VPCI enabled and not otherwise.
Fix this by defining both of the definitions with __must_check.

Fixes: 14583a590783 ("7fbb096bf345 kconfig: don't select VPCI if building a 
shim-only binary")

Signed-off-by: Oleksandr Andrushchenko 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain

[Julien Grall]

Hi Oleksandr,

On 09/12/2021 07:29, Oleksandr Andrushchenko wrote:

+unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
+{
+if ( !has_vpci(d) )
+return 0;
+
+if ( is_hardware_domain(d) )
+{
+int ret = pci_host_iterate_bridges_and_count(d, 
vpci_get_num_handlers_cb);
+
+return ret < 0 ? 0 : ret;


Sorry I only spotted this now. AFAICT, ret is not meant to return ret < 
0 in this case. But if it were then I think it would be wrong to 
continue as nothing happened because the code will likely fall 
over/crash when registering the I/O handlers.


I would document this oddity with

if ( ret < 0 )
{
   ASSERT_UNREACHABLE();
   return 0;
}

I can do the change on commit if the others are happy with it.

Cheers,

--
Julien Grall

Re: [PATCH] arm/efi: Handle Xen bootargs from both xen.cfg and DT

[Julien Grall]

Hi Luca,

On 10/12/2021 10:26, Luca Fancellu wrote:




On 8 Dec 2021, at 18:10, Julien Grall  wrote:

Hi Luca,

On 06/12/2021 15:36, Luca Fancellu wrote:

Currently the Xen UEFI stub can accept Xen boot arguments from
the Xen configuration file using the "options=" keyword, but also
directly from the device tree specifying xen,xen-bootargs
property.
When the configuration file is used, device tree boot arguments
are ignored and overwritten even if the keyword "options=" is
not used.
This patch handle this case, if xen,xen-bootargs is found in the
device tree, it is used for xen boot arguments regardless they
are specified in the Xen configuration file or not.
Signed-off-by: Luca Fancellu 
---
  docs/misc/efi.pandoc| 4 
  xen/arch/arm/efi/efi-boot.h | 7 +++
  2 files changed, 11 insertions(+)
diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
index abafb3452758..b7d99de87f15 100644
--- a/docs/misc/efi.pandoc
+++ b/docs/misc/efi.pandoc
@@ -249,6 +249,10 @@ UEFI stub for module loading.
  When adding DomU modules to device tree, also add the property
  xen,uefi-cfg-load under chosen for Xen to load the Xen config file.
  Otherwise, Xen will skip the config file and rely on device tree alone.
+When using the Xen configuration file in conjunction with the device tree, you
+can specify the Xen boot arguments in the configuration file with the 
"options="
+keyword or in the device tree with the "xen,xen-bootargs" property, but be
+aware that a device tree value has a precedence over the configuration file.


I am not sure I agree with the precedence chosen here. With UEFI environment it 
is a lot easier to update the configuration file over the Device-Tree. So this 
could be used to quickly test a command line before updating the Device-Tree.

Also, somewhat unrelated to this patch. Looking at the code, the command line is a 
concatenation of "options=" from the configuration file and the extra arguments 
provided on the command line used to execute the UEFI binary.

When using the command line from the Device-Tree, I think it would still make 
sense to append the later because it could allow a user to provide a single 
Device-Tree with extra options on the UEFI command line.

But I think this is a separate subject. So if we are going to go with the 
precedence you suggested, then we should at least clarify in the documentation 
that it will replace both.


Yes I see your point, currently the boot arguments are done in this way  
  as you pointed out,

would it be ok in your opinion to check if  is specified and if it’s 
not, use the  instead (if any)?


I am happy with this approach.

Cheers,

--
Julien Grall

[xen-unstable test] 167336: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10  fail blocked in 167312
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167312
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167312
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167312
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167312
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 167312
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 167312
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167312
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167312
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167312
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167312
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167312
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167312
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  

Re: [PATCH v1.1 64/65] x86/efi: Disable CET-IBT around Runtime Services calls

[Andrew Cooper]

On 06/12/2021 11:06, Jan Beulich wrote:
> On 26.11.2021 17:38, Andrew Cooper wrote:
>> --- a/xen/arch/x86/efi/stub.c
>> +++ b/xen/arch/x86/efi/stub.c
>> @@ -11,6 +11,8 @@
>>  #include 
>>  #include 
>>  
>> +bool __initdata efi_no_cet_ibt;
> I'm having trouble seeing what this is needed for - when this file gets
> built, neither boot.c nor runtime.c will get compiled, and hence there
> should not be any reference to the symbol that needs satisfying.
>
>> @@ -735,6 +736,14 @@ static void __init efi_init(EFI_HANDLE ImageHandle, 
>> EFI_SYSTEM_TABLE *SystemTabl
>>  
>>  StdOut = SystemTable->ConOut;
>>  StdErr = SystemTable->StdErr ?: StdOut;
>> +
>> +#ifdef CONFIG_X86
> CONFIG_XEN_IBT?
>
>> +/*
>> + * Heuristic.  Look under an arbitrary function pointer to see if UEFI 
>> was
>> + * compiled with CET-IBT support.  Experimentally some are not.
>> + */
>> +efi_no_cet_ibt = !is_endbr64(efi_rs->GetTime);
> I'm afraid I consider this insufficient. Even if the core EFI was built
> with IBT support, some driver may not have been.

That's not an issue.  Everything is built together in practice.

>  Hence I think there
> needs to be a command line control to force turning off IBT. The only
> question is whether we want to also honor its positive form - that
> would, afaict, be a recipe for a guaranteed crash if used wrongly (and
> it would be meaningless when used on IBT-aware firmware).

It turns out that IBT support is lacking from tianocore, so nothing is
going to support IBT for a good while yet.

https://bugzilla.tianocore.org/show_bug.cgi?id=3726 is the proposed
change to the spec to support this.

In the meantime, I'm just going to blanket disable IBT for RS calls.

~Andrew

[ovmf test] 167346: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   34 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days8 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH 63/65] x86/setup: Rework MSR_S_CET handling for CET-IBT

[Jan Beulich]

On 10.12.2021 17:19, Andrew Cooper wrote:
> On 06/12/2021 10:49, Jan Beulich wrote:
>> On 26.11.2021 13:34, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/acpi/wakeup_prot.S
>>> +++ b/xen/arch/x86/acpi/wakeup_prot.S
>>> @@ -63,7 +63,24 @@ ENTRY(s3_resume)
>>>  pushq   %rax
>>>  lretq
>>>  1:
>>> -#ifdef CONFIG_XEN_SHSTK
>>> +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT)
>>> +callxen_msr_s_cet_value
>>> +test%eax, %eax
>>> +je  .L_cet_done
>> Nit: I consider it generally misleading to use JE / JNE (and a few
>> other Jcc) with other than CMP-like insns. Only those handle actual
>> "relations", whereas e.g. TEST only produces particular flag states,
>> so would more consistently be followed by JZ / JNZ in cases like
>> this one. But since this is very much a matter of taste, I'm not
>> going to insist on a change here.
> 
> Fixed.
> 
>>
>>> +/* Set up MSR_S_CET. */
>>> +mov $MSR_S_CET, %ecx
>>> +xor %edx, %edx
>>> +wrmsr
>>> +
>>> +/* Enable CR4.CET. */
>>> +mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx
>>> +mov %rcx, %cr4
>> Is it valid / safe to enable CR4.CET (with CET_SHSTK_EN already
>> active) before ...
>>
>>> +#if defined(CONFIG_XEN_SHSTK)
>>> +test$CET_SHSTK_EN, %eax
>> (Intermediate remark: Using %al would seem to suffice and be a
>> shorter encoding.)
> 
> Fixed.
> 
>>
>>> +je  .L_cet_done
>>> +
>>>  /*
>>>   * Restoring SSP is a little complicated, because we are 
>>> intercepting
>>>   * an in-use shadow stack.  Write a temporary token under the 
>>> stack,
>>> @@ -71,14 +88,6 @@ ENTRY(s3_resume)
>>>   * reset MSR_PL0_SSP to its usual value and pop the temporary 
>>> token.
>>>   */
>>>  mov saved_ssp(%rip), %rdi
>>> -cmpq$1, %rdi
>>> -je  .L_shstk_done
>>> -
>>> -/* Set up MSR_S_CET. */
>>> -mov $MSR_S_CET, %ecx
>>> -xor %edx, %edx
>>> -mov $CET_SHSTK_EN | CET_WRSS_EN, %eax
>>> -wrmsr
>>>  
>>>  /* Construct the temporary supervisor token under SSP. */
>>>  sub $8, %rdi
>>> @@ -90,12 +99,9 @@ ENTRY(s3_resume)
>>>  mov %edi, %eax
>>>  wrmsr
>>>  
>>> -/* Enable CET.  MSR_INTERRUPT_SSP_TABLE is set up later in 
>>> load_system_tables(). */
>>> -mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx
>>> -mov %rbx, %cr4
>> ... the writing of MSR_PL0_SSP in context here? ISTR some ordering
>> issues back at the time when you introduced CET-SS, so I thought I'd
>> better ask to be sure.
> 
> Yes, it is safe, but the reasons why aren't entirely trivial.
> 
> To set up CET-SS, we need to do the following things:
> 
> 1) CR4.CET=1
> 2) Configure MSR_S_CET.SHSTK_EN
> 3) Configure MSR_PL0_SSP pointing at a non-busy supervisor token
> 4) Configure MSR_ISST_SSP to point at the IST shadow stacks, again with
> non-busy tokens
> 5) execute SETSSBSY to load SSP
> 
> The MSRs can be configured whenever, subject to suitable hardware
> support.  In both of these cases, we've actually pre-configured the
> non-busy supervisor tokens which is why we don't set those up directly. 
> 
> Furthermore, we defer setting up MSR_ISST_SSP to when we set up the IDT
> and TSS, and that's fine because it doesn't make interrupts/exceptions
> any less fatal.
> 
> The only hard ordering is that SETSSBSY depends on CR4.CET &&
> MSR_S_CET.SHSTK_EN in order to not #UD.
> 
> However, between CR4.CET && MSR_S_CET.SHSTK_EN and SETSSBSY, we're
> operating with an SSP of 0, meaning that any call/ret/etc are fatal. 
> That is why I previously grouped the 3 actions as close to together as
> possible.
> 
> For the CONFIG_XEN_IBT && !CONFIG_XEN_SHSTK case, we need to set up CR4
> and MSR_S_CET only.  This was the only way I could find to lay out the
> logic in a half-reasonable way.  It does mean that MSR_PL0_SSP is set up
> during the critical call/ret region, but that's the smallest price I
> could find to pay.  Anything else would have had more conditionals, and
> substantially more #ifdef-ary.
> 
> 
> I have put in this:
> 
> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
> index 9178b2e6a039..6a4834f9813a 100644
> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -45,6 +45,8 @@ ENTRY(__high_start)
>  mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx
>  mov %rcx, %cr4
>  
> +    /* WARNING! call/ret/etc now fatal (iff SHSTK) until SETSSBSY
> loads SSP */
> +
>  #if defined(CONFIG_XEN_SHSTK)
>  test    $CET_SHSTK_EN, %al
>  jz  .L_ap_cet_done
> 
> 
> which mirrors our Spectre-v2 warning in the entry paths.

Thanks, I think this may end up helpful down the road.

Jan

Re: [PATCH V4 6/6] dt-bindings: xen: Clarify "reg" purpose

[Rob Herring]

On Fri, 10 Dec 2021 13:36:41 +0200, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko 
> 
> Xen on Arm has gained new support recently to calculate and report
> extended regions (unused address space) safe to use for external
> mappings. These regions are reported via "reg" property under
> "hypervisor" node in the guest device-tree. As region 0 is reserved
> for grant table space (always present), the indexes for extended
> regions are 1...N.
> 
> No device-tree bindings update is needed (except clarifying the text)
> as guest infers the presence of extended regions from the number
> of regions in "reg" property.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> ---
> Changes V2 -> V3:
>- new patch
> 
> Changes V3 -> V4:
>- add Stefano's R-b and Rob's A-b
>- remove sentence about ACPI for "reg" and "interrupts"
>  properties
> 
> Changes V4 -> V4.1
>- bring the mentioning of ACPI back which, as was pointed out by Julien,
>  fits in the context:
>  
> https://lore.kernel.org/xen-devel/9602b019-6c20-cdc7-23f3-9e4f8fd72...@xen.org/T/#t
>  so technically restore V3 state
>- remove Stefano's R-b and Rob's A-b as I am not 100% sure they are
>  happy with that
> ---
>  Documentation/devicetree/bindings/arm/xen.txt | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

Acked-by: Rob Herring 

Re: [PATCH v2] libxc: avoid clobbering errno in xc_domain_pod_target()

[Jan Beulich]

On 10.12.2021 15:00, Bertrand Marquis wrote:
>> On 10 Dec 2021, at 13:54, Jan Beulich  wrote:
>> On 10.12.2021 14:50, Bertrand Marquis wrote:
 On 10 Dec 2021, at 13:11, Jan Beulich  wrote:

 do_memory_op() supplies return value and has "errno" set the usual way.
 Don't overwrite "errno" with 1 (aka EPERM on at least Linux). There's
 also no reason to overwrite "err".

 Signed-off-by: Jan Beulich 
>>> Reviewed-by: Bertrand Marquis 
>>
>> Thanks.
>>
>>> But if err can really only be 0 or -1, I do wonder if the else forcing err 
>>> to 0 could
>>> be removed but I must say I have no idea if do_memory_op could return a 
>>> value >0.
>>
>> Indeed - see ...
>>
 ---
 While the hypervisor side of the hypercall gives the impression of being
 able to return positive values as of 637a283f17eb ("PoD: Allow
 pod_set_cache_target hypercall to be preempted"), due to the use of
 "rc >= 0" there, afaict that's not actually the case. IOW "err" can
 really only be 0 or -1 here, and hence its setting to zero may also be
 worthwhile to drop.
 ---
>>
>> ... this.
> 
> So the else should be dropped then, why not doing it and just mentioning it 
> there ?

Well, I'd like confirmation by a maintainer. There are a few aspects to how
things are done in the tool stack which I'm not always aware of. IOW there
might be reasons to keep things as they are after this variant of the patch.

Jan

[ovmf test] 167345: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   33 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days7 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH 63/65] x86/setup: Rework MSR_S_CET handling for CET-IBT

[Andrew Cooper]

On 06/12/2021 10:49, Jan Beulich wrote:
> On 26.11.2021 13:34, Andrew Cooper wrote:
>> --- a/xen/arch/x86/acpi/wakeup_prot.S
>> +++ b/xen/arch/x86/acpi/wakeup_prot.S
>> @@ -63,7 +63,24 @@ ENTRY(s3_resume)
>>  pushq   %rax
>>  lretq
>>  1:
>> -#ifdef CONFIG_XEN_SHSTK
>> +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT)
>> +callxen_msr_s_cet_value
>> +test%eax, %eax
>> +je  .L_cet_done
> Nit: I consider it generally misleading to use JE / JNE (and a few
> other Jcc) with other than CMP-like insns. Only those handle actual
> "relations", whereas e.g. TEST only produces particular flag states,
> so would more consistently be followed by JZ / JNZ in cases like
> this one. But since this is very much a matter of taste, I'm not
> going to insist on a change here.

Fixed.

>
>> +/* Set up MSR_S_CET. */
>> +mov $MSR_S_CET, %ecx
>> +xor %edx, %edx
>> +wrmsr
>> +
>> +/* Enable CR4.CET. */
>> +mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx
>> +mov %rcx, %cr4
> Is it valid / safe to enable CR4.CET (with CET_SHSTK_EN already
> active) before ...
>
>> +#if defined(CONFIG_XEN_SHSTK)
>> +test$CET_SHSTK_EN, %eax
> (Intermediate remark: Using %al would seem to suffice and be a
> shorter encoding.)

Fixed.

>
>> +je  .L_cet_done
>> +
>>  /*
>>   * Restoring SSP is a little complicated, because we are 
>> intercepting
>>   * an in-use shadow stack.  Write a temporary token under the stack,
>> @@ -71,14 +88,6 @@ ENTRY(s3_resume)
>>   * reset MSR_PL0_SSP to its usual value and pop the temporary token.
>>   */
>>  mov saved_ssp(%rip), %rdi
>> -cmpq$1, %rdi
>> -je  .L_shstk_done
>> -
>> -/* Set up MSR_S_CET. */
>> -mov $MSR_S_CET, %ecx
>> -xor %edx, %edx
>> -mov $CET_SHSTK_EN | CET_WRSS_EN, %eax
>> -wrmsr
>>  
>>  /* Construct the temporary supervisor token under SSP. */
>>  sub $8, %rdi
>> @@ -90,12 +99,9 @@ ENTRY(s3_resume)
>>  mov %edi, %eax
>>  wrmsr
>>  
>> -/* Enable CET.  MSR_INTERRUPT_SSP_TABLE is set up later in 
>> load_system_tables(). */
>> -mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx
>> -mov %rbx, %cr4
> ... the writing of MSR_PL0_SSP in context here? ISTR some ordering
> issues back at the time when you introduced CET-SS, so I thought I'd
> better ask to be sure.

Yes, it is safe, but the reasons why aren't entirely trivial.

To set up CET-SS, we need to do the following things:

1) CR4.CET=1
2) Configure MSR_S_CET.SHSTK_EN
3) Configure MSR_PL0_SSP pointing at a non-busy supervisor token
4) Configure MSR_ISST_SSP to point at the IST shadow stacks, again with
non-busy tokens
5) execute SETSSBSY to load SSP

The MSRs can be configured whenever, subject to suitable hardware
support.  In both of these cases, we've actually pre-configured the
non-busy supervisor tokens which is why we don't set those up directly. 

Furthermore, we defer setting up MSR_ISST_SSP to when we set up the IDT
and TSS, and that's fine because it doesn't make interrupts/exceptions
any less fatal.

The only hard ordering is that SETSSBSY depends on CR4.CET &&
MSR_S_CET.SHSTK_EN in order to not #UD.

However, between CR4.CET && MSR_S_CET.SHSTK_EN and SETSSBSY, we're
operating with an SSP of 0, meaning that any call/ret/etc are fatal. 
That is why I previously grouped the 3 actions as close to together as
possible.

For the CONFIG_XEN_IBT && !CONFIG_XEN_SHSTK case, we need to set up CR4
and MSR_S_CET only.  This was the only way I could find to lay out the
logic in a half-reasonable way.  It does mean that MSR_PL0_SSP is set up
during the critical call/ret region, but that's the smallest price I
could find to pay.  Anything else would have had more conditionals, and
substantially more #ifdef-ary.


I have put in this:

diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 9178b2e6a039..6a4834f9813a 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -45,6 +45,8 @@ ENTRY(__high_start)
 mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx
 mov %rcx, %cr4
 
+    /* WARNING! call/ret/etc now fatal (iff SHSTK) until SETSSBSY
loads SSP */
+
 #if defined(CONFIG_XEN_SHSTK)
 test    $CET_SHSTK_EN, %al
 jz  .L_ap_cet_done


which mirrors our Spectre-v2 warning in the entry paths.

~Andrew

[ovmf test] 167344: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   32 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days6 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH v2 12/18] AMD/IOMMU: allow use of superpage mappings

[Roger Pau Monné]

On Fri, Sep 24, 2021 at 11:52:14AM +0200, Jan Beulich wrote:
> No separate feature flags exist which would control availability of
> these; the only restriction is HATS (establishing the maximum number of
> page table levels in general), and even that has a lower bound of 4.
> Thus we can unconditionally announce 2M, 1G, and 512G mappings. (Via
> non-default page sizes the implementation in principle permits arbitrary
> size mappings, but these require multiple identical leaf PTEs to be
> written, which isn't all that different from having to write multiple
> consecutive PTEs with increasing frame numbers. IMO that's therefore
> beneficial only on hardware where suitable TLBs exist; I'm unaware of
> such hardware.)

Also replacing/shattering such non-standard page sizes will require
more logic, so unless there's a performance benefit I would just skip
using them.

> 
> Signed-off-by: Jan Beulich 
> ---
> I'm not fully sure about allowing 512G mappings: The scheduling-for-
> freeing of intermediate page tables can take quite a while when
> replacing a tree of 4k mappings by a single 512G one. Plus (or otoh)
> there's no present code path via which 512G chunks of memory could be
> allocated (and hence mapped) anyway.

I would limit to 1G, which is what we support for CPU page tables
also.

> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -32,12 +32,13 @@ static unsigned int pfn_to_pde_idx(unsig
>  }
>  
>  static union amd_iommu_pte clear_iommu_pte_present(unsigned long l1_mfn,
> -   unsigned long dfn)
> +   unsigned long dfn,
> +   unsigned int level)
>  {
>  union amd_iommu_pte *table, *pte, old;
>  
>  table = map_domain_page(_mfn(l1_mfn));
> -pte = [pfn_to_pde_idx(dfn, 1)];
> +pte = [pfn_to_pde_idx(dfn, level)];
>  old = *pte;
>  
>  write_atomic(>raw, 0);
> @@ -288,10 +289,31 @@ static int iommu_pde_from_dfn(struct dom
>  return 0;
>  }
>  
> +static void queue_free_pt(struct domain *d, mfn_t mfn, unsigned int 
> next_level)

Nit: should the last parameter be named level rather than next_level?
AFAICT it's the level of the mfn parameter.

Should we also assert that level (or next_level) is always != 0 for
extra safety?

Thanks, Roger.

[ovmf test] 167342: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   31 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days5 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH 08/65] xen: Annotate fnptr targets from acpi_table_parse()

[Andrew Cooper]

On 06/12/2021 08:36, Jan Beulich wrote:
> On 26.11.2021 13:33, Andrew Cooper wrote:
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
> Elsewhere in this file we have
>
> rc = map ?   map_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn))
>  : unmap_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn));
>
> which has been in this shape only as of commit e3b418ac4919
> ("x86/pvh-dom0: Remove unnecessary function pointer call from
> modify_identity_mmio()"). Aren't we relying on the compiler not
> transforming this back into the earlier
>
> rc = (map ? map_mmio_regions : unmap_mmio_regions)
>  (d, _gfn(pfn), nr_pages, _mfn(pfn));
>
> ?

That old code was especially dumb even before retpoline.  See also the
damage caused by c/s 245a320ce2.

Yes, we are relying on the compiler not to do transformations behind our
backs, but it won't of its own accord.

>  And aren't we further relying on the compiler not transforming direct
> calls into indirect ones for other reasons (I recall Microsoft's compiler
> being pretty aggressive about this when the same function was called
> more than once in close succession, it at least certain past versions)?

That sounds like a broken compiler.

There are legal cases where a direct call has to turn into an indirect
one, and that's when we need to traverse more than disp32 distance.

But without going to a larger mcmodel, we'd get linker errors before
that becomes a problem, because R_X86_64_PLT32 relocations can't be
retrofitted into an indirect call at link time.

> Is the widened effect of the annotation intended to also guarantee that
> indirect calls will not be produced by the compiler for any reason when
> the annotation is absent on a targeted function's declaration?

That would be one for the clang and gcc developers.

I don't see a plausible problem here.

~Andrew

[xen-unstable-smoke test] 167337: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  29e31aaf5d81e57679c2abfe8ffd3851a87042b5
baseline version:
 xen  1384d4e1e4ef6e846a1cac54f6d2868d40309607

Last test of basis   167290  2021-12-09 20:01:35 Z0 days
Testing same since   167337  2021-12-10 10:00:26 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony PERARD 
  Ian Jackson 
  Jan Beulich 
  Stefano Stabellini 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   1384d4e1e4..29e31aaf5d  29e31aaf5d81e57679c2abfe8ffd3851a87042b5 -> smoke

Re: [PATCH 01/65] x86: Introduce support for CET-IBT

[Andrew Cooper]

On 29/11/2021 09:21, Jan Beulich wrote:
> On 26.11.2021 16:21, Andrew Cooper wrote:
>> On 26/11/2021 14:10, Jan Beulich wrote:
>>> On 26.11.2021 13:33, Andrew Cooper wrote:
 @@ -124,6 +129,18 @@ config XEN_SHSTK
  When CET-SS is active, 32bit PV guests cannot be used.  Backwards
  compatiblity can be provided via the PV Shim mechanism.
  
 +config XEN_IBT
 +  bool "Supervisor Indirect Branch Tracking"
 +  depends on HAS_CC_CET_IBT
 +  default y
 +  help
 +Control-flow Enforcement Technology (CET) is a set of features in
 +hardware designed to combat Return-oriented Programming (ROP, also
 +call/jump COP/JOP) attacks.  Indirect Branch Tracking is one CET
 +feature designed to provide function pointer protection.
 +
 +This option arranges for Xen to use CET-IBT for its own protection.
>>> Shouldn't this depend on BROKEN until it's actually functional?
>> It compiles fine right from now, and making it BROKEN would inhibit
>> bisection through the series.
>>
>> Nothing actually matters until patch 65 turns on MSR_S_CET.ENDBR_EN.
> "Nothing" except that until then the promised extra security isn't
> there.

The series is very likely to be committed in one fell swoop, but even
that aside, it really doesn't matter until 4.17-rc1

As it stands, this is ~65 patches of incremental changes to the binary,
and oughtn't to be 65 nops and a massive switch at the end.

~Andrew

[ovmf test] 167341: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 167239
 build-i386-xsm6 xen-buildfail REGR. vs. 167239
 build-i3866 xen-buildfail REGR. vs. 167239
 build-amd64-xsm   6 xen-buildfail REGR. vs. 167239

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf 0f1d7477c0a86a31e2edede7d3a3c74087bb6e21
baseline version:
 ovmf c82ab4d8c148c4009e0b31d1dd2ea6f7d4aea80d

Last test of basis   167239  2021-12-09 06:23:17 Z1 days
Failing since167240  2021-12-09 08:42:46 Z1 days   30 attempts
Testing same since   167338  2021-12-10 10:40:22 Z0 days4 attempts


People who touched revisions under test:
  Brijesh Singh 
  Brijesh Singh via groups.io 
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Philippe Mathieu-Daude 
  Ray Ni 
  Tom Lendacky 

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



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 1070 lines long.)

Re: [PATCH v2 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

[Oleksandr Andrushchenko]

Hi, Jan!

On 10.12.21 11:40, Jan Beulich wrote:
> Determining that behavior is correct (i.e. results in failure) for a
> passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
> more obvious by checking input in generic code - both for singular
> requests to not match the value and for range ones to not pass / wrap
> through it.
>
> For Arm similarly make more obvious that no wrapping of MFNs passed
> for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
> Drop the "nr" parameter of the function to avoid future callers
> appearing which might not themselves check for wrapping. Otherwise
> the respective ASSERT() in rangeset_contains_range() could trigger.
>
> Signed-off-by: Jan Beulich 
> ---
> v2: Add comment to BUILD_BUG_ON(). Avoid transiently #define-ing _gfn()
>  (by way of new prereq patch).
>
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
>   break;
>   }
>   case XENMAPSPACE_dev_mmio:
> -rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
> +rc = map_dev_mmio_region(d, gfn, _mfn(idx));
Technically this is ok, but reads odd now: the function maps a single
page, but its name has "region" in it (which might also be ok, e.g.
for a region of a single page).

I think it is worth either implementing full mfn range check inside
map_dev_mmio_region or renaming it to something else:
with mfn check map_dev_mmio_region will indeed be able to map
a region consisting of multiple pages and perform required validation.

Thank you,
Oleksandr
>   return rc;
>   
>   default:
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1357,19 +1357,18 @@ int unmap_mmio_regions(struct domain *d,
>   
>   int map_dev_mmio_region(struct domain *d,
>   gfn_t gfn,
> -unsigned long nr,
>   mfn_t mfn)
>   {
>   int res;
>   
> -if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) 
> )
> +if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
>   return 0;
>   
> -res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
> +res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
>   if ( res < 0 )
>   {
> -printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" 
> in Dom%d\n",
> -   mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
> +printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
> +   mfn_x(mfn), d);
>   return res;
>   }
>   
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -4150,7 +4150,10 @@ int gnttab_map_frame(struct domain *d, u
>   bool status = false;
>   
>   if ( gfn_eq(gfn, INVALID_GFN) )
> +{
> +ASSERT_UNREACHABLE();
>   return -EINVAL;
> +}
>   
>   grant_write_lock(gt);
>   
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -832,6 +832,9 @@ int xenmem_add_to_physmap(struct domain
>   return -EACCES;
>   }
>   
> +if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )
> +return -EINVAL;
> +
>   if ( xatp->space == XENMAPSPACE_gmfn_foreign )
>   extra.foreign_domid = DOMID_INVALID;
>   
> @@ -842,6 +845,18 @@ int xenmem_add_to_physmap(struct domain
>   if ( xatp->size < start )
>   return -EILSEQ;
>   
> +if ( xatp->gpfn + xatp->size < xatp->gpfn ||
> + xatp->idx + xatp->size < xatp->idx )
> +{
> +/*
> + * Make sure INVALID_GFN is the highest representable value, i.e.
> + * guaranteeing that it won't fall in the middle of the
> + * [xatp->gpfn, xatp->gpfn + xatp->size) range checked above.
> + */
> +BUILD_BUG_ON(INVALID_GFN_RAW + 1);
> +return -EOVERFLOW;
> +}
> +
>   xatp->idx += start;
>   xatp->gpfn += start;
>   xatp->size -= start;
> @@ -962,6 +977,9 @@ static int xenmem_add_to_physmap_batch(s
>  extent, 1)) )
>   return -EFAULT;
>   
> +if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )
> +return -EINVAL;
> +
>   rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
>  idx, _gfn(gpfn));
>   
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -297,7 +297,6 @@ int unmap_regions_p2mt(struct domain *d,
>   
>   int map_dev_mmio_region(struct domain *d,
>   gfn_t gfn,
> -unsigned long nr,
>   mfn_t mfn);
>   
>   int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr,
>
>

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Juergen Gross]

On 10.12.21 14:49, Andrew Cooper wrote:

On 10/12/2021 11:16, Juergen Gross wrote:

On 09.12.21 18:07, Andrew Cooper wrote:

The values are already available in dom->{console,xenstore}_pfn, just
like on
the PV side of things.  No need to ask Xen.

Signed-off-by: Andrew Cooper 
---
CC: Wei Liu 
CC: Anthony PERARD 
CC: Juergen Gross 
---
   tools/libs/light/libxl_dom.c | 17 +
   1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index c9c24666cd04..03841243ab47 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
   }
     static int hvm_build_set_params(xc_interface *handle, uint32_t
domid,
-    libxl_domain_build_info *info,
-    unsigned long *store_mfn,
-    unsigned long *console_mfn)
+    libxl_domain_build_info *info)
   {
   struct hvm_info_table *va_hvm;
   uint8_t *va_map, sum;
-    uint64_t str_mfn, cons_mfn;
   int i;
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {


What about moving this if () to the only caller and renaming the
function from hvm_build_set_params() to hvm_set_info_table()?


Because I was hoping to delete it outright in a subsequent patch.


I'd suggest to either do the renaming or to add that subsequent patch
making this a small series.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v2] libxc: avoid clobbering errno in xc_domain_pod_target()

[Bertrand Marquis]

Hi Jan,

> On 10 Dec 2021, at 13:54, Jan Beulich  wrote:
> 
> On 10.12.2021 14:50, Bertrand Marquis wrote:
>>> On 10 Dec 2021, at 13:11, Jan Beulich  wrote:
>>> 
>>> do_memory_op() supplies return value and has "errno" set the usual way.
>>> Don't overwrite "errno" with 1 (aka EPERM on at least Linux). There's
>>> also no reason to overwrite "err".
>>> 
>>> Signed-off-by: Jan Beulich 
>> Reviewed-by: Bertrand Marquis 
> 
> Thanks.
> 
>> But if err can really only be 0 or -1, I do wonder if the else forcing err 
>> to 0 could
>> be removed but I must say I have no idea if do_memory_op could return a 
>> value >0.
> 
> Indeed - see ...
> 
>>> ---
>>> While the hypervisor side of the hypercall gives the impression of being
>>> able to return positive values as of 637a283f17eb ("PoD: Allow
>>> pod_set_cache_target hypercall to be preempted"), due to the use of
>>> "rc >= 0" there, afaict that's not actually the case. IOW "err" can
>>> really only be 0 or -1 here, and hence its setting to zero may also be
>>> worthwhile to drop.
>>> ---
> 
> ... this.

So the else should be dropped then, why not doing it and just mentioning it 
there ?

Bertrand

> 
> Jan
> 

Re: [PATCH V4 6/6] dt-bindings: xen: Clarify "reg" purpose

[Bertrand Marquis]

Hi Oleksandr,

> On 10 Dec 2021, at 11:36, Oleksandr Tyshchenko  wrote:
> 
> From: Oleksandr Tyshchenko 
> 
> Xen on Arm has gained new support recently to calculate and report
> extended regions (unused address space) safe to use for external
> mappings. These regions are reported via "reg" property under
> "hypervisor" node in the guest device-tree. As region 0 is reserved
> for grant table space (always present), the indexes for extended
> regions are 1...N.
> 
> No device-tree bindings update is needed (except clarifying the text)
> as guest infers the presence of extended regions from the number
> of regions in "reg" property.
> 
> Signed-off-by: Oleksandr Tyshchenko 
Reviewed-by: Bertrand Marquis 

Cheers
Bertrand

> ---
> Changes V2 -> V3:
>   - new patch
> 
> Changes V3 -> V4:
>   - add Stefano's R-b and Rob's A-b
>   - remove sentence about ACPI for "reg" and "interrupts"
> properties
> 
> Changes V4 -> V4.1
>   - bring the mentioning of ACPI back which, as was pointed out by Julien,
> fits in the context:
> 
> https://lore.kernel.org/xen-devel/9602b019-6c20-cdc7-23f3-9e4f8fd72...@xen.org/T/#t
> so technically restore V3 state
>   - remove Stefano's R-b and Rob's A-b as I am not 100% sure they are
> happy with that
> ---
> Documentation/devicetree/bindings/arm/xen.txt | 12 
> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
> b/Documentation/devicetree/bindings/arm/xen.txt
> index db5c56d..156fe10b 100644
> --- a/Documentation/devicetree/bindings/arm/xen.txt
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -7,10 +7,14 @@ the following properties:
>   compatible = "xen,xen-", "xen,xen";
>   where  is the version of the Xen ABI of the platform.
> 
> -- reg: specifies the base physical address and size of a region in
> -  memory where the grant table should be mapped to, using an
> -  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
> -  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
> +- reg: specifies the base physical address and size of the regions in memory
> +  where the special resources should be mapped to, using an 
> HYPERVISOR_memory_op
> +  hypercall.
> +  Region 0 is reserved for mapping grant table, it must be always present.
> +  The memory region is large enough to map the whole grant table (it is 
> larger
> +  or equal to gnttab_max_grant_frames()).
> +  Regions 1...N are extended regions (unused address space) for mapping 
> foreign
> +  GFNs and grants, they might be absent if there is nothing to expose.
>   This property is unnecessary when booting Dom0 using ACPI.
> 
> - interrupts: the interrupt used by Xen to inject event notifications.
> -- 
> 2.7.4
> 
> 

Re: [PATCH v2] libxc: avoid clobbering errno in xc_domain_pod_target()

[Jan Beulich]

On 10.12.2021 14:50, Bertrand Marquis wrote:
>> On 10 Dec 2021, at 13:11, Jan Beulich  wrote:
>>
>> do_memory_op() supplies return value and has "errno" set the usual way.
>> Don't overwrite "errno" with 1 (aka EPERM on at least Linux). There's
>> also no reason to overwrite "err".
>>
>> Signed-off-by: Jan Beulich 
> Reviewed-by: Bertrand Marquis 

Thanks.

> But if err can really only be 0 or -1, I do wonder if the else forcing err to 
> 0 could
> be removed but I must say I have no idea if do_memory_op could return a value 
> >0.

Indeed - see ...

>> ---
>> While the hypervisor side of the hypercall gives the impression of being
>> able to return positive values as of 637a283f17eb ("PoD: Allow
>> pod_set_cache_target hypercall to be preempted"), due to the use of
>> "rc >= 0" there, afaict that's not actually the case. IOW "err" can
>> really only be 0 or -1 here, and hence its setting to zero may also be
>> worthwhile to drop.
>> ---

... this.

Jan

Re: [PATCH v2 11/18] AMD/IOMMU: return old PTE from {set,clear}_iommu_pte_present()

[Roger Pau Monné]

On Fri, Dec 10, 2021 at 01:59:02PM +0100, Jan Beulich wrote:
> On 10.12.2021 13:05, Roger Pau Monné wrote:
> > On Fri, Sep 24, 2021 at 11:51:40AM +0200, Jan Beulich wrote:
> >> In order to free intermediate page tables when replacing smaller
> >> mappings by a single larger one callers will need to know the full PTE.
> >> Flush indicators can be derived from this in the callers (and outside
> >> the locked regions). First split set_iommu_pte_present() from
> >> set_iommu_ptes_present(): Only the former needs to return the old PTE,
> >> while the latter (like also set_iommu_pde_present()) doesn't even need
> >> to return flush indicators. Then change return types/values and callers
> >> accordingly.
> > 
> > Without looking at further patches I would say you only care to know
> > whether the old PTE was present (ie: pr bit set), at which point those
> > functions could also return a boolean instead of a full PTE?
> 
> But looking at further patches will reveal that I then also need the
> next_level field from the old PTE (to tell superpages from page tables).

Oh, OK. I was expecting something like that.

Reviewed-by: Roger Pau Monné 

I wouldn't mind if you added a note to the commit message that the
full PTE is returned because new callers will require more data.

Thanks, Roger.

Re: [PATCH v2 08/18] IOMMU/x86: support freeing of pagetables

[Roger Pau Monné]

On Fri, Sep 24, 2021 at 11:48:21AM +0200, Jan Beulich wrote:
> For vendor specific code to support superpages we need to be able to
> deal with a superpage mapping replacing an intermediate page table (or
> hierarchy thereof). Consequently an iommu_alloc_pgtable() counterpart is
> needed to free individual page tables while a domain is still alive.
> Since the freeing needs to be deferred until after a suitable IOTLB
> flush was performed, released page tables get queued for processing by a
> tasklet.
> 
> Signed-off-by: Jan Beulich 
> ---
> I was considering whether to use a softirq-taklet instead. This would
> have the benefit of avoiding extra scheduling operations, but come with
> the risk of the freeing happening prematurely because of a
> process_pending_softirqs() somewhere.

The main one that comes to mind would be the debug keys and it's usage
of process_pending_softirqs that could interfere with iommu unmaps, so
I guess if only for that reason it's best to run in idle vcpu context.

> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -12,6 +12,7 @@
>   * this program; If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -463,6 +464,85 @@ struct page_info *iommu_alloc_pgtable(st
>  return pg;
>  }
>  
> +/*
> + * Intermediate page tables which get replaced by large pages may only be
> + * freed after a suitable IOTLB flush. Hence such pages get queued on a
> + * per-CPU list, with a per-CPU tasklet processing the list on the assumption
> + * that the necessary IOTLB flush will have occurred by the time tasklets get
> + * to run. (List and tasklet being per-CPU has the benefit of accesses not
> + * requiring any locking.)
> + */
> +static DEFINE_PER_CPU(struct page_list_head, free_pgt_list);
> +static DEFINE_PER_CPU(struct tasklet, free_pgt_tasklet);
> +
> +static void free_queued_pgtables(void *arg)
> +{
> +struct page_list_head *list = arg;
> +struct page_info *pg;
> +
> +while ( (pg = page_list_remove_head(list)) )
> +free_domheap_page(pg);

Should you add a preempt check here to yield and schedule the tasklet
again, in order to be able to process any pending work?

Maybe just calling process_pending_softirqs would be enough?

Thanks, Roger.