date:20200311

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

2020-03-11 Thread osstest service owner

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 6b7855209ad8e3e077a24faa40f69acdc231ff4f
baseline version:
 ovmf a3e25cc8a1dd3d1ea24ed02f90c44221e015e965

Last test of basis   148345  2020-03-09 22:26:54 Z2 days
Testing same since   148414  2020-03-11 05:11:21 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Christopher J Zurcher 
  Zurcher, Christopher J 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   a3e25cc8a1..6b7855209a  6b7855209ad8e3e077a24faa40f69acdc231ff4f -> 
xen-tested-master

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

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

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 146182
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 146182
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 146182
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 146182

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  9fe6c1dc7fad81c21737063ab92eef100f00bb7e
baseline version:
 libvirt  a1cd25b919509be2645dbe6f952d5263e0d4e4e5

Last test of basis   146182  2020-01-17 06:00:23 Z   54 days
Failing since146211  2020-01-18 04:18:52 Z   53 days   50 attempts
Testing same since   148406  2020-03-11 02:28:00 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Arnaud Patard 
  Boris Fiuczynski 
  Christian Ehrhardt 
  Collin Walling 
  Daniel Henrique Barboza 
  Daniel P. Berrangé 
  Daniel Veillard 
  Dario Faggioli 
  Erik Skultety 
  Gaurav Agrawal 
  Han Han 
  Jim Fehlig 
  Jiri Denemark 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Laine Stump 
  Marek Marczykowski-Górecki 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Pavel Hrdina 
  Pavel Mores 
  Peter Krempa 
  Richard W.M. Jones 
  Rikard Falkeborn 
  Ryan Moeller 
  Sahid Orentino Ferdjaoui 
  Stefan Berger 
  Stefan Berger 
  Stefan Hajnoczi 
  Thomas Huth 
  Your Name 
  zhenwei pi 
  Zhimin Feng 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-arm64-libvirt  fail
 build-armhf-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   blocked 
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked 
 test-amd64-amd64-libvirt-xsm blocked 
 test-arm64-arm64-libvirt-xsm blocked 
 test-amd64-i386-libvirt-xsm  blocked 
 test-amd64-amd64-libvirt blocked 
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-libvirt-pairblocked 
 test-amd64-i386-libvirt-pair blocked 
 test-arm64-arm64-libvirt-qcow2   blocked 
 test-armhf-armhf-libvirt-raw blocked 
 test-amd64-amd64-libvirt-vhd blocked 



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

Logs,

[Xen-devel] [linux-4.14 test] 148371: regressions - FAIL

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-rtds broken  in 148294
 test-amd64-amd64-xl-qemut-debianhvm-amd64 broken in 148294
 test-amd64-i386-libvirt  broken  in 148294
 test-amd64-i386-qemuu-rhel6hvm-amdbroken in 148294
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict  broken in 
148294
 test-amd64-i386-freebsd10-amd64   broken in 148294
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  broken in 
148294
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow   broken in 148294
 test-amd64-i386-xl-qemuu-ws16-amd64   broken in 148294
 test-amd64-amd64-xl-qemuu-win7-amd64  broken in 148294
 test-amd64-amd64-libvirt-pair broken in 148294
 test-amd64-i386-xl-qemut-ws16-amd64   broken in 148294
 test-armhf-armhf-xl-credit1  broken  in 148294
 test-amd64-amd64-xl-credit1  broken  in 148294
 test-amd64-amd64-xl-pvhv2-amd broken in 148294
 test-amd64-i386-xl-qemuu-ovmf-amd64   broken in 148294
 test-amd64-i386-xl-qemuu-debianhvm-amd64  broken in 148294
 test-amd64-amd64-qemuu-nested-intel   broken in 148294
 test-amd64-amd64-xl-multivcpu broken in 148294
 test-amd64-amd64-xl-pvshim   broken  in 148294
 test-armhf-armhf-xl-credit2   7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-examine  8 reboot   fail REGR. vs. 142849
 test-armhf-armhf-xl-credit1   7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-libvirt-raw  7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-xl   7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-xl-multivcpu  7 xen-bootfail REGR. vs. 142849
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
142849
 test-armhf-armhf-libvirt  7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-xl-vhd   7 xen-boot fail REGR. vs. 142849
 test-armhf-armhf-xl-arndale   7 xen-boot fail REGR. vs. 142849

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken in 
148294 pass in 148371
 test-armhf-armhf-xl-credit1  4 host-install(4) broken in 148294 pass in 148371
 test-amd64-amd64-xl-credit1  4 host-install(4) broken in 148294 pass in 148371
 test-amd64-amd64-xl-pvhv2-amd 4 host-install(4) broken in 148294 pass in 148371
 test-amd64-i386-freebsd10-amd64 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-i386-qemuu-rhel6hvm-amd 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-amd64-xl-rtds 4 host-install(4) broken in 148294 pass in 148371
 test-amd64-amd64-xl-qemuu-win7-amd64 4 host-install(4) broken in 148294 pass 
in 148371
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 
148294 pass in 148371
 test-amd64-amd64-libvirt-pair 4 host-install/src_host(4) broken in 148294 pass 
in 148371
 test-amd64-amd64-libvirt-pair 5 host-install/dst_host(5) broken in 148294 pass 
in 148371
 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken in 148294 
pass in 148371
 test-amd64-i386-xl-qemut-ws16-amd64 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-amd64-qemuu-nested-intel 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-i386-xl-qemuu-ovmf-amd64 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-i386-libvirt  4 host-install(4) broken in 148294 pass in 148371
 test-amd64-i386-xl-qemuu-ws16-amd64 4 host-install(4) broken in 148294 pass in 
148371
 test-amd64-amd64-xl-qemut-debianhvm-amd64 4 host-install(4) broken in 148294 
pass in 148371
 test-amd64-amd64-xl-pvshim   4 host-install(4) broken in 148294 pass in 148371
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken 
in 148294 pass in 148371
 test-amd64-amd64-xl-multivcpu 4 host-install(4) broken in 148294 pass in 148371
 test-amd64-amd64-xl-shadow 18 guest-localmigrate/x10 fail in 148232 pass in 
148371
 test-armhf-armhf-xl-rtds 12 guest-start  fail in 148294 pass in 148371
 test-amd64-i386-libvirt  12 guest-startfail pass in 148232
 test-amd64-amd64-xl-rtds 16 guest-localmigrate fail pass in 148232
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail pass in 148294

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds  18 guest-localmigrate/x10 fail in 148232 like 142849
 test-amd64-i386-libvirt 13 migrate-support-check fail in 148232 never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop   fail in 148232 never pass

[Xen-devel] [qemu-mainline bisection] complete test-amd64-i386-xl-qemuu-debianhvm-amd64

2020-03-11 Thread osstest service owner

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemuu-debianhvm-amd64
testid debian-hvm-install

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  qemuu git://git.qemu.org/qemu.git
  Bug introduced:  ca6155c0f2bd39b4b4162533be401c98bd960820
  Bug not present: c220cdec4845f305034330f80ce297f1f997f2d3
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/148455/


  (Revision log too long, omitted.)


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/test-amd64-i386-xl-qemuu-debianhvm-amd64.debian-hvm-install.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/qemu-mainline/test-amd64-i386-xl-qemuu-debianhvm-amd64.debian-hvm-install
 --summary-out=tmp/148455.bisection-summary --basis-template=144861 
--blessings=real,real-bisect qemu-mainline 
test-amd64-i386-xl-qemuu-debianhvm-amd64 debian-hvm-install
Searching for failure / basis pass:
 148340 fail [host=albana0] / 147546 ok.
Failure / basis pass flights: 148340 / 147546
(tree with no url: minios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3b9cd714542a8744252d973e1f163222a9f21b9e 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
373c7068dd610e97f0b551b5a6d0a27cd6da4506 
24d3938ca96a6420ec1a5f1f8479f90f2e9fdd56 
0d99c909d7e1cbe69329a00f7772946f10a7865b
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
70911f1f4aee0366b6122f2b90d367ec0f066beb 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
c1e667d2598b9b3ce62b8e89ed22dd38dfe9f57f 
76551856b28d227cb0386a1ab0e774329b941f7d 
c47984aabead53918e5ba6d43cdb3f1467452739
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/osstest/ovmf.git#70911f1f4aee0366b6122f2b90d367ec0f066beb-3b9cd714542a8744252d973e1f163222a9f21b9e
 git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484\
 fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 
git://git.qemu.org/qemu.git#c1e667d2598b9b3ce62b8e89ed22dd38dfe9f57f-373c7068dd610e97f0b551b5a6d0a27cd6da4506
 
git://xenbits.xen.org/osstest/seabios.git#76551856b28d227cb0386a1ab0e774329b941f7d-24d3938ca96a6420ec1a5f1f8479f90f2e9fdd56
 
git://xenbits.xen.org/xen.git#c47984aabead53918e5ba6d43cdb3f1467452739-0d99c909d7e1cbe69329a00f7772946f10a7865b
From git://cache:9419/git://xenbits.xen.org/xen
   e19d3a942e..a9b6dacf88  smoke  -> origin/smoke
Use of uninitialized value $parents in array dereference at 
./adhoc-revtuple-generator line 465.
Use of uninitialized value in concatenation (.) or string at 
./adhoc-revtuple-generator line 465.
Use of uninitialized value $parents in array dereference at 
./adhoc-revtuple-generator line 465.
Use of uninitialized value in concatenation (.) or string at 
./adhoc-revtuple-generator line 465.
Loaded 22396 nodes in revision graph
Searching for test results:
 147546 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
70911f1f4aee0366b6122f2b90d367ec0f066beb 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
c1e667d2598b9b3ce62b8e89ed22dd38dfe9f57f 
76551856b28d227cb0386a1ab0e774329b941f7d 
c47984aabead53918e5ba6d43cdb3f1467452739
 147641 fail irrelevant
 147710 fail irrelevant
 147758 fail irrelevant
 147821 fail irrelevant
 148010 fail irrelevant
 148184 fail irrelevant
 148120 fail irrelevant
 148261 fail c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3b9cd714542a8744252d973e1f163222a9f21b9e 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
67f17e23baca5dd545fe98b01169cc351a70fe35 
24d3938ca96a6420ec1a5f1f8479f90f2e9fdd56 
0d99c909d7e1cbe69329a00f7772946f10a7865b
 148266 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
70911f1f4aee0366b6122f2b90d367ec0f066beb 
d0d8ad39ecb51cd7497cd524484fe09f50876798

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

2020-03-11 Thread osstest service owner

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  a9b6dacf88fe99fbb69a2ee505833851ffdc9cec
baseline version:
 xen  e19d3a942e4b6f6c5b19287a4a6f5020bdab2936

Last test of basis   148408  2020-03-11 03:01:39 Z0 days
Testing same since   148436  2020-03-11 13:00:35 Z0 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 

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
   e19d3a942e..a9b6dacf88  a9b6dacf88fe99fbb69a2ee505833851ffdc9cec -> smoke

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

[Xen-devel] [PATCH] x86/vvmx: Fix deadlock with MSR bitmap merging

2020-03-11 Thread Andrew Cooper

c/s c47984aabead "nvmx: implement support for MSR bitmaps" introduced a use of
map_domain_page() which may get used in the middle of context switch.

This is not safe, and causes Xen to deadlock on the mapcache lock:

  (XEN) Xen call trace:
  (XEN)[] R _spin_lock+0x34/0x5e
  (XEN)[] F map_domain_page+0x250/0x527
  (XEN)[] F do_page_fault+0x420/0x780
  (XEN)[] F 
x86_64/entry.S#handle_exception_saved+0x68/0x94
  (XEN)[] F __find_next_zero_bit+0x28/0x69
  (XEN)[] F map_domain_page+0x2c6/0x527
  (XEN)[] F nvmx_update_exec_control+0x1d7/0x323
  (XEN)[] F vmx_update_cpu_exec_control+0x23/0x40
  (XEN)[] F 
arch/x86/hvm/vmx/vmx.c#vmx_ctxt_switch_from+0xb7/0x121
  (XEN)[] F arch/x86/domain.c#__context_switch+0x124/0x4a9
  (XEN)[] F context_switch+0x154/0x62c
  (XEN)[] F 
common/sched/core.c#sched_context_switch+0x16a/0x175
  (XEN)[] F common/sched/core.c#schedule+0x2ad/0x2bc
  (XEN)[] F common/softirq.c#__do_softirq+0xb7/0xc8
  (XEN)[] F do_softirq+0x18/0x1a
  (XEN)[] F vmx_asm_do_vmentry+0x2b/0x30

Convert the domheap page into being a xenheap page.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: Kevin Tian 

I suspect this is the not-quite-consistent-enough-to-bisect issue which
OSSTest is hitting and interfering with pushes to master.
---
 xen/arch/x86/hvm/vmx/vvmx.c| 19 ---
 xen/include/asm-x86/hvm/vmx/vvmx.h |  2 +-
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 926a11c15f..f049920196 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -130,12 +130,9 @@ int nvmx_vcpu_initialise(struct vcpu *v)
 
 if ( cpu_has_vmx_msr_bitmap )
 {
-nvmx->msr_merged = alloc_domheap_page(d, MEMF_no_owner);
+nvmx->msr_merged = alloc_xenheap_page();
 if ( !nvmx->msr_merged )
-{
-gdprintk(XENLOG_ERR, "nest: allocation for MSR bitmap failed\n");
 return -ENOMEM;
-}
 }
 
 nvmx->ept.enabled = 0;
@@ -198,11 +195,7 @@ static void vcpu_relinquish_resources(struct vcpu *v)
 {
 struct nestedvmx *nvmx = _2_nvmx(v);
 
-if ( nvmx->msr_merged )
-{
-free_domheap_page(nvmx->msr_merged);
-nvmx->msr_merged = NULL;
-}
+FREE_XENHEAP_PAGE(nvmx->msr_merged);
 }
 
 void nvmx_domain_relinquish_resources(struct domain *d)
@@ -575,14 +568,12 @@ unsigned long *_shadow_io_bitmap(struct vcpu *v)
 static void update_msrbitmap(struct vcpu *v, uint32_t shadow_ctrl)
 {
 struct nestedvmx *nvmx = _2_nvmx(v);
-struct vmx_msr_bitmap *msr_bitmap;
+struct vmx_msr_bitmap *msr_bitmap = nvmx->msr_merged;
 
 if ( !(shadow_ctrl & CPU_BASED_ACTIVATE_MSR_BITMAP) ||
  !nvmx->msrbitmap )
return;
 
-msr_bitmap = __map_domain_page(nvmx->msr_merged);
-
 bitmap_or(msr_bitmap->read_low, nvmx->msrbitmap->read_low,
   v->arch.hvm.vmx.msr_bitmap->read_low,
   sizeof(msr_bitmap->read_low) * 8);
@@ -603,9 +594,7 @@ static void update_msrbitmap(struct vcpu *v, uint32_t 
shadow_ctrl)
 bitmap_set(msr_bitmap->read_low, MSR_X2APIC_FIRST, 0x100);
 bitmap_set(msr_bitmap->write_low, MSR_X2APIC_FIRST, 0x100);
 
-unmap_domain_page(msr_bitmap);
-
-__vmwrite(MSR_BITMAP, page_to_maddr(nvmx->msr_merged));
+__vmwrite(MSR_BITMAP, virt_to_maddr(nvmx->msr_merged));
 }
 
 void nvmx_update_exec_control(struct vcpu *v, u32 host_cntrl)
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h 
b/xen/include/asm-x86/hvm/vmx/vvmx.h
index c41f089939..d5f68f30b1 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -38,7 +38,7 @@ struct nestedvmx {
 paddr_tvmxon_region_pa;
 void   *iobitmap[2];   /* map (va) of L1 guest I/O bitmap */
 struct vmx_msr_bitmap *msrbitmap;  /* map (va) of L1 guest MSR bitmap */
-struct page_info *msr_merged;  /* merged L1 and L2 MSR bitmap */
+struct vmx_msr_bitmap *msr_merged; /* merged L1 and L2 MSR bitmap */
 /* deferred nested interrupt */
 struct {
 unsigned long intr_info;
-- 
2.11.0


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

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

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-multivcpu broken in 148288
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict  broken in 
148288
 test-amd64-i386-qemut-rhel6hvm-amdbroken in 148288
 test-amd64-amd64-xl-qemut-debianhvm-amd64 broken in 148288
 test-amd64-amd64-xl-qemuu-win7-amd64  broken in 148288
 test-amd64-i386-qemut-rhel6hvm-intel  broken in 148288
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  broken in 148288
 test-amd64-amd64-xl-pvhv2-amd broken in 148288
 test-amd64-amd64-qemuu-nested-amd broken in 148288
 test-amd64-i386-xl-shadowbroken  in 148288
 test-amd64-i386-freebsd10-i386broken in 148288
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  broken in 
148288
 test-amd64-i386-libvirt  broken  in 148288
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
142932

Tests which are failing intermittently (not blocking):
 test-amd64-i386-qemut-rhel6hvm-intel 4 host-install(4) broken in 148288 pass 
in 148364
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 
148288 pass in 148364
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken in 
148288 pass in 148364
 test-amd64-i386-freebsd10-i386 4 host-install(4) broken in 148288 pass in 
148364
 test-amd64-i386-xl-shadow4 host-install(4) broken in 148288 pass in 148364
 test-amd64-i386-libvirt  4 host-install(4) broken in 148288 pass in 148364
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken 
in 148288 pass in 148364
 test-amd64-amd64-qemuu-nested-amd 4 host-install(4) broken in 148288 pass in 
148364
 test-amd64-amd64-xl-pvhv2-amd 4 host-install(4) broken in 148288 pass in 148364
 test-amd64-amd64-xl-qemuu-win7-amd64 4 host-install(4) broken in 148288 pass 
in 148364
 test-amd64-amd64-xl-qemut-debianhvm-amd64 4 host-install(4) broken in 148288 
pass in 148364
 test-amd64-i386-qemut-rhel6hvm-amd 4 host-install(4) broken in 148288 pass in 
148364
 test-amd64-amd64-xl-multivcpu 4 host-install(4) broken in 148288 pass in 148364
 test-amd64-amd64-libvirt 12 guest-startfail pass in 148288
 test-amd64-i386-libvirt-xsm  12 guest-startfail pass in 148288
 test-amd64-amd64-xl-qcow211 guest-startfail pass in 148288
 test-amd64-amd64-xl-credit1  20 guest-start/debian.repeat  fail pass in 148288
 test-amd64-amd64-libvirt-xsm 19 guest-start.2  fail pass in 148288
 test-arm64-arm64-xl-credit1  16 guest-start/debian.repeat  fail pass in 148288
 test-armhf-armhf-xl-arndale  16 guest-start/debian.repeat  fail pass in 148288
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat  fail pass in 148288
 test-armhf-armhf-xl-multivcpu 12 guest-start   fail pass in 148288
 test-armhf-armhf-xl-credit1  12 guest-startfail pass in 148288
 test-armhf-armhf-xl  12 guest-startfail pass in 148288
 test-amd64-i386-xl-qemut-debianhvm-amd64 18 guest-start/debianhvm.repeat fail 
pass in 148288
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 18 
guest-start/debianhvm.repeat fail pass in 148288
 test-amd64-i386-xl-qemuu-debianhvm-amd64 18 guest-start/debianhvm.repeat fail 
pass in 148288
 test-armhf-armhf-libvirt 16 guest-start/debian.repeat  fail pass in 148288
 test-arm64-arm64-xl-seattle  16 guest-start/debian.repeat  fail pass in 148288

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt13 migrate-support-check fail in 148288 never pass
 test-amd64-i386-libvirt-xsm 13 migrate-support-check fail in 148288 never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-check fail in 148288 never 
pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-check fail in 148288 
never pass
 test-armhf-armhf-xl-credit1 13 migrate-support-check fail in 148288 never pass
 test-armhf-armhf-xl-credit1 14 saverestore-support-check fail in 148288 never 
pass
 test-armhf-armhf-xl 13 migrate-support-check fail in 148288 never pass
 test-armhf-armhf-xl 14 saverestore-support-check fail in 148288 never pass
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 142880
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 142932
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11

[Xen-devel] nvmx deadlock with MSR bitmaps

2020-03-11 Thread Andrew Cooper

Hello,

Testing has encountered this deadlock:

(XEN) Watchdog timer detects that CPU0 is stuck!
(XEN) [ Xen-4.14.0-9.0.4-d  x86_64  debug=y   Not tainted ]
(XEN) CPU:    0
(XEN) RIP:    e008:[] _spin_lock+0x34/0x5e
(XEN) RFLAGS: 0002   CONTEXT: hypervisor (d0v0)
(XEN) rax: 9a44   rbx: 8308466712b8   rcx: 9a45
(XEN) rdx: 8308466712b8   rsi: 9a44   rdi: 8308466712be
(XEN) rbp: 8300952c7b08   rsp: 8300952c7af8   r8:  
(XEN) r9:     r10:    r11: 
(XEN) r12: 8308466712be   r13: 83084665a000   r14: 00555e52
(XEN) r15: 0002   cr0: 8005003b   cr4: 001526e0
(XEN) cr3: 000555e52000   cr2: 820060001008
(XEN) fsb:    gsb:    gss: 8e80987a
(XEN) ds:    es:    fs:    gs:    ss:    cs: e008
(XEN) Xen code around  (_spin_lock+0x34/0x5e):
(XEN)  da 89 c1 f3 90 66 8b 02 <66> 39 c1 75 f6 4c 89 e7 e8 85 fe ff ff
48 8d 05
(XEN) Xen stack trace from rsp=8300952c7af8:
(XEN)    830846671000 820060001008 8300952c7b58 82d0803219d7
(XEN)    8308466712b8 0086 82d08038d979 8300952c7bd8
(XEN)    820060001008   0001
(XEN)    8300952c7bc8 82d080356332 0046 82d08038d96d
(XEN)     8038d96d 82d08038d979 82d08038d96d
(XEN)    82d08038d979 83084665a000  
(XEN)    8300952c7fff  7cff6ad38407 82d08038da3d
(XEN)    0001 005d4a49 83084665a000 0080
(XEN)    8300952c7c88 005e 048ee9d28fde 048ee9c2fae4
(XEN)    2000  0001 001e
(XEN)    0022 0080 820060001008 000e
(XEN)    82d08031729f e008 00010016 8300952c7c80
(XEN)     830846671000 8300952c7cd8 82d080321a4d
(XEN)    8308466712b8 0086 00557140 b6a065fa
(XEN)    830555e6e000 1000 7d20 8305571a4000
(XEN)    8300952c7d08 82d08029eeb2 830555e6e000 830846671000
(XEN)     830555e6e000 8300952c7d28 82d080299f5a
(XEN)    830555e6e000 830555e6e000 8300952c7d48 82d08029a3f7
(XEN)    8300952c7d48 83084665a000 8300952c7da8 82d08031d796
(XEN)    8300952c7d78 8300952c7ef8 8300952c7d78 82d08029e684
(XEN) Xen call trace:
(XEN)    [] R _spin_lock+0x34/0x5e
(XEN)    [] F map_domain_page+0x250/0x527
(XEN)    [] F do_page_fault+0x420/0x780
(XEN)    [] F
x86_64/entry.S#handle_exception_saved+0x68/0x94
(XEN)    [] F __find_next_zero_bit+0x28/0x69
(XEN)    [] F map_domain_page+0x2c6/0x527
(XEN)    [] F nvmx_update_exec_control+0x1d7/0x323
(XEN)    [] F vmx_update_cpu_exec_control+0x23/0x40
(XEN)    [] F
arch/x86/hvm/vmx/vmx.c#vmx_ctxt_switch_from+0xb7/0x121
(XEN)    [] F
arch/x86/domain.c#__context_switch+0x124/0x4a9
(XEN)    [] F context_switch+0x154/0x62c
(XEN)    [] F
common/sched/core.c#sched_context_switch+0x16a/0x175
(XEN)    [] F common/sched/core.c#schedule+0x2ad/0x2bc
(XEN)    [] F common/softirq.c#__do_softirq+0xb7/0xc8
(XEN)    [] F do_softirq+0x18/0x1a
(XEN)    [] F vmx_asm_do_vmentry+0x2b/0x30
(XEN)
(XEN) CPU1 @ e008:82d080252d45 (sched_context_switched+0xa3/0x132)
(XEN) CPU7 @ e008:82d08022d6ae (_spin_lock+0x34/0x5e)
(XEN) CPU3 @ e008:82d08022d6ae (_spin_lock+0x34/0x5e)
(XEN) CPU5 @ e008:82d08022d6ab (_spin_lock+0x31/0x5e)
(XEN) CPU4 @ e008:82d08022d547 (common/spinlock.c#got_lock+0x7/0x23)
(XEN) CPU6 @ e008:82d08025318a
(common/sched/core.c#sched_wait_rendezvous_in+0x241/0x39e)
(XEN) CPU2 @ e008:82d08022d6ae (_spin_lock+0x34/0x5e)
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) FATAL TRAP: vector = 2 (nmi)
(XEN) [error_code=] , IN INTERRUPT CONTEXT
(XEN) 
(XEN)
(XEN) Reboot in five seconds...
(XEN) Executing kexec image on cpu0
(XEN) Shot down all CPUs

This is ultimately cased by c47984aab "nvmx: implement support for MSR
bitmaps", which adds a map_domain_page() call in the middle of
context_switch(), which isn't safe.

Specifically, this is a switch from an HVM vcpu, to a PV vcpu, where the
mapcache code tries to access the per-domain mappings on the HVM monitor
table.  It ends up trying to recursively acquire the mapcache lock while
trying to walk %cr2 to identify the source of the fault.

For nvmx->msr_merged, this needs to either be a xenheap page, or a
globally mapped domheap page.  I'll draft a patch in a moment.

For map_domain_page(), is there anything we can rationally do to assert
that it isn't called in the middle of a context switch?  This is the
kind of thing which needs

[Xen-devel] [XEN PATCH 1/2] tools/python: Fix install-wrap

2020-03-11 Thread Anthony PERARD

This allows to use install-wrap when the source scripts is in a
subdirectory.

Signed-off-by: Anthony PERARD 
---
 tools/python/install-wrap | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/python/install-wrap b/tools/python/install-wrap
index 00e2014016f9..fef24e01708d 100755
--- a/tools/python/install-wrap
+++ b/tools/python/install-wrap
@@ -44,7 +44,7 @@ shift
 destf="$dest"
 for srcf in ${srcs}; do
if test -d "$dest"; then
-   destf="$dest/${srcf%%*/}"
+   destf="$dest/${srcf##*/}"
fi
org="$(sed -n '2q; /^#! *\/usr\/bin\/env python *$/p' $srcf)"
if test "x$org" = x; then
-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 0/2] Fix installation of python scripts

2020-03-11 Thread Anthony PERARD

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git 
br.fix-python-install-v1

Hi,

A patch to make packaging of xen on centos8 easier. rpmbuild
prevents unversions python shebang from been packaged.
And the first patch fix a bug discovered with the second.

Cheers,

Anthony PERARD (2):
  tools/python: Fix install-wrap
  tools: Use INSTALL_PYTHON_PROG

 tools/misc/xencov_split   | 2 +-
 tools/python/Makefile | 4 ++--
 tools/python/install-wrap | 2 +-
 tools/xenmon/Makefile | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 2/2] tools: Use INSTALL_PYTHON_PROG

2020-03-11 Thread Anthony PERARD

Whenever python scripts are install, have the shebang be modified to use
whatever PYTHON_PATH is. This is useful for system where python isn't 
available, or
where the package build tools prevent unversioned shebang.

INSTALL_PYTHON_PROG only looks for "#!/usr/bin/env python".

Signed-off-by: Anthony PERARD 
---
 tools/misc/xencov_split | 2 +-
 tools/python/Makefile   | 4 ++--
 tools/xenmon/Makefile   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
index 1f20518b8637..5771f6cfc285 100755
--- a/tools/misc/xencov_split
+++ b/tools/misc/xencov_split
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 import sys, os, os.path as path, struct, errno
 from optparse import OptionParser
diff --git a/tools/python/Makefile b/tools/python/Makefile
index e99f78a5373a..8d22c0367657 100644
--- a/tools/python/Makefile
+++ b/tools/python/Makefile
@@ -20,8 +20,8 @@ install:
setup.py install --record $(INSTALL_LOG) $(PYTHON_PREFIX_ARG) \
--root="$(DESTDIR)" --force
 
-   $(INSTALL_PROG) scripts/convert-legacy-stream $(DESTDIR)$(LIBEXEC_BIN)
-   $(INSTALL_PROG) scripts/verify-stream-v2 $(DESTDIR)$(LIBEXEC_BIN)
+   $(INSTALL_PYTHON_PROG) scripts/convert-legacy-stream 
$(DESTDIR)$(LIBEXEC_BIN)
+   $(INSTALL_PYTHON_PROG) scripts/verify-stream-v2 $(DESTDIR)$(LIBEXEC_BIN)
 
 .PHONY: uninstall
 uninstall:
diff --git a/tools/xenmon/Makefile b/tools/xenmon/Makefile
index e1712304d07d..3e150b065998 100644
--- a/tools/xenmon/Makefile
+++ b/tools/xenmon/Makefile
@@ -32,7 +32,7 @@ install: build
$(INSTALL_DIR) $(DESTDIR)$(sbindir)
$(INSTALL_PROG) xenbaked $(DESTDIR)$(sbindir)/xenbaked
$(INSTALL_PROG) xentrace_setmask  $(DESTDIR)$(sbindir)/xentrace_setmask
-   $(INSTALL_PROG) xenmon.py  $(DESTDIR)$(sbindir)/xenmon
+   $(INSTALL_PYTHON_PROG) xenmon.py  $(DESTDIR)$(sbindir)/xenmon
 
 .PHONY: uninstall
 uninstall:
-- 
Anthony PERARD


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

Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

2020-03-11 Thread Anthony PERARD

On Wed, Mar 11, 2020 at 05:21:24PM +, Julien Grall wrote:
> On 11/03/2020 15:26, Anthony PERARD wrote:
> > On Wed, Mar 11, 2020 at 02:18:20PM +, Julien Grall wrote:
> > > > +config EARLY_UART_BASE_ADDRESS
> > > > +   depends on EARLY_PRINTK
> > > > +   hex "Early printk, physical base address of debug UART"
> > > > +   default 0x87e02400 if EARLY_PRINTK_THUNDERX
> > > 
> > > You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform 
> > > but
> > > the address is above 4G. I suspect this would break randconfig build.
> > 
> > gcc doesn't seems to complain :-).
> 
> I was expecting GAS to throw an error because the 64-bit value does not fit
> in a 32-bit register. But... it looks like GAS will silently truncate the
> value to 0x2400 :(.
>   
> > (I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")
> > 
> > But I can have thunderx depends on arm_64.
> Is there a way to constrainst the address in Kconfig?

There is! I can add "range 0x0 0x if ARM_32".
But Kconfig doesn't say anything if a default value is too high, and
silently set the value to the maximum.
Still, it's better. I just need to add depends on ARM_64 for thunderx,
and that should be fine.
And that prevent users from setting a too hight value, as kconfig will
not accept a value outside the range.

Thanks,

-- 
Anthony PERARD

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

[Xen-devel] [PATCH OSSTEST v5 5/5] make-flight: add dom0 PVH test

2020-03-11 Thread Roger Pau Monne

Add a dom0 PVH test, the test to be run is launching a PV guest
(test-debian). Note the PVH dom0 tests are only enabled for Xen >=
4.13.

The runvar difference is:

+test-amd64-amd64-dom0pvh-xl-amd   all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-intel all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-amd   all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-intel all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-amd   all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-amd,iommu
+test-amd64-amd64-dom0pvh-xl-intel all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-intel,iommu
+test-amd64-amd64-dom0pvh-xl-amd   archamd64
+test-amd64-amd64-dom0pvh-xl-intel archamd64
+test-amd64-amd64-dom0pvh-xl-amd   buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-intel buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-intel debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-intel debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-amd   debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-intel debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-amd   kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-intel kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-amd   kernkindpvops
+test-amd64-amd64-dom0pvh-xl-intel kernkindpvops
+test-amd64-amd64-dom0pvh-xl-amd   toolstack   xl
+test-amd64-amd64-dom0pvh-xl-intel toolstack   xl
+test-amd64-amd64-dom0pvh-xl-amd   xen_boot_append dom0=pvh,verbose
+test-amd64-amd64-dom0pvh-xl-intel xen_boot_append dom0=pvh,verbose

Signed-off-by: Roger Pau Monné 
Reviewed-by: Ian Jackson 
---
Changes since v1:
 - Request hosts with iommu flag.
---
 make-flight | 24 
 1 file changed, 24 insertions(+)

diff --git a/make-flight b/make-flight
index b08431dc..48f164cc 100755
--- a/make-flight
+++ b/make-flight
@@ -753,6 +753,16 @@ test_matrix_do_one () {
   *)test_shim=y ;;
   esac
 
+  # PVH dom0 tests for versions >= 4.13 only
+  case "$xenbranch" in
+  xen-3.*-testing)  test_dom0pvh=n ;;
+  xen-4.?-testing)  test_dom0pvh=n ;;
+  xen-4.10-testing) test_dom0pvh=n ;;
+  xen-4.11-testing) test_dom0pvh=n ;;
+  xen-4.12-testing) test_dom0pvh=n ;;
+  *)test_dom0pvh=y ;;
+  esac
+
   # xend PV guest test on x86 only
   if [ x$test_xend = xy -a \( $dom0arch = "i386" -o $dom0arch = "amd64" \) ]; 
then
 job_create_test test-$xenarch$kern-$dom0arch-pv test-debian xend \
@@ -861,6 +871,20 @@ test_matrix_do_one () {
 
   fi
 
+  if [ x$test_dom0pvh = xy -a $xenarch = amd64 -a $dom0arch = amd64 ]; then
+
+for cpuvendor in amd intel; do
+
+  job_create_test test-$xenarch$kern-$dom0arch-dom0pvh-xl-$cpuvendor \
+test-debian xl $xenarch $dom0arch $debian_runvars \
+all_hostflags=$most_hostflags,hvm-$cpuvendor,iommu \
+xen_boot_append='dom0=pvh,verbose'
+
+done
+
+  fi
+
+
   if [ x$test_shim = xy -a $xenarch = amd64 ]; then
 
 job_create_test test-$xenarch$kern-$dom0arch-xl-pvshim \
-- 
2.25.0


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

Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

2020-03-11 Thread Julien Grall


Hi Anthony,

On 11/03/2020 15:26, Anthony PERARD wrote:

On Wed, Mar 11, 2020 at 02:18:20PM +, Julien Grall wrote:

diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index 89e081e51eaf..c61973013097 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -1,64 +1,39 @@
   How to enable early printk
-Early printk can only be enabled if debug=y. You may want to enable it if
-you are debbuging code that executes before the console is initialized.
+Early printk can only be enabled if CONFIG_DEBUG=y.  You may want to enable


NIT: AFAICT, the file is using one space after full stop. I would like to
keep it like that for consistency :).


Sound good, I should look at how to fix my vim configuration so it stop
adding extra spaces :-(

:set nojoinspaces

Won't happen again :-).


diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index b3511e81a275..ee6ee33b69be 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -128,6 +128,8 @@ config XMEM_POOL_POISON
  Poison free blocks with 0xAA bytes and verify them when a block is
  allocated in order to spot use-after-free issues.
+source "arch/$(SRCARCH)/Kconfig.debug"


To double check, this means that earlyprintk can be selected in EXPERT mode
now. However, in the document early-printk.txt, the feature is said to only
be enabled with CONFIG_DEBUG=y.

I like the idea of allowing a user to enable earlyprintk in EXPERT mode
(some early boot bug may only occur in non-debug build). So I am happy to
keep the code like. Can you update the doc accordingly?


Will do.


+
   endif # DEBUG || EXPERT
   endmenu
diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
new file mode 100644
index ..ffb21e8ac20a
--- /dev/null
+++ b/xen/arch/arm/Kconfig.debug
@@ -0,0 +1,287 @@
+choice
+   bool "Early printk"
+   optional
+   help
+   You may want to enable early printk if you are debugging code
+   that executes before the console is initialized.
+
+   Note that selecting this option will limit Xen to a single UART
+   definition. Attempting to boot Xen image on a different
+   platform *will not work*, so this option should not be enable
+   for Xens that are intended to be portable.
+
+   Choose one of the UART drivers for early printk, then you'll
+   have to specify the parameters, like the base address.
+
+   Alternatively, there are platform specific options which will
+   have default values for the various parameters.


Would it be worth to mention such options are deprecated?


Yes, I should mention that here. (And probably in the early-printk.txt
doc as well.)

How about:
 Alternatively, there are platform specific options which will
 have default values for the various parameters. But such option are
 deprecated and will soon be removed.

Or it would be better to highlight the fact that they are deprecated, so
maybe the following would be better:
 Deprecated: Alternatively, there are platform specific options which
 will have default values for the various parameters. But such option
 will soon be removed.


The second version looks better to me.


+
+   config EARLY_PRINTK_BRCM
+   bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 
processors"
+   select EARLY_UART_8250


I noticed below you added "depends on ARM_64" on the Xilinx SoC. In general,
platform specific options are tied to either arm32 or arm64, even if the
UART "driver" is arch agnostic.


Those "depends" are only there because the early uart driver is only
available for one arch. "debug-cadence.inc" can only be found in
"arch/arm/arm64/", not in arm32, for example.


You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
support 32-bit at the hypervisor level, but we never supported this case. So
I am wondering whether we should add depends on each earlyprintk. Stefano,
any opinions?


+config EARLY_UART_BASE_ADDRESS
+   depends on EARLY_PRINTK
+   hex "Early printk, physical base address of debug UART"
+   default 0x87e02400 if EARLY_PRINTK_THUNDERX


You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
the address is above 4G. I suspect this would break randconfig build.


gcc doesn't seems to complain :-).


I was expecting GAS to throw an error because the 64-bit value does not 
fit in a 32-bit register. But... it looks like GAS will silently 
truncate the value to 0x2400 :(.



(I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")

But I can have thunderx depends on arm_64.

Is there a way to constrainst the address in Kconfig?

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH OSSTEST v5 4/5] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Roger Pau Monne

Introduce a new test to check for iommu availability and add it as a
hostflag if found.

Signed-off-by: Roger Pau Monné 
---
Changes since v4:
 - Split out code into separate patches.

Changes since v3:
 - Fail if `xl info` command fails.

Changes since v2:
 - Allow flags to be removed.
 - Fix set_flag addition to HostBD/Static.pm.
---
 sg-run-job   |  1 +
 ts-examine-iommu | 32 
 2 files changed, 33 insertions(+)
 create mode 100755 ts-examine-iommu

diff --git a/sg-run-job b/sg-run-job
index 7c58d4ba..f6bfdfd5 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
 if {$ok} {
run-ts -.  =   ts-examine-serial-post + host
run-ts .   =   ts-examine-logs-save   + host
+   run-ts .   =   ts-examine-iommu   + host
run-ts .   =   ts-examine-hostprops-save
 }
 }
diff --git a/ts-examine-iommu b/ts-examine-iommu
new file mode 100755
index ..099d4be5
--- /dev/null
+++ b/ts-examine-iommu
@@ -0,0 +1,32 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2020 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see .
+
+use strict qw(vars);
+BEGIN { unshift @INC, qw(.); }
+use Osstest;
+use Osstest::TestSupport;
+
+tsreadconfig();
+
+our ($whhost) = @ARGV;
+$whhost ||= 'host';
+our $ho= selecthost($whhost);
+our $info = target_cmd_output_root($ho, 'xl info', 10);
+our $has_iommu = $info =~ /directio/;
+
+logm("$ho->{Ident} iommu: $has_iommu");
+hostflag_putative_record($ho, "iommu", $has_iommu);
-- 
2.25.0


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

[Xen-devel] [PATCH OSSTEST v5 1/5] host: introduce modify_host

2020-03-11 Thread Roger Pau Monne

Abstract the set_property checks and DB call into a helper.

No functional change.

Signed-off-by: Roger Pau Monné 
---
Requested on IRC:
17:09:30 Diziet Also if it were me I would put the modify_host refactoring in 
its own nfc patch,
but I won't insist on that...
---
changes since v4:
 - New in this version.
---
 Osstest/HostDB/Executive.pm | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/Osstest/HostDB/Executive.pm b/Osstest/HostDB/Executive.pm
index 7ffca6c4..d402bcac 100644
--- a/Osstest/HostDB/Executive.pm
+++ b/Osstest/HostDB/Executive.pm
@@ -51,6 +51,16 @@ END
 }
 }
 
+sub modify_host ($$$) {
+my ($hd, $ho, $query) = @_;
+my $blessing = intended_blessing();
+
+die "Attempting to modify host with intended blessing $blessing != real"
+if $blessing ne "real";
+
+db_retry($dbh_tests, [qw(resources)], $query);
+}
+
 sub set_property() {
 my ($hd, $ho, $prop, $val) = @_;
 my $rmq = $dbh_tests->prepare({Name}, $prop);
 if (length $val) {
 $addq->execute($ho->{Name}, $prop, $val);
-- 
2.25.0


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

[Xen-devel] [PATCH OSSTEST v5 2/5] host: introduce a helper to modify hostflags

2020-03-11 Thread Roger Pau Monne

Add a generic function to perform database changes related to a host
flag and add a wrapper to TestSupport.

Signed-off-by: Roger Pau Monné 
---
Changes since v4:
 - Move addition of hostflag_putative_record to a different patch.
 - Introduce a single helper in TestSupport: modify_host_flag.

Changes since v3:
 - Introduce modify_flag instead of {set/remove}_flag.
 - Introduce a generic modify_host helper.
 - Split from patch 1.
---
Requested on IRC:
17:08:58 Diziet royger: I think your ts-examine-hostprops-save hunk in 2/ 
belongs in 1/ ?  (Or in
a separate 1.5/ along with hostflag_putative_record.)
---
 Osstest/HostDB/Executive.pm | 17 +
 Osstest/HostDB/Static.pm|  7 +++
 Osstest/TestSupport.pm  |  8 +++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Osstest/HostDB/Executive.pm b/Osstest/HostDB/Executive.pm
index d402bcac..a6dc4462 100644
--- a/Osstest/HostDB/Executive.pm
+++ b/Osstest/HostDB/Executive.pm
@@ -96,6 +96,23 @@ END
 return $flags;
 }
 
+sub modify_flag () {
+my ($hd, $ho, $flag, $set) = @_;
+my $rmq = $dbh_tests->prepare({Name}, $flag);
+if ($set) {
+$addq->execute($ho->{Name}, $flag);
+}
+});
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/HostDB/Static.pm b/Osstest/HostDB/Static.pm
index 0c6be3ee..d0669fb2 100644
--- a/Osstest/HostDB/Static.pm
+++ b/Osstest/HostDB/Static.pm
@@ -72,6 +72,13 @@ sub get_flags ($$) { #method
 return $flags;
 }
 
+sub modify_flag () {
+my ($hd, $ho, $flag, $set) = @_;
+
+die
+"Cannot modify flags in standalone mode for $ho->{Name} $flag set: $set\n";
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index f49ed529..ceb6bb7b 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -84,7 +84,7 @@ BEGIN {
   get_target_property get_host_native_linux_console
   hostnamepath hostnamepath_list set_runtime_hostflag
   power_state power_cycle power_reboot_attempts
-  serial_fetch_logs set_host_property
+  serial_fetch_logs set_host_property modify_host_flag
   propname_massage propname_check
   hostprop_putative_record
  
@@ -1411,6 +1411,12 @@ sub hostprop_putative_record ($$$) {
 store_runvar("hostprop/$ho->{Ident}/$prop", $val);
 }
 
+sub modify_host_flag ($$$) {
+my ($ho, $flag, $set) = @_;
+
+$mhostdb->modify_flag($ho, $flag, $set);
+}
+
 sub get_target_property ($$;$);
 sub get_target_property ($$;$) {
 my ($ho, $prop, $defval) = @_;
-- 
2.25.0


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

[Xen-devel] [PATCH OSSTEST v5 3/5] ts-examine-hostprops-save: record hostflags also

2020-03-11 Thread Roger Pau Monne

Commit putative hotflags into the database if present on the runvars.

Signed-off-by: Roger Pau Monné 
---
Changes since v4:
 - New in this version.
---
Requested by Ian on IRC:
17:08:58 Diziet royger: I think your ts-examine-hostprops-save hunk in 2/ 
belongs in 1/ ?  (Or in
a separate 1.5/ along with hostflag_putative_record.)
---
 Osstest/TestSupport.pm|  8 +++-
 ts-examine-hostprops-save | 23 ++-
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index ceb6bb7b..1c13e2af 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -86,7 +86,7 @@ BEGIN {
   power_state power_cycle power_reboot_attempts
   serial_fetch_logs set_host_property modify_host_flag
   propname_massage propname_check
-  hostprop_putative_record
+  hostprop_putative_record hostflag_putative_record
  
   get_stashed open_unique_stashfile compress_stashed
   dir_identify_vcs
@@ -1417,6 +1417,12 @@ sub modify_host_flag ($$$) {
 $mhostdb->modify_flag($ho, $flag, $set);
 }
 
+sub hostflag_putative_record ($$$) {
+my ($ho, $prop, $set) = @_;
+
+store_runvar("hostflag/$ho->{Ident}/$prop", !!$set);
+}
+
 sub get_target_property ($$;$);
 sub get_target_property ($$;$) {
 my ($ho, $prop, $defval) = @_;
diff --git a/ts-examine-hostprops-save b/ts-examine-hostprops-save
index 55d23392..e50ea7fb 100755
--- a/ts-examine-hostprops-save
+++ b/ts-examine-hostprops-save
@@ -27,20 +27,25 @@ tsreadconfig();
 
 our $blessing = intended_blessing();
 
-logm("setting host properties");
+logm("setting host properties and flags");
 
 # NB: in order to aid debug only attempt to save the host props on flights
 # with intended real blessing, for the rest just do a dry run.
 our $dry_run = $blessing ne "real";
-logm("not saving host props with intended blessing $blessing != real")
+logm("not saving host props/flags with intended blessing $blessing != real")
 if $dry_run;
 
 foreach my $k (sort keys %r) {
-next unless $k =~ m/^hostprop\/([^\/]*)\/([^\/]*)$/;
-my $ho = selecthost($1);
-my $prop = $2;
-
-logm("recording for $ho->{Name} $prop=$r{$k}");
-
-set_host_property($ho, $prop, $r{$k}) if !$dry_run;
+next unless $k =~ m/^host(prop|flag)\/([^\/]*)\/([^\/]*)$/;
+my $type = $1;
+my $ho = selecthost($2);
+my $prop = $3;
+
+if ($type eq "flag") {
+logm("recording flag $prop set: $r{$k} for $ho->{Name}");
+modify_host_flag($ho, $prop, !!$r{$k}) if !$dry_run;
+} else {
+logm("recording prop for $ho->{Name} $prop=$r{$k}");
+set_host_property($ho, $prop, $r{$k}) if !$dry_run;
+}
 }
-- 
2.25.0


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

[Xen-devel] [xen-unstable-smoke test] 148436: regressions - FAIL

2020-03-11 Thread osstest service owner

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 18 guest-start/debian.repeatfail  like 148393
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  a9b6dacf88fe99fbb69a2ee505833851ffdc9cec
baseline version:
 xen  e19d3a942e4b6f6c5b19287a4a6f5020bdab2936

Last test of basis   148408  2020-03-11 03:01:39 Z0 days
Testing same since   148436  2020-03-11 13:00:35 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 

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-amd64fail
 test-amd64-amd64-libvirt fail



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

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

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

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


Not pushing.


commit a9b6dacf88fe99fbb69a2ee505833851ffdc9cec
Author: Juergen Gross 
Date:   Wed Mar 11 13:18:49 2020 +0100

rcu: fix rcu_lock_domain()

rcu_lock_domain() misuses the domain structure as rcu lock, which is
working only as long as rcu_read_lock() isn't evaluating the lock.

Fix that by adding a rcu lock to struct domain and use that for
rcu_lock_domain().

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 

commit 53ddfc80a84a6e4b46531ada092f66839661ee8a
Author: Juergen Gross 
Date:   Wed Mar 11 13:17:41 2020 +0100

rcu: use rcu softirq for forcing quiescent state

As rcu callbacks are processed in __do_softirq() there is no need to
use the scheduling softirq for forcing quiescent state. Any other
softirq would do the job and the scheduling one is the most expensive.

So use the already existing rcu softirq for that purpose. For telling
apart why the rcu softirq was raised add a flag for the current usage.

Signed-off-by: Juergen Gross 
Acked-by: Andrew Cooper 
(qemu changes not included)

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

Re: [Xen-devel] [PATCH v5 2/2] x86: add accessors for scratch cpu mask

2020-03-11 Thread Jan Beulich

On 11.03.2020 16:51, Roger Pau Monné wrote:
> On Wed, Mar 11, 2020 at 04:37:50PM +0100, Jan Beulich wrote:
>> On 11.03.2020 16:34, Roger Pau Monné wrote:
>>> On Fri, Feb 28, 2020 at 01:42:58PM +0100, Jan Beulich wrote:
 On 28.02.2020 13:07, Roger Pau Monne wrote:
> Current usage of the per-CPU scratch cpumask is dangerous since
> there's no way to figure out if the mask is already being used except
> for manual code inspection of all the callers and possible call paths.
>
> This is unsafe and not reliable, so introduce a minimal get/put
> infrastructure to prevent nested usage of the scratch mask and usage
> in interrupt context.
>
> Move the declaration of scratch_cpumask to smp.c in order to place the
> declaration and the accessors as close as possible.
>
> Signed-off-by: Roger Pau Monné 

 Reviewed-by: Jan Beulich 
>>>
>>> Ping? This seems to have the required RB, but hasn't been committed.
>>
>> While as per the R-b this technically is fine, I continue to be
>> uncertain whether we actually want to go this far.
> 
> If this had been in place 5500d265a2a8fa6 ('x86/smp: use APIC ALLBUT
> destination shorthand when possible') wouldn't have introduced a
> bogus usage of the scratch per cpu mask, as the check would have
> triggered.
> 
> After finding that one of my commits introduced a bug I usually do the
> exercise of trying to figure out which checks or safeguards would have
> prevented it, and hence came up with this patch.
> 
> I would also like to note that this adds 0 overhead to non-debug
> builds.
> 
>> Andrew, as
>> per a discussion we had when I was pondering whether to commit
>> this, also looks to have similar concerns (which iirc he said he
>> had voiced on irc).
> 
> Is the concern only related to the fact that you have to use the
> get/put accessors and thus more lines of code are added, or is there
> something else?

Afaic - largely this, along with it making it more likely that
error paths will be non-trivial (and hence possibly get converted
to use goto-s). I can't speak for Andrew, of course.

Jan

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

Re: [Xen-devel] [PATCH OSSTEST v4 3/3] make-flight: add dom0 PVH test

2020-03-11 Thread Ian Jackson

Roger Pau Monne writes ("[PATCH OSSTEST v4 3/3] make-flight: add dom0 PVH 
test"):
> Add a dom0 PVH test, the test to be run is launching a PV guest
> (test-debian). Note the PVH dom0 tests are only enabled for Xen >=
> 4.13.

Reviewed-by: Ian Jackson 

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

[Xen-devel] [PATCH OSSTEST v4 1/3] host: introduce helpers to modify hostflags

2020-03-11 Thread Roger Pau Monne

Add a generic function to perform database changes related to a host
and use it to implement set_property and and {set/remove}_flag.

Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - Introduce modify_flag instead of {set/remove}_flag.
 - Introduce a generic modify_host helper.
 - Split from patch 1.
---
 Osstest/HostDB/Executive.pm | 33 -
 Osstest/HostDB/Static.pm|  7 +++
 Osstest/TestSupport.pm  | 21 -
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/Osstest/HostDB/Executive.pm b/Osstest/HostDB/Executive.pm
index 7ffca6c4..a6dc4462 100644
--- a/Osstest/HostDB/Executive.pm
+++ b/Osstest/HostDB/Executive.pm
@@ -51,6 +51,16 @@ END
 }
 }
 
+sub modify_host ($$$) {
+my ($hd, $ho, $query) = @_;
+my $blessing = intended_blessing();
+
+die "Attempting to modify host with intended blessing $blessing != real"
+if $blessing ne "real";
+
+db_retry($dbh_tests, [qw(resources)], $query);
+}
+
 sub set_property() {
 my ($hd, $ho, $prop, $val) = @_;
 my $rmq = $dbh_tests->prepare({Name}, $prop);
 if (length $val) {
 $addq->execute($ho->{Name}, $prop, $val);
@@ -90,6 +96,23 @@ END
 return $flags;
 }
 
+sub modify_flag () {
+my ($hd, $ho, $flag, $set) = @_;
+my $rmq = $dbh_tests->prepare({Name}, $flag);
+if ($set) {
+$addq->execute($ho->{Name}, $flag);
+}
+});
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/HostDB/Static.pm b/Osstest/HostDB/Static.pm
index 0c6be3ee..d0669fb2 100644
--- a/Osstest/HostDB/Static.pm
+++ b/Osstest/HostDB/Static.pm
@@ -72,6 +72,13 @@ sub get_flags ($$) { #method
 return $flags;
 }
 
+sub modify_flag () {
+my ($hd, $ho, $flag, $set) = @_;
+
+die
+"Cannot modify flags in standalone mode for $ho->{Name} $flag set: $set\n";
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index f49ed529..b80e89bc 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -85,8 +85,9 @@ BEGIN {
   hostnamepath hostnamepath_list set_runtime_hostflag
   power_state power_cycle power_reboot_attempts
   serial_fetch_logs set_host_property
+  set_host_flag remove_host_flag
   propname_massage propname_check
-  hostprop_putative_record
+  hostprop_putative_record hostflag_putative_record
  
   get_stashed open_unique_stashfile compress_stashed
   dir_identify_vcs
@@ -1411,6 +1412,24 @@ sub hostprop_putative_record ($$$) {
 store_runvar("hostprop/$ho->{Ident}/$prop", $val);
 }
 
+sub set_host_flag ($$) {
+my ($ho,$flag) = @_;
+
+$mhostdb->modify_flag($ho, $flag, 1);
+}
+
+sub remove_host_flag ($$) {
+my ($ho,$flag) = @_;
+
+$mhostdb->modify_flag($ho, $flag, 0);
+}
+
+sub hostflag_putative_record ($$$) {
+my ($ho, $prop, $set) = @_;
+
+store_runvar("hostflag/$ho->{Ident}/$prop", !!$set);
+}
+
 sub get_target_property ($$;$);
 sub get_target_property ($$;$) {
 my ($ho, $prop, $defval) = @_;
-- 
2.25.0


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

[Xen-devel] [PATCH OSSTEST v4 3/3] make-flight: add dom0 PVH test

2020-03-11 Thread Roger Pau Monne

Add a dom0 PVH test, the test to be run is launching a PV guest
(test-debian). Note the PVH dom0 tests are only enabled for Xen >=
4.13.

The runvar difference is:

+test-amd64-amd64-dom0pvh-xl-amd   all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-intel all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-amd   all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-intel all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-amd   all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-amd,iommu
+test-amd64-amd64-dom0pvh-xl-intel all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-intel,iommu
+test-amd64-amd64-dom0pvh-xl-amd   archamd64
+test-amd64-amd64-dom0pvh-xl-intel archamd64
+test-amd64-amd64-dom0pvh-xl-amd   buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-intel buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-intel debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-intel debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-amd   debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-intel debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-amd   kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-intel kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-amd   kernkindpvops
+test-amd64-amd64-dom0pvh-xl-intel kernkindpvops
+test-amd64-amd64-dom0pvh-xl-amd   toolstack   xl
+test-amd64-amd64-dom0pvh-xl-intel toolstack   xl
+test-amd64-amd64-dom0pvh-xl-amd   xen_boot_append dom0=pvh,verbose
+test-amd64-amd64-dom0pvh-xl-intel xen_boot_append dom0=pvh,verbose

Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Request hosts with iommu flag.
---
 make-flight | 24 
 1 file changed, 24 insertions(+)

diff --git a/make-flight b/make-flight
index b08431dc..48f164cc 100755
--- a/make-flight
+++ b/make-flight
@@ -753,6 +753,16 @@ test_matrix_do_one () {
   *)test_shim=y ;;
   esac
 
+  # PVH dom0 tests for versions >= 4.13 only
+  case "$xenbranch" in
+  xen-3.*-testing)  test_dom0pvh=n ;;
+  xen-4.?-testing)  test_dom0pvh=n ;;
+  xen-4.10-testing) test_dom0pvh=n ;;
+  xen-4.11-testing) test_dom0pvh=n ;;
+  xen-4.12-testing) test_dom0pvh=n ;;
+  *)test_dom0pvh=y ;;
+  esac
+
   # xend PV guest test on x86 only
   if [ x$test_xend = xy -a \( $dom0arch = "i386" -o $dom0arch = "amd64" \) ]; 
then
 job_create_test test-$xenarch$kern-$dom0arch-pv test-debian xend \
@@ -861,6 +871,20 @@ test_matrix_do_one () {
 
   fi
 
+  if [ x$test_dom0pvh = xy -a $xenarch = amd64 -a $dom0arch = amd64 ]; then
+
+for cpuvendor in amd intel; do
+
+  job_create_test test-$xenarch$kern-$dom0arch-dom0pvh-xl-$cpuvendor \
+test-debian xl $xenarch $dom0arch $debian_runvars \
+all_hostflags=$most_hostflags,hvm-$cpuvendor,iommu \
+xen_boot_append='dom0=pvh,verbose'
+
+done
+
+  fi
+
+
   if [ x$test_shim = xy -a $xenarch = amd64 ]; then
 
 job_create_test test-$xenarch$kern-$dom0arch-xl-pvshim \
-- 
2.25.0


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

[Xen-devel] [PATCH OSSTEST v4 2/3] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Roger Pau Monne

Introduce a new test to check for iommu availability and add it as a
hostflag if found.

Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - Fail if `xl info` command fails.

Changes since v2:
 - Allow flags to be removed.
 - Fix set_flag addition to HostBD/Static.pm.
---
 sg-run-job|  1 +
 ts-examine-hostprops-save | 28 +++-
 ts-examine-iommu  | 32 
 3 files changed, 52 insertions(+), 9 deletions(-)
 create mode 100755 ts-examine-iommu

diff --git a/sg-run-job b/sg-run-job
index 7c58d4ba..f6bfdfd5 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
 if {$ok} {
run-ts -.  =   ts-examine-serial-post + host
run-ts .   =   ts-examine-logs-save   + host
+   run-ts .   =   ts-examine-iommu   + host
run-ts .   =   ts-examine-hostprops-save
 }
 }
diff --git a/ts-examine-hostprops-save b/ts-examine-hostprops-save
index 55d23392..b1c8de6d 100755
--- a/ts-examine-hostprops-save
+++ b/ts-examine-hostprops-save
@@ -27,20 +27,30 @@ tsreadconfig();
 
 our $blessing = intended_blessing();
 
-logm("setting host properties");
+logm("setting host properties and flags");
 
 # NB: in order to aid debug only attempt to save the host props on flights
 # with intended real blessing, for the rest just do a dry run.
 our $dry_run = $blessing ne "real";
-logm("not saving host props with intended blessing $blessing != real")
+logm("not saving host props/flags with intended blessing $blessing != real")
 if $dry_run;
 
 foreach my $k (sort keys %r) {
-next unless $k =~ m/^hostprop\/([^\/]*)\/([^\/]*)$/;
-my $ho = selecthost($1);
-my $prop = $2;
-
-logm("recording for $ho->{Name} $prop=$r{$k}");
-
-set_host_property($ho, $prop, $r{$k}) if !$dry_run;
+next unless $k =~ m/^host(prop|flag)\/([^\/]*)\/([^\/]*)$/;
+my $type = $1;
+my $ho = selecthost($2);
+my $prop = $3;
+
+if ($type eq "flag") {
+if ($r{$k} && !$dry_run) {
+logm("recording flag $prop for $ho->{Name}");
+set_host_flag($ho, $prop);
+} elsif (!$dry_run) {
+logm("removing flag $prop for $ho->{Name}");
+remove_host_flag($ho, $prop);
+}
+} else {
+logm("recording prop for $ho->{Name} $prop=$r{$k}");
+set_host_property($ho, $prop, $r{$k}) if !$dry_run;
+}
 }
diff --git a/ts-examine-iommu b/ts-examine-iommu
new file mode 100755
index ..099d4be5
--- /dev/null
+++ b/ts-examine-iommu
@@ -0,0 +1,32 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2020 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see .
+
+use strict qw(vars);
+BEGIN { unshift @INC, qw(.); }
+use Osstest;
+use Osstest::TestSupport;
+
+tsreadconfig();
+
+our ($whhost) = @ARGV;
+$whhost ||= 'host';
+our $ho= selecthost($whhost);
+our $info = target_cmd_output_root($ho, 'xl info', 10);
+our $has_iommu = $info =~ /directio/;
+
+logm("$ho->{Ident} iommu: $has_iommu");
+hostflag_putative_record($ho, "iommu", $has_iommu);
-- 
2.25.0


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

Re: [Xen-devel] [PATCH v5 2/2] x86: add accessors for scratch cpu mask

2020-03-11 Thread Roger Pau Monné

On Wed, Mar 11, 2020 at 04:37:50PM +0100, Jan Beulich wrote:
> On 11.03.2020 16:34, Roger Pau Monné wrote:
> > On Fri, Feb 28, 2020 at 01:42:58PM +0100, Jan Beulich wrote:
> >> On 28.02.2020 13:07, Roger Pau Monne wrote:
> >>> Current usage of the per-CPU scratch cpumask is dangerous since
> >>> there's no way to figure out if the mask is already being used except
> >>> for manual code inspection of all the callers and possible call paths.
> >>>
> >>> This is unsafe and not reliable, so introduce a minimal get/put
> >>> infrastructure to prevent nested usage of the scratch mask and usage
> >>> in interrupt context.
> >>>
> >>> Move the declaration of scratch_cpumask to smp.c in order to place the
> >>> declaration and the accessors as close as possible.
> >>>
> >>> Signed-off-by: Roger Pau Monné 
> >>
> >> Reviewed-by: Jan Beulich 
> > 
> > Ping? This seems to have the required RB, but hasn't been committed.
> 
> While as per the R-b this technically is fine, I continue to be
> uncertain whether we actually want to go this far.

If this had been in place 5500d265a2a8fa6 ('x86/smp: use APIC ALLBUT
destination shorthand when possible') wouldn't have introduced a
bogus usage of the scratch per cpu mask, as the check would have
triggered.

After finding that one of my commits introduced a bug I usually do the
exercise of trying to figure out which checks or safeguards would have
prevented it, and hence came up with this patch.

I would also like to note that this adds 0 overhead to non-debug
builds.

> Andrew, as
> per a discussion we had when I was pondering whether to commit
> this, also looks to have similar concerns (which iirc he said he
> had voiced on irc).

Is the concern only related to the fact that you have to use the
get/put accessors and thus more lines of code are added, or is there
something else?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v5 2/2] x86: add accessors for scratch cpu mask

2020-03-11 Thread Jan Beulich

On 11.03.2020 16:34, Roger Pau Monné wrote:
> On Fri, Feb 28, 2020 at 01:42:58PM +0100, Jan Beulich wrote:
>> On 28.02.2020 13:07, Roger Pau Monne wrote:
>>> Current usage of the per-CPU scratch cpumask is dangerous since
>>> there's no way to figure out if the mask is already being used except
>>> for manual code inspection of all the callers and possible call paths.
>>>
>>> This is unsafe and not reliable, so introduce a minimal get/put
>>> infrastructure to prevent nested usage of the scratch mask and usage
>>> in interrupt context.
>>>
>>> Move the declaration of scratch_cpumask to smp.c in order to place the
>>> declaration and the accessors as close as possible.
>>>
>>> Signed-off-by: Roger Pau Monné 
>>
>> Reviewed-by: Jan Beulich 
> 
> Ping? This seems to have the required RB, but hasn't been committed.

While as per the R-b this technically is fine, I continue to be
uncertain whether we actually want to go this far. Andrew, as
per a discussion we had when I was pondering whether to commit
this, also looks to have similar concerns (which iirc he said he
had voiced on irc).

Jan

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

Re: [Xen-devel] [PATCH v5 2/2] x86: add accessors for scratch cpu mask

2020-03-11 Thread Roger Pau Monné

On Fri, Feb 28, 2020 at 01:42:58PM +0100, Jan Beulich wrote:
> On 28.02.2020 13:07, Roger Pau Monne wrote:
> > Current usage of the per-CPU scratch cpumask is dangerous since
> > there's no way to figure out if the mask is already being used except
> > for manual code inspection of all the callers and possible call paths.
> > 
> > This is unsafe and not reliable, so introduce a minimal get/put
> > infrastructure to prevent nested usage of the scratch mask and usage
> > in interrupt context.
> > 
> > Move the declaration of scratch_cpumask to smp.c in order to place the
> > declaration and the accessors as close as possible.
> > 
> > Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Jan Beulich 

Ping? This seems to have the required RB, but hasn't been committed.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 1/9] x86/HVM: reduce domain.h include dependencies

2020-03-11 Thread Andrew Cooper

On 11/03/2020 15:22, Jan Beulich wrote:
> On 11.03.2020 14:09, Andrew Cooper wrote:
>> On 10/03/2020 15:48, Jan Beulich wrote:
>>> Drop #include-s not needed by the header itself. Put the ones needed
>>> into whichever other files actually need them.
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> v2: Also make things build with XSM=y.
>> Looking better, but still got problems.
>>
>> xen_pv_console.c: In function ‘pv_console_init’:
>> xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first
>> use in this function)
>>  r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, _pfn);
>>  ^
>>
>> and
>>
>> shim.c: In function ‘pv_shim_fixup_e820’:
>> shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in
>> this function)
>>  MARK_PARAM_RAM(HVM_PARAM_STORE_PFN);
> Oh, so that's an XSM+shim config aiui; Among the sets of what
> I regularly test I have only an XSM one and a shim one. The funs
> of allowing too wide a variety of different configs ... In cases
> like this series here I really don't see how one is supposed to
> cover _all_ possible configs; randconfig builds won't help with
> this unless one would let them run until they've covered all
> possible ones.

This happens to be my default config.

I wouldn't expect to be perfect even under weird combinations, but these
aren't weird, and this is the kind of change that `make allyesconfig` is
intended for.

~Andrew

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

Re: [Xen-devel] [PATCH v5 6/6] domain: use PGC_extra domheap page for shared_info

2020-03-11 Thread Paul Durrant

> -Original Message-
> From: Jan Beulich 
> Sent: 11 March 2020 09:17
> To: p...@xen.org
> Cc: xen-devel@lists.xenproject.org; 'Paul Durrant' ; 
> 'Stefano Stabellini'
> ; 'Julien Grall' ; 'Volodymyr Babchuk'
> ; 'Andrew Cooper' ; 
> 'George Dunlap'
> ; 'Ian Jackson' ; 
> 'Konrad Rzeszutek Wilk'
> ; 'Wei Liu' 
> Subject: Re: [PATCH v5 6/6] domain: use PGC_extra domheap page for shared_info
> 
> On 10.03.2020 18:33, Paul Durrant wrote:
> >> -Original Message-
> >> From: Jan Beulich 
> >> Sent: 09 March 2020 15:56
> >>
> >> On 09.03.2020 11:23, p...@xen.org wrote:
> >>> --- a/xen/common/time.c
> >>> +++ b/xen/common/time.c
> >>> @@ -99,6 +99,18 @@ void update_domain_wallclock_time(struct domain *d)
> >>>  uint32_t *wc_version;
> >>>  uint64_t sec;
> >>>
> >>> +if ( d != current->domain )
> >>> +{
> >>> +/*
> >>> + * We need to check is_dying here as, if it is set, the
> >>> + * shared_info may have been freed. To do this safely we need
> >>> + * hold the domain lock.
> >>> + */
> >>> +domain_lock(d);
> >>> +if ( d->is_dying )
> >>> +goto unlock;
> >>> +}
> >>
> >> This shouldn't happen very often, but it's pretty heavy a lock.
> >> It's a fundamental aspect of xenheap pages that their disposal
> >> can b e delay until almost the last moment of guest cleanup. I
> >> continue to think it's not really a good ideal to have special
> >> purpose allocation (and mapping) accompanied by these pages
> >> getting taken care of by the generic relinquish-resources logic
> >> here (from a more general pov such is of course often nice to
> >> have). Instead of freeing these pages there, couldn't they just
> >> be taken off d->page_list, with the unmapping and freeing left
> >> as it was?
> >
> > I don't think this can be achieved without being able de-assign
> > pages and I don't really want to have to invent new logic to do
> > that (basically re-implementing what happens to xenheap pages).
> 
> Where's the connection to being able to de-assign pages here?
> There'll be one when the same conversion is to be done for
> gnttab code, but I don't see it here - the shared info page is
> never to be de-assigned. As to gnttab code, I think it was
> noted before that we may be better off not "unpopulating"
> status pages when switching back from v2 to v1. At which point
> the de-assignment need would go away there, too.

Ok, maybe I'm misunderstanding something then. We need to call 
free_domheap_pages() on all pages assigned to a domain so that the domain 
references get dropped. The xenpage ref is dropped when d->xenheap_pages == 0. 
The domheap ref is dropped when domain_adjust_tot_pages() returns zero. (This 
is what I meant by de-assigning... but that was probably a poor choice of 
words). So, because domain_adjust_tot_pages() returns d->tot_pages (which 
includes the extra_pages count) it won't fall to zero until the last put_page() 
on any PGC_extra page. So how is it possible to free shared_info in domain 
destroy? We'll never get that far, because the domheap ref will never get 
dropped. I guess this could be fixed by having domain_adjust_tot_pages() return 
the same values as domain_tot_pages() (i.e. tot_pages - extra_pages). Is that 
what you're suggesting?

> 
> > I really don't think it is that bad to deal with shared info
> > and grant table pages as domheap pages. Yes, we have to be
> > careful, but in this case the lock is only taken in a
> > toolstack update of the wallclock and, apart from start of
> > day access, I think this is limited to XEN_DOMCTL_settimeoffset
> > and XEN_DOMCTL_setvcpucontext neither of which I believe is
> > particularly performance-critical.
> 
> It's not, I agree (and hence I had started my previous reply
> also with "This shouldn't happen very often"). How all of this
> is going to look like with the new extra_page_list I'll have
> to see anyway. But for now I remain unconvinced of the want /
> need to de-allocate the shared info page early.
> 

Well hopefully I've explained above why that is currently necessary if it 
becomes a domheap page.

  Paul



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

Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

2020-03-11 Thread Anthony PERARD

On Wed, Mar 11, 2020 at 02:18:20PM +, Julien Grall wrote:
> > diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> > index 89e081e51eaf..c61973013097 100644
> > --- a/docs/misc/arm/early-printk.txt
> > +++ b/docs/misc/arm/early-printk.txt
> > @@ -1,64 +1,39 @@
> >   How to enable early printk
> > -Early printk can only be enabled if debug=y. You may want to enable it if
> > -you are debbuging code that executes before the console is initialized.
> > +Early printk can only be enabled if CONFIG_DEBUG=y.  You may want to enable
> 
> NIT: AFAICT, the file is using one space after full stop. I would like to
> keep it like that for consistency :).

Sound good, I should look at how to fix my vim configuration so it stop
adding extra spaces :-(

:set nojoinspaces

Won't happen again :-).

> > diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> > index b3511e81a275..ee6ee33b69be 100644
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -128,6 +128,8 @@ config XMEM_POOL_POISON
> >   Poison free blocks with 0xAA bytes and verify them when a block is
> >   allocated in order to spot use-after-free issues.
> > +source "arch/$(SRCARCH)/Kconfig.debug"
> 
> To double check, this means that earlyprintk can be selected in EXPERT mode
> now. However, in the document early-printk.txt, the feature is said to only
> be enabled with CONFIG_DEBUG=y.
> 
> I like the idea of allowing a user to enable earlyprintk in EXPERT mode
> (some early boot bug may only occur in non-debug build). So I am happy to
> keep the code like. Can you update the doc accordingly?

Will do.

> > +
> >   endif # DEBUG || EXPERT
> >   endmenu
> > diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> > new file mode 100644
> > index ..ffb21e8ac20a
> > --- /dev/null
> > +++ b/xen/arch/arm/Kconfig.debug
> > @@ -0,0 +1,287 @@
> > +choice
> > +   bool "Early printk"
> > +   optional
> > +   help
> > +   You may want to enable early printk if you are debugging code
> > +   that executes before the console is initialized.
> > +
> > +   Note that selecting this option will limit Xen to a single UART
> > +   definition. Attempting to boot Xen image on a different
> > +   platform *will not work*, so this option should not be enable
> > +   for Xens that are intended to be portable.
> > +
> > +   Choose one of the UART drivers for early printk, then you'll
> > +   have to specify the parameters, like the base address.
> > +
> > +   Alternatively, there are platform specific options which will
> > +   have default values for the various parameters.
> 
> Would it be worth to mention such options are deprecated?

Yes, I should mention that here. (And probably in the early-printk.txt
doc as well.)

How about:
Alternatively, there are platform specific options which will
have default values for the various parameters. But such option are
deprecated and will soon be removed.

Or it would be better to highlight the fact that they are deprecated, so
maybe the following would be better:
Deprecated: Alternatively, there are platform specific options which
will have default values for the various parameters. But such option
will soon be removed.



> > +
> > +   config EARLY_PRINTK_BRCM
> > +   bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 
> > processors"
> > +   select EARLY_UART_8250
> 
> I noticed below you added "depends on ARM_64" on the Xilinx SoC. In general,
> platform specific options are tied to either arm32 or arm64, even if the
> UART "driver" is arch agnostic.

Those "depends" are only there because the early uart driver is only
available for one arch. "debug-cadence.inc" can only be found in
"arch/arm/arm64/", not in arm32, for example.

> You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
> support 32-bit at the hypervisor level, but we never supported this case. So
> I am wondering whether we should add depends on each earlyprintk. Stefano,
> any opinions?
> 
> > +config EARLY_UART_BASE_ADDRESS
> > +   depends on EARLY_PRINTK
> > +   hex "Early printk, physical base address of debug UART"
> > +   default 0x87e02400 if EARLY_PRINTK_THUNDERX
> 
> You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
> the address is above 4G. I suspect this would break randconfig build.

gcc doesn't seems to complain :-).

(I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")

But I can have thunderx depends on arm_64.

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v2 1/9] x86/HVM: reduce domain.h include dependencies

2020-03-11 Thread Jan Beulich

On 11.03.2020 14:09, Andrew Cooper wrote:
> On 10/03/2020 15:48, Jan Beulich wrote:
>> Drop #include-s not needed by the header itself. Put the ones needed
>> into whichever other files actually need them.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> v2: Also make things build with XSM=y.
> 
> Looking better, but still got problems.
> 
> xen_pv_console.c: In function ‘pv_console_init’:
> xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first
> use in this function)
>  r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, _pfn);
>  ^
> 
> and
> 
> shim.c: In function ‘pv_shim_fixup_e820’:
> shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in
> this function)
>  MARK_PARAM_RAM(HVM_PARAM_STORE_PFN);

Oh, so that's an XSM+shim config aiui; Among the sets of what
I regularly test I have only an XSM one and a shim one. The funs
of allowing too wide a variety of different configs ... In cases
like this series here I really don't see how one is supposed to
cover _all_ possible configs; randconfig builds won't help with
this unless one would let them run until they've covered all
possible ones.

Jan

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

Re: [Xen-devel] [PATCH v4 3/3] golang/xenlight: Implement DomainCreateNew

2020-03-11 Thread George Dunlap

On 3/11/20 2:59 PM, Nick Rosbrook wrote:
> Looks good, I just have two small comments:
> 
>> diff --git a/tools/golang/xenlight/xenlight.go 
>> b/tools/golang/xenlight/xenlight.go
>> index 56fa31fd7b..808b4a327c 100644
>> --- a/tools/golang/xenlight/xenlight.go
>> +++ b/tools/golang/xenlight/xenlight.go
>> @@ -,3 +,24 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid 
>> uint32) (path string, err error)
>> path = C.GoString(cpath)
>> return
>>  }
>> +
>> +// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config 
>> *d_config,
>> +// uint32_t *domid,
>> +// const libxl_asyncop_how *ao_how,
>> +// const libxl_asyncprogress_how 
>> *aop_console_how)
> 
> Conventionally, we want to have comments for exported functions along
> the lines of:
> 
> // DomainCreateNew creates a new domain with config, and returns
> its Domid on success.
> // A non-nil error is returned if config cannot be marshaled, or
> an error occurs within libxl.
> 
> Besides being easier to read, it makes documentation more clear on
> godoc/pkg.go.dev.

Yes, absolutely, that's something we need to change before we declare
"1.0".  But that should probably be done in a series which changes all
such comments together.

>> +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> 
> Capitalizing "Ctx" here is a little weird to me. Since it's only the
> receiver name, there's no effect, but since capitalized identifiers
> have special-meaning in other contexts, I would avoid doing this.

I c'd this from another method and just changed the signature.  It
probably would be good to make all of them lower-case.  I may change
this one on check in though.

> I only point those out in case you want to change it on check-in. Besides 
> that,
> 
> Reviewed-by: Nick Rosbrook 

Thanks!

 -George

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

Re: [Xen-devel] [PATCH v4 3/3] golang/xenlight: Implement DomainCreateNew

2020-03-11 Thread Nick Rosbrook

Looks good, I just have two small comments:

> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index 56fa31fd7b..808b4a327c 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -,3 +,24 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid uint32) 
> (path string, err error)
> path = C.GoString(cpath)
> return
>  }
> +
> +// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
> +// uint32_t *domid,
> +// const libxl_asyncop_how *ao_how,
> +// const libxl_asyncprogress_how 
> *aop_console_how)

Conventionally, we want to have comments for exported functions along
the lines of:

// DomainCreateNew creates a new domain with config, and returns
its Domid on success.
// A non-nil error is returned if config cannot be marshaled, or
an error occurs within libxl.

Besides being easier to read, it makes documentation more clear on
godoc/pkg.go.dev.

> +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {

Capitalizing "Ctx" here is a little weird to me. Since it's only the
receiver name, there's no effect, but since capitalized identifiers
have special-meaning in other contexts, I would avoid doing this.

I only point those out in case you want to change it on check-in. Besides that,

Reviewed-by: Nick Rosbrook 

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 17:41, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


11.03.2020 12:38, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

   scripts: Coccinelle script to use auto-propagated errp

or

   scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:

[...]

+// Note, that we update everything related to matched by rule1 function name
+// and local_err name. We may match something not related to the pattern
+// matched by rule1. For example, local_err may be defined with the same name
+// in different blocks inside one function, and in one block follow the
+// propagation pattern and in other block doesn't. Or we may have several
+// functions with the same name (for different configurations).


Context: rule1 matches functions that have all three of

* an Error **errp parameter

* an Error *local_err = NULL variable declaration

* an error_propagate(errp, local_err) or error_propagate_prepend(errp,
 local_err, ...) expression, where @errp is the parameter and
 @local_err is the variable.

If I understand you correctly, you're pointing out two potential issues:

1. This rule can match functions rule1 does not match if there is
another function with the same name that rule1 does match.

2. This rule matches in the entire function matched by rule1, even when
parts of that function use a different @errp or @local_err.

I figure these apply to all rules with identifier rule1.fn, not just
this one.  Correct?

Regarding 1.  There must be a better way to chain rules together, but I
don't know it.


Hmm, what about something like this:

@rule1 disable optional_qualifier exists@
identifier fn, local_err;
symbol errp;
@@

   fn(..., Error **
- errp
+ ___errp_coccinelle_updating___
  , ...)
   {
   ...
   Error *local_err = NULL;
   ...
(
  error_propagate_prepend(errp, local_err, ...);
|
  error_propagate(errp, local_err);
)
   ...
   }


[..]

match symbol ___errp_coccinelle_updating___ in following rules in function 
header

[..]


@ disable optional_qualifier@
identifier fn, local_err;
symbol errp;
@@

   fn(..., Error **
- ___errp_coccinelle_updating___
+ errp
  , ...)
   {
   ...
   }


- hacky, but seems not more hacky than python detection, and should work better


As simple, forceful and unsubtle as a sledgehammer.  I like it :)




Hmm, not so simple.

It leads to reindenting of function header, which is bad.


Because ___errp_coccinelle_updating___ is longer than errp, I guess.
Try ?


I'm afraid not. It's because it just adds \n, when I do

...,

- errp
+ ___errp_coccinelle_updating___
,...




Possible solution is instead

fn(...)
{
+   ___errp_coccinelle_updating___();


but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.

.

So, I'm returning to just a warning.

I think something simple like

@@
identifier rule1.fn;
position p != rule1.p;
@@

fn@p(...) {...}

@ script:python@



should work.


Up to you.




--
Best regards,
Vladimir

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

Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-11 Thread Anthony PERARD

On Wed, Mar 11, 2020 at 01:57:37PM +, Julien Grall wrote:
> Hi Anthony,
> 
> On 09/03/2020 17:45, Anthony PERARD wrote:
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index e9d356f05c2b..2b593c5ef99a 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -36,8 +36,8 @@
> >   #define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
> >   #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
> 
> NIT: I would also take the opportunity to clean-up the line by remove the
> extra () and the space before (. Something like:
> 
> #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
> 
> [...]
> 
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index e5015f93a2d8..4d45ea3dac3c 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -45,8 +45,8 @@
> >   #define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
> >(__HEAD_FLAG_PHYS_BASE << 3))
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
> 
> Same here.

Those clean-up sounds good.

> I am happy to fix both cases on commit:
>
> Acked-by: Julien Grall 

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> 11.03.2020 12:38, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  writes:
>>
>>> 09.03.2020 12:56, Markus Armbruster wrote:
 Suggest

   scripts: Coccinelle script to use auto-propagated errp

 or

   scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

 Vladimir Sementsov-Ogievskiy  writes:
>> [...]
> +// Note, that we update everything related to matched by rule1 function 
> name
> +// and local_err name. We may match something not related to the pattern
> +// matched by rule1. For example, local_err may be defined with the same 
> name
> +// in different blocks inside one function, and in one block follow the
> +// propagation pattern and in other block doesn't. Or we may have several
> +// functions with the same name (for different configurations).

 Context: rule1 matches functions that have all three of

 * an Error **errp parameter

 * an Error *local_err = NULL variable declaration

 * an error_propagate(errp, local_err) or error_propagate_prepend(errp,
 local_err, ...) expression, where @errp is the parameter and
 @local_err is the variable.

 If I understand you correctly, you're pointing out two potential issues:

 1. This rule can match functions rule1 does not match if there is
 another function with the same name that rule1 does match.

 2. This rule matches in the entire function matched by rule1, even when
 parts of that function use a different @errp or @local_err.

 I figure these apply to all rules with identifier rule1.fn, not just
 this one.  Correct?

 Regarding 1.  There must be a better way to chain rules together, but I
 don't know it.
>>>
>>> Hmm, what about something like this:
>>>
>>> @rule1 disable optional_qualifier exists@
>>> identifier fn, local_err;
>>> symbol errp;
>>> @@
>>>
>>>   fn(..., Error **
>>> - errp
>>> + ___errp_coccinelle_updating___
>>>  , ...)
>>>   {
>>>   ...
>>>   Error *local_err = NULL;
>>>   ...
>>> (
>>>  error_propagate_prepend(errp, local_err, ...);
>>> |
>>>  error_propagate(errp, local_err);
>>> )
>>>   ...
>>>   }
>>>
>>>
>>> [..]
>>>
>>> match symbol ___errp_coccinelle_updating___ in following rules in function 
>>> header
>>>
>>> [..]
>>>
>>>
>>> @ disable optional_qualifier@
>>> identifier fn, local_err;
>>> symbol errp;
>>> @@
>>>
>>>   fn(..., Error **
>>> - ___errp_coccinelle_updating___
>>> + errp
>>>  , ...)
>>>   {
>>>   ...
>>>   }
>>>
>>>
>>> - hacky, but seems not more hacky than python detection, and should work 
>>> better
>>
>> As simple, forceful and unsubtle as a sledgehammer.  I like it :)
>>
>
>
> Hmm, not so simple.
>
> It leads to reindenting of function header, which is bad.

Because ___errp_coccinelle_updating___ is longer than errp, I guess.
Try ?

> Possible solution is instead
>
> fn(...)
> {
> +   ___errp_coccinelle_updating___();
>
>
> but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.
>
> .
>
> So, I'm returning to just a warning.
>
> I think something simple like
>
> @@
> identifier rule1.fn;
> position p != rule1.p;
> @@
>
> fn@p(...) {...}
>
> @ script:python@
>
> 
>
> should work.

Up to you.


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

Re: [Xen-devel] [PATCH v4 2/3] golang/xenlight: Notify xenlight of SIGCHLD

2020-03-11 Thread Nick Rosbrook

> libxl forks external processes and waits for them to complete; it
> therefore needs to be notified when children exit.
>
> In absence of instructions to the contrary, libxl sets up its own
> SIGCHLD handlers.
>
> Golang always unmasks and handles SIGCHLD itself.  libxl thankfully
> notices this and throws an assert() rather than clobbering SIGCHLD
> handlers.
>
> Tell libxl that we'll be responsible for getting SIGCHLD notifications
> to it.  Arrange for a channel in the context to receive notifications
> on SIGCHLD, and set up a goroutine that will pass these on to libxl.
>
> NB that every libxl context needs a notification; so multiple contexts
> will each spin up their own goroutine when opening a context, and shut
> it down on close.
>
> libxl also wants to hold on to a const pointer to
> xenlight_childproc_hooks rather than do a copy; so make a global
> structure in C space.  Make it `static const`, just for extra safety;
> this requires making a function in the C space to pass it to libxl.
>
> While here, add a few comments to make the context set-up a bit easier
> to follow.
>
> Signed-off-by: George Dunlap 

Reviewed-by: Nick Rosbrook 

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

Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

2020-03-11 Thread Julien Grall


Hi Anthony,

On 09/03/2020 17:45, Anthony PERARD wrote:

At the moment, early printk can only be configured on the make command
line. It is not very handy because a user has to remove the option
everytime it is using another command other than compiling the
hypervisor.

Furthermore, early printk is one of the few odds one that are not
using Kconfig.

So this is about time to move it to Kconfig.

The new kconfigs options allow a user to eather select a UART driver
to use at boot time, and set the parameters, or it is still possible
to select a platform which will set the parameters.

If CONFIG_EARLY_PRINTK is present in the environment or on the make
command line, make will return an error.

Signed-off-by: Julien Grall 
Signed-off-by: Anthony PERARD 


The patch looks good to me. I have only a couple of comments (see below).



---

Original patch:
 [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk 
using Kconfig
 <20190913103953.8182-1-julien.gr...@arm.com>
---

Notes:
 v3:
 - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
   select which object to build).
 - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
 - typos
 - drop the list of aliases in early-printk.txt. Kconfig choice menu
   should be enough.
 - reword early-printk.txt.
 - rework how EARLY_PRINTK is set to Y
   and use that instead of a list of all EARLY_UART_*
 - Add a check to ask user to use Kconfig to set early printk.
 - rework the possible choice to have all uart driver and platform
   specific option together.
 - have added or reword prompt and help messages of the different
   options. The platform specific option don't have extended help, the
   prompt is probably enough.
   (The non-platform specific options have the help message that Julien
   have written in the first version.)
 - have made EARLY_UART_INIT dependent on the value of
   EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
   users.

  docs/misc/arm/early-printk.txt |  71 +++-
  xen/Kconfig.debug  |   2 +
  xen/arch/arm/Kconfig.debug | 287 +
  xen/arch/arm/Makefile  |   2 +-
  xen/arch/arm/Rules.mk  |  74 +
  xen/arch/arm/arm32/Makefile|   2 +-
  xen/arch/arm/arm64/Makefile|   2 +-
  xen/arch/x86/Kconfig.debug |   0
  8 files changed, 317 insertions(+), 123 deletions(-)
  create mode 100644 xen/arch/arm/Kconfig.debug
  create mode 100644 xen/arch/x86/Kconfig.debug

diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index 89e081e51eaf..c61973013097 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -1,64 +1,39 @@
  How to enable early printk
  
-Early printk can only be enabled if debug=y. You may want to enable it if

-you are debbuging code that executes before the console is initialized.
+Early printk can only be enabled if CONFIG_DEBUG=y.  You may want to enable


NIT: AFAICT, the file is using one space after full stop. I would like 
to keep it like that for consistency :).



+it if you are debugging code that executes before the console is
+initialized.
  
  Note that selecting this option will limit Xen to a single UART definition.

  Attempting to boot Xen image on a different platform *will not work*, so this
  option should not be enable for Xens that are intended to be portable.
  
-CONFIG_EARLY_PRINTK=,,

+Select one of the "Early printk via * UART" in the choice possible for
+"Early printk" in the "Debugging options" of Kconfig. You will then need to
+set other options, which depends on the driver selected.
  
- and  are mandatory arguments:

+CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory argument, it is the base
+physical address of the UART to use.
  
-  -  is the name of the driver, see xen/arch/arm/arm{32,64}/debug-*.inc

-(where  corresponds to the wildcarded *).
-  -  is the base physical address of the UART to use
+Other options depends on the driver selected:
+  - 8250
+- CONFIG_EARLY_UART_8250_REG_SHIFT is, optionally, the left-shift to
+  apply to the register offsets within the uart.
+  - pl011
+- CONFIG_EARLY_UART_PL011_BAUD_RATE is, optionally, a baud rate which
+  should be used to configure the UART at start of day.
  
- varies depending on :

+  If CONFIG_EARLY_UART_PL011_BAUD_RATE  is set to 0 then the code will
+  not try to initialize the UART, so that bootloader or firmware
+  settings can be used for maximum compatibility.
+  - scif
+- CONFIG_EARLY_UART_SCIF_VERSION_* is, optionally, the interface version
+  of the UART. Default to version NONE.
  
-  - 8250,,

--  is, optionally, the left-shift to apply to the
-  register offsets within the uart.
-  - pl011,,
--  is, optionally a baud rate which should be used to
-  configure the UART at start of day.
-
-  If  is not

Re: [Xen-devel] [PATCH v4 1/3] golang/xenlight: Don't try to marshall zero-length arrays in fromC

2020-03-11 Thread Nicholas Rosbrook

> The current fromC array code will do the "magic" casting and
> martialling even when num_foo variable is 0.  Go crashes when doing
> the cast.
> 
> Only do array marshalling if the number of elements is non-zero;
> otherwise, leave the target pointer empty (nil for Go slices, NULL for
> C arrays).
> 
> Signed-off-by: George Dunlap 

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

Re: [Xen-devel] [PATCH OSSTEST v2 2/2] make-flight: add dom0 PVH test

2020-03-11 Thread Roger Pau Monné

On Tue, Mar 10, 2020 at 03:52:56PM +, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH OSSTEST v2 2/2] make-flight: add dom0 PVH 
> test"):
> > Add a dom0 PVH test, the test to be run is launching a PV guest
> > (test-debian). Note the PVH dom0 tests are only enabled for Xen >=
> > 4.13.
> > 
> > The runvar difference is:
> 
> Are you sure this is still true ?  It doesn't mention the iommu host
> flag which

Updated it on v3.

> > +  job_create_test test-$xenarch$kern-$dom0arch-dom0pvh-xl-$cpuvendor \
> > +test-debian xl $xenarch $dom0arch $debian_runvars \
> > +all_hostflags=$most_hostflags,hvm-$cpuvendor,iommu \
> > +xen_boot_append='dom0=pvh,verbose'
> 
> appears here.  So please regenerate this.
> 
> I gave your previous version a Reviewed-by.  Is this the only thing
> you changed ?

Yes.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 12:38, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

  scripts: Coccinelle script to use auto-propagated errp

or

  scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:

[...]

+// Note, that we update everything related to matched by rule1 function name
+// and local_err name. We may match something not related to the pattern
+// matched by rule1. For example, local_err may be defined with the same name
+// in different blocks inside one function, and in one block follow the
+// propagation pattern and in other block doesn't. Or we may have several
+// functions with the same name (for different configurations).


Context: rule1 matches functions that have all three of

* an Error **errp parameter

* an Error *local_err = NULL variable declaration

* an error_propagate(errp, local_err) or error_propagate_prepend(errp,
local_err, ...) expression, where @errp is the parameter and
@local_err is the variable.

If I understand you correctly, you're pointing out two potential issues:

1. This rule can match functions rule1 does not match if there is
another function with the same name that rule1 does match.

2. This rule matches in the entire function matched by rule1, even when
parts of that function use a different @errp or @local_err.

I figure these apply to all rules with identifier rule1.fn, not just
this one.  Correct?

Regarding 1.  There must be a better way to chain rules together, but I
don't know it.


Hmm, what about something like this:

@rule1 disable optional_qualifier exists@
identifier fn, local_err;
symbol errp;
@@

  fn(..., Error **
- errp
+ ___errp_coccinelle_updating___
 , ...)
  {
  ...
  Error *local_err = NULL;
  ...
(
 error_propagate_prepend(errp, local_err, ...);
|
 error_propagate(errp, local_err);
)
  ...
  }


[..]

match symbol ___errp_coccinelle_updating___ in following rules in function 
header

[..]


@ disable optional_qualifier@
identifier fn, local_err;
symbol errp;
@@

  fn(..., Error **
- ___errp_coccinelle_updating___
+ errp
 , ...)
  {
  ...
  }


- hacky, but seems not more hacky than python detection, and should work better


As simple, forceful and unsubtle as a sledgehammer.  I like it :)




Hmm, not so simple.

It leads to reindenting of function header, which is bad.

Possible solution is instead

fn(...)
{
+   ___errp_coccinelle_updating___();


but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.

.

So, I'm returning to just a warning.

I think something simple like

@@
identifier rule1.fn;
position p != rule1.p;
@@

fn@p(...) {...}

@ script:python@



should work.

--
Best regards,
Vladimir

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

Re: [Xen-devel] [PATCH OSSTEST v2 1/2] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Ian Jackson

Roger Pau Monne writes ("Re: [PATCH OSSTEST v2 1/2] examine: detect IOMMU 
availability and add it as a hostflag"):
> On Tue, Mar 10, 2020 at 03:51:34PM +, Ian Jackson wrote:
> > Firstly, can you break this new code out into its own patch ?
> 
> Can do, but then there will be no user of the introduced code which I
> tend to avoid.

I prefer that to the alternative of mixing it up.

> [rest snipped]

OK, good.

Thanks,
Ian.

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

Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-11 Thread Julien Grall


Hi Anthony,

On 09/03/2020 17:45, Anthony PERARD wrote:

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index e9d356f05c2b..2b593c5ef99a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -36,8 +36,8 @@
  #define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
  #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
  
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))

-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


NIT: I would also take the opportunity to clean-up the line by remove 
the extra () and the space before (. Something like:


#if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)

[...]


diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index e5015f93a2d8..4d45ea3dac3c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -45,8 +45,8 @@
  #define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
   (__HEAD_FLAG_PHYS_BASE << 3))
  
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))

-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


Same here.

I am happy to fix both cases on commit:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH OSSTEST v2 1/2] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Roger Pau Monné

On Tue, Mar 10, 2020 at 03:51:34PM +, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH OSSTEST v2 1/2] examine: detect IOMMU 
> availability and add it as a hostflag"):
> > Introduce a new test to check for iommu availability and add it as a
> > hostflag if found.
> 
> Thanks.
> 
> > +sub set_flag($$$) {
> 
> Firstly, can you break this new code out into its own patch ?

Can do, but then there will be no user of the introduced code which I
tend to avoid.

> > +my ($hd, $ho, $flag) = @_;
> 
> Secondly, this doesn't take a booleanish for yes/no.  I think it
> should.  Ie, it should be capable of both setting and clearing.
> I think leaving that functionality out now is too close to Extreme
> Programming for my taste, even for osstest :-).
> 
> > +my $rmq = $dbh_tests->prepare(< > +DELETE FROM hostflags WHERE hostname=? AND hostflag=?
> > +END
> > +my $addq = $dbh_tests->prepare(< > +INSERT INTO hostflags (hostname,hostflag) VALUES (?,?)
> > +END
> > +my $blessing = intended_blessing();
> > +
> > +die "Attempting to modify host flags with intended blessing $blessing 
> > != real"
> > +if $blessing ne "real";
> 
> Much of this code is identical to that in set_property.
> I think maybe you could introduce
> 
>sub modify_host ($$$) {
>my ($hd, $ho, $fn) = @_;
> 
> which contains the call to intended_blessing and passes its $fn
> argument to db_retry ?

Ack.

> 
> > +++ b/Osstest/HostDB/Static.pm
> ...
> > +sub set_property($$$) {
> > +my ($hd, $ho, $flag) = @_;
> > +
> > +die "Cannot set flags in standalone mode for $ho->{Name} $flag\n";
> > +}
> 
> I considered whether this meant that modify_host ought to be part of
> some base class but $rmq etc. would need setting up and plumbing
> through so that seems both too annoying and to make things too
> abstract.  But if you think you can and would like to, please do...
> 
> > diff --git a/ts-examine-iommu b/ts-examine-iommu
> > new file mode 100755
> > index ..ae91d4d2
> > --- /dev/null
> > +++ b/ts-examine-iommu
> > @@ -0,0 +1,31 @@
> > +#!/usr/bin/perl -w
> ...
> > +our $has_iommu = !target_cmd_root_status($ho, 'xl info|grep directio', 10);
> 
> Please fetch the output of xl info and do the grepping in perl.
> 
> The way you do it here will miss a situation where xl info fails
> completely, which ought to be fatal, not treated as "no iommu".

Sure.

> Apart from these things, the code all LGTM.

Thanks, Roger.

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

[Xen-devel] [linux-5.4 test] 148356: regressions - FAIL

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-debianhvm-amd64  broken in 148282
 test-amd64-amd64-xl-credit2  broken  in 148282
 test-amd64-i386-qemut-rhel6hvm-intel  broken in 148282
 test-amd64-amd64-xl-credit1  broken  in 148282
 test-amd64-amd64-xl-xsm  broken  in 148282
 test-amd64-i386-xl-qemut-win7-amd64   broken in 148282
 test-amd64-amd64-xl-qemut-win7-amd64  broken in 148282
 test-amd64-amd64-xl-qemuu-ovmf-amd64  broken in 148282
 test-amd64-i386-xl-shadowbroken  in 148282
 test-amd64-amd64-xl-multivcpu broken in 148282
 test-amd64-amd64-libvirt-vhd broken  in 148282
 test-amd64-amd64-xl-rtds broken  in 148282
 test-amd64-i386-pair broken  in 148282
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm   broken in 148282
 test-amd64-i386-xl-qemuu-ovmf-amd64   broken in 148282
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
146121

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-multivcpu 4 host-install(4) broken in 148282 pass in 148356
 test-amd64-i386-xl-qemut-win7-amd64 4 host-install(4) broken in 148282 pass in 
148356
 test-amd64-amd64-xl-xsm  4 host-install(4) broken in 148282 pass in 148356
 test-amd64-amd64-xl-credit2  4 host-install(4) broken in 148282 pass in 148356
 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken in 148282 
pass in 148356
 test-amd64-amd64-libvirt-vhd 4 host-install(4) broken in 148282 pass in 148356
 test-amd64-amd64-xl-qemuu-ovmf-amd64 4 host-install(4) broken in 148282 pass 
in 148356
 test-amd64-amd64-examine  5 host-install   broken in 148282 pass in 148356
 test-amd64-amd64-xl-qemut-win7-amd64 4 host-install(4) broken in 148282 pass 
in 148356
 test-amd64-amd64-xl-credit1  4 host-install(4) broken in 148282 pass in 148356
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken in 148282 
pass in 148356
 test-amd64-i386-qemut-rhel6hvm-intel 4 host-install(4) broken in 148282 pass 
in 148356
 test-amd64-i386-xl-qemuu-ovmf-amd64 4 host-install(4) broken in 148282 pass in 
148356
 test-amd64-i386-xl-shadow4 host-install(4) broken in 148282 pass in 148356
 test-amd64-i386-pair 5 host-install/dst_host(5) broken in 148282 pass in 148356
 test-amd64-amd64-xl-rtds 4 host-install(4) broken in 148282 pass in 148356
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail in 148210 
pass in 148356
 test-arm64-arm64-xl-seattle   7 xen-boot fail in 148282 pass in 148356
 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail in 148282 pass in 
148356
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 10 debian-hvm-install fail in 
148282 pass in 148356
 test-amd64-i386-pair 21 guest-start/debian fail pass in 148210
 test-arm64-arm64-xl-seattle  16 guest-start/debian.repeat  fail pass in 148210
 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat  fail pass in 148210
 test-amd64-amd64-xl-pvshim   12 guest-startfail pass in 148282
 test-arm64-arm64-xl-thunderx 12 guest-startfail pass in 148282
 test-amd64-i386-libvirt  19 guest-start.2  fail pass in 148282
 test-arm64-arm64-xl-credit2  16 guest-start/debian.repeat  fail pass in 148282
 test-arm64-arm64-xl-credit1  16 guest-start/debian.repeat  fail pass in 148282
 test-arm64-arm64-xl  16 guest-start/debian.repeat  fail pass in 148282
 test-arm64-arm64-xl-xsm  16 guest-start/debian.repeat  fail pass in 148282
 test-armhf-armhf-xl-arndale  16 guest-start/debian.repeat  fail pass in 148282
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat  fail pass in 148282
 test-amd64-amd64-xl-shadow   20 guest-start/debian.repeat  fail pass in 148282
 test-armhf-armhf-libvirt 12 guest-startfail pass in 148282
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 18 guest-start/debianhvm.repeat fail 
pass in 148282
 test-armhf-armhf-xl-credit2  16 guest-start/debian.repeat  fail pass in 148282
 test-amd64-i386-xl-qemut-debianhvm-amd64 18 guest-start/debianhvm.repeat fail 
pass in 148282
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat  fail pass in 148282

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-thunderx 13 migrate-support-check fail in 148210 never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-check fail in 148210 never 
pass
 test-armhf-armhf-libvirt13 migrate-support-check fail in 148210 never pass
 test-armhf-armhf-libvirt 14 saverestore-support-check fail in 148210 never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail

Re: [Xen-devel] [PATCH v2 1/9] x86/HVM: reduce domain.h include dependencies

2020-03-11 Thread Andrew Cooper

On 10/03/2020 15:48, Jan Beulich wrote:
> Drop #include-s not needed by the header itself. Put the ones needed
> into whichever other files actually need them.
>
> Signed-off-by: Jan Beulich 
> ---
> v2: Also make things build with XSM=y.

Looking better, but still got problems.

xen_pv_console.c: In function ‘pv_console_init’:
xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first
use in this function)
 r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, _pfn);
 ^

and

shim.c: In function ‘pv_shim_fixup_e820’:
shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in
this function)
 MARK_PARAM_RAM(HVM_PARAM_STORE_PFN);
    ^

~Andrew

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

Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-11 Thread Jens Axboe

On 3/10/20 6:22 PM, Matteo Croce wrote:
> The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined
> several times in different flavours across the whole tree.
> Define them just once in a common header.
> 
> While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too
> and rename SECTOR_MASK to PAGE_SECTORS_MASK.

Applied, thanks.

-- 
Jens Axboe


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

Re: [Xen-devel] [PATCH OSSTEST v3 1/2] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Ian Jackson

Roger Pau Monne writes ("[PATCH OSSTEST v3 1/2] examine: detect IOMMU 
availability and add it as a hostflag"):
> Introduce a new test to check for iommu availability and add it as a
> hostflag if found.

I regret to have to say that you seem to have overlooked my emailed
review comments on v2.  Did you receive them ?
  Subject: Re: [PATCH OSSTEST v2 1/2] examine: detect IOMMU availability and 
add it as a hostflag
  Date: Tue, 10 Mar 2020 15:51:34 +
  Message-ID: <24167.46982.390819.472...@mariner.uk.xensource.com>

Please could you find that mail and address my comments.

(As you do that, please make sure that you combine set_flag and
remove_flag into one function which takes a boolean parameter.  I
already asked for the boolean parameter in v2.)

(There was also a reply to v2 2/2.  I haven't looked at v3 2/2 to see
if you addressed those comments.)

Sorry,
Ian.

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

Re: [Xen-devel] [PATCH v4 6/6] xen/rcu: add per-lock counter in debug builds

2020-03-11 Thread Jürgen Groß


On 11.03.20 13:14, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

Add a lock specific counter to rcu read locks in debug builds. This
allows to test for matching lock/unlock calls.


Similar checking doesn't exist for e.g. spin locks iirc, and hence
I think you want to spend the word on the "why" here.


With spinlock debugging turned on there is such a check in rel_lock():
The locking cpu has to match and on unlock the locking cpu is set to
SPINLOCK_NO_CPU.

I can add something like:

"This will help top avoid cases like the one fixed by commit
 98ed1f43cc2c89 where different rcu read locks were referenced in the
 lock and unlock calls."


Juergen

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

Re: [Xen-devel] [PATCH] scripts: Use stat to check lock claim

2020-03-11 Thread Jason Andryuk

On Tue, Mar 10, 2020 at 4:06 PM Jason Andryuk  wrote:
>
> On Tue, Mar 10, 2020 at 11:43 AM Ian Jackson  wrote:
> > Alternatively, if you don't mind using --printf instead of -c,
> >
> > $ bash -c 'x=$( stat -L --format "%D.%i " t u 2>/dev/null || : ); echo 
> > ${x%% *} = ${x#* }'
> > fe04.844307 = fe04.826417
> > $
> >
> > I don't know when --format was introduced.
>
> Looks like --printf was introduced in 2005.  I think I prefer this to
> having the newlines.  You still have some of the string substitution
> concerns, but I think think relying on a successful stat(1) call to
> give two output values is reasonable.

busybox stat does not support --printf.  This is not an immediate
concern for me, but it's something I thought of and just tested.  The
newline approach avoids this complication.

> > I'm sorry to bounce the patch over such a small thing, but this is
> > path is already quite slow and is critical for domain creation and I
> > would prefer not to add (two) additional subprocess invocations here.
>
> No worries.

The above gyrations can be avoided if we just call stat twice - once
for the fd and once for the file.  They aren't required to be in a
single call.  But moving forward with a single call, we have a few
options:

We could use an array to side-step the line splitting:

if stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null )
then
stat=(${stat})
fd_stat=${stat[0]}
file_stat=${stat[1]}
[ "$fd_stat" = "$file_stat" ]

Another option is to use the bashism $'\n' instead of the literal newlines:

if stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null )
then
fd_stat=${stat%$'\n'*}
file_stat=${stat#*$'\n'}

Or just use your newline construct.  Which do you prefer?

Regards,
Jason

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

Re: [Xen-devel] [PATCH v4 6/6] xen/rcu: add per-lock counter in debug builds

2020-03-11 Thread Jan Beulich

On 10.03.2020 08:28, Juergen Gross wrote:
> Add a lock specific counter to rcu read locks in debug builds. This
> allows to test for matching lock/unlock calls.

Similar checking doesn't exist for e.g. spin locks iirc, and hence
I think you want to spend the word on the "why" here.

Jan

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jürgen Groß


On 11.03.20 12:33, Jan Beulich wrote:

On 11.03.2020 10:47, Jürgen Groß wrote:

On 11.03.20 10:36, Jan Beulich wrote:

On 11.03.2020 10:27, Jürgen Groß wrote:

On 11.03.20 10:25, Jan Beulich wrote:

On 11.03.2020 07:07, Jürgen Groß wrote:

On 10.03.20 18:02, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, 
int level,
 struct amd_iommu_pte *pde = _vaddr[index];
 
 if ( !(index % 2) )

-process_pending_softirqs();
+process_pending_softirqs_norcu();


At the example of this - the property of holding an RCU lock is
entirely invisible here, as it's the generic
iommu_dump_p2m_table() which acquires it. This suggest to me that
going forward breaking this is going to be very likely. Couldn't
process_pending_softirqs() exclude RCU handling when finding
preempt_count() to return non-zero?


This can be done, but then the non-debug build would require to have
non-empty rcu lock functions.


I guess I don't understand - I see only one version of them:

#define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
#define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })

Same for the preempt count adjustment operations.


See patch 5.


Which I haven't looked at yet, and which I also shouldn't need to
look at to understand the patch here. If this is a preparatory
change rather than some form of fix or improvement, then the
description should say so.


This was just meant as an answer to your question regarding considering
preempt_count(). Just changing this patch here and then undoing the
change again in patch 5 is no option IMO, so I gave a hint why using
preempt_count() might be a bad idea.


I've briefly looked at patch 5, and I don't see why the counter you
introduce there couldn't also be maintained in non-debug builds.


Okay. I'll modify patches 3 and 5 accordingly.


Juergen


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

Re: [Xen-devel] [PATCH v4 4/6] xen/rcu: fix rcu_lock_domain()

2020-03-11 Thread Jürgen Groß


On 11.03.20 12:35, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

rcu_lock_domain() misuses the domain structure as rcu lock, which is
working only as long as rcu_read_lock() isn't evaluating the lock.

Fix that by adding a rcu lock to struct domain and use that for
rcu_lock_domain().

Signed-off-by: Juergen Gross 


Reviewed-by: Jan Beulich 

I guess this one is independent of patches 2 and 3, and hence could
go in together with patch 1?


Yes.


Juergen

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

Re: [Xen-devel] [PATCH v4 4/6] xen/rcu: fix rcu_lock_domain()

2020-03-11 Thread Jan Beulich

On 10.03.2020 08:28, Juergen Gross wrote:
> rcu_lock_domain() misuses the domain structure as rcu lock, which is
> working only as long as rcu_read_lock() isn't evaluating the lock.
> 
> Fix that by adding a rcu lock to struct domain and use that for
> rcu_lock_domain().
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Jan Beulich 

I guess this one is independent of patches 2 and 3, and hence could
go in together with patch 1?

Jan

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jan Beulich

On 11.03.2020 10:47, Jürgen Groß wrote:
> On 11.03.20 10:36, Jan Beulich wrote:
>> On 11.03.2020 10:27, Jürgen Groß wrote:
>>> On 11.03.20 10:25, Jan Beulich wrote:
 On 11.03.2020 07:07, Jürgen Groß wrote:
> On 10.03.20 18:02, Jan Beulich wrote:
>> On 10.03.2020 08:28, Juergen Gross wrote:
>>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> @@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct 
>>> page_info* pg, int level,
>>> struct amd_iommu_pte *pde = _vaddr[index];
>>> 
>>> if ( !(index % 2) )
>>> -process_pending_softirqs();
>>> +process_pending_softirqs_norcu();
>>
>> At the example of this - the property of holding an RCU lock is
>> entirely invisible here, as it's the generic
>> iommu_dump_p2m_table() which acquires it. This suggest to me that
>> going forward breaking this is going to be very likely. Couldn't
>> process_pending_softirqs() exclude RCU handling when finding
>> preempt_count() to return non-zero?
>
> This can be done, but then the non-debug build would require to have
> non-empty rcu lock functions.

 I guess I don't understand - I see only one version of them:

 #define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
 #define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })

 Same for the preempt count adjustment operations.
>>>
>>> See patch 5.
>>
>> Which I haven't looked at yet, and which I also shouldn't need to
>> look at to understand the patch here. If this is a preparatory
>> change rather than some form of fix or improvement, then the
>> description should say so.
> 
> This was just meant as an answer to your question regarding considering
> preempt_count(). Just changing this patch here and then undoing the
> change again in patch 5 is no option IMO, so I gave a hint why using
> preempt_count() might be a bad idea.

I've briefly looked at patch 5, and I don't see why the counter you
introduce there couldn't also be maintained in non-debug builds.

Jan

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

Re: [Xen-devel] [PATCH v3] block: refactor duplicated macros

2020-03-11 Thread Coly Li

On 2020/3/11 8:22 上午, Matteo Croce wrote:
> The macros PAGE_SECTORS, PAGE_SECTORS_SHIFT and SECTOR_MASK are defined
> several times in different flavours across the whole tree.
> Define them just once in a common header.
> 
> While at it, replace replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT" too
> and rename SECTOR_MASK to PAGE_SECTORS_MASK.
> 
> Signed-off-by: Matteo Croce 

Hi Matteo,

For the bcache part, it looks good to me.

Acked-by: Coly Li 

> ---
> v3:
> As Guoqing Jiang suggested, replace "PAGE_SHIFT - 9" with "PAGE_SECTORS_SHIFT"
> 
> v2:
> As Dan Williams suggested:
> 
>  #define PAGE_SECTORS_MASK(~(PAGE_SECTORS - 1))
> 
>  block/blk-lib.c  |  2 +-
>  block/blk-settings.c |  4 ++--
>  block/partition-generic.c|  2 +-
>  drivers/block/brd.c  |  3 ---
>  drivers/block/null_blk_main.c| 14 +-
>  drivers/block/zram/zram_drv.c|  8 
>  drivers/block/zram/zram_drv.h|  2 --
>  drivers/dax/super.c  |  2 +-
>  drivers/md/bcache/util.h |  2 --
>  drivers/md/dm-bufio.c|  6 +++---
>  drivers/md/dm-integrity.c| 10 +-
>  drivers/md/dm-table.c|  2 +-
>  drivers/md/md.c  |  4 ++--
>  drivers/md/raid1.c   |  2 +-
>  drivers/md/raid10.c  |  2 +-
>  drivers/md/raid5-cache.c | 10 +-
>  drivers/md/raid5.h   |  2 +-
>  drivers/mmc/core/host.c  |  3 ++-
>  drivers/nvme/host/fc.c   |  2 +-
>  drivers/nvme/target/loop.c   |  2 +-
>  drivers/scsi/xen-scsifront.c |  4 ++--
>  fs/erofs/internal.h  |  2 +-
>  fs/ext2/dir.c|  2 +-
>  fs/iomap/buffered-io.c   |  2 +-
>  fs/libfs.c   |  2 +-
>  fs/nfs/blocklayout/blocklayout.h |  2 --
>  fs/nilfs2/dir.c  |  2 +-
>  include/linux/blkdev.h   |  4 
>  include/linux/device-mapper.h|  1 -
>  mm/page_io.c |  4 ++--
>  mm/swapfile.c| 12 ++--
>  31 files changed, 56 insertions(+), 65 deletions(-)
> 

[snipped]

> diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
> index c029f7443190..55196e0f37c3 100644
> --- a/drivers/md/bcache/util.h
> +++ b/drivers/md/bcache/util.h
> @@ -15,8 +15,6 @@
>  
>  #include "closure.h"
>  
> -#define PAGE_SECTORS (PAGE_SIZE / 512)
> -
>  struct closure;
>  
>  #ifdef CONFIG_BCACHE_DEBUG

[snipped]


-- 

Coly Li

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

[Xen-devel] [PATCH OSSTEST v3 1/2] examine: detect IOMMU availability and add it as a hostflag

2020-03-11 Thread Roger Pau Monne

Introduce a new test to check for iommu availability and add it as a
hostflag if found.

Signed-off-by: Roger Pau Monné 
---
Changes since v2:
 - Allow flags to be removed.
 - Fix set_flag addition to HostBD/Static.pm.
---
 Osstest/HostDB/Executive.pm | 34 ++
 Osstest/HostDB/Static.pm| 12 
 Osstest/TestSupport.pm  | 21 -
 sg-run-job  |  1 +
 ts-examine-hostprops-save   | 27 ++-
 ts-examine-iommu| 31 +++
 6 files changed, 116 insertions(+), 10 deletions(-)
 create mode 100755 ts-examine-iommu

diff --git a/Osstest/HostDB/Executive.pm b/Osstest/HostDB/Executive.pm
index 7ffca6c4..e7dd2a21 100644
--- a/Osstest/HostDB/Executive.pm
+++ b/Osstest/HostDB/Executive.pm
@@ -90,6 +90,40 @@ END
 return $flags;
 }
 
+sub set_flag($$$) {
+my ($hd, $ho, $flag) = @_;
+my $rmq = $dbh_tests->prepare({Name}, $flag);
+$addq->execute($ho->{Name}, $flag);
+});
+}
+
+sub remove_flag($$$) {
+my ($hd, $ho, $flag) = @_;
+my $rmq = $dbh_tests->prepare({Name}, $flag);
+});
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/HostDB/Static.pm b/Osstest/HostDB/Static.pm
index 0c6be3ee..d19795b0 100644
--- a/Osstest/HostDB/Static.pm
+++ b/Osstest/HostDB/Static.pm
@@ -72,6 +72,18 @@ sub get_flags ($$) { #method
 return $flags;
 }
 
+sub set_flag($$$) {
+my ($hd, $ho, $flag) = @_;
+
+die "Cannot set flags in standalone mode for $ho->{Name} $flag\n";
+}
+
+sub remove_flag($$$) {
+my ($hd, $ho, $flag) = @_;
+
+die "Cannot remove flags in standalone mode for $ho->{Name} $flag\n";
+}
+
 sub get_arch_platforms ($$$) {
 my ($hd, $blessing, $arch, $suite) = @_;
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index f49ed529..8e69e116 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -85,8 +85,9 @@ BEGIN {
   hostnamepath hostnamepath_list set_runtime_hostflag
   power_state power_cycle power_reboot_attempts
   serial_fetch_logs set_host_property
+  set_host_flag remove_host_flag
   propname_massage propname_check
-  hostprop_putative_record
+  hostprop_putative_record hostflag_putative_record
  
   get_stashed open_unique_stashfile compress_stashed
   dir_identify_vcs
@@ -1411,6 +1412,24 @@ sub hostprop_putative_record ($$$) {
 store_runvar("hostprop/$ho->{Ident}/$prop", $val);
 }
 
+sub set_host_flag ($$) {
+my ($ho,$flag) = @_;
+
+$mhostdb->set_flag($ho, $flag);
+}
+
+sub remove_host_flag ($$) {
+my ($ho,$flag) = @_;
+
+$mhostdb->remove_flag($ho, $flag);
+}
+
+sub hostflag_putative_record ($$$) {
+my ($ho, $prop, $set) = @_;
+
+store_runvar("hostflag/$ho->{Ident}/$prop", !!$set);
+}
+
 sub get_target_property ($$;$);
 sub get_target_property ($$;$) {
 my ($ho, $prop, $defval) = @_;
diff --git a/sg-run-job b/sg-run-job
index 7c58d4ba..f6bfdfd5 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -679,6 +679,7 @@ proc examine-host-examine {install} {
 if {$ok} {
run-ts -.  =   ts-examine-serial-post + host
run-ts .   =   ts-examine-logs-save   + host
+   run-ts .   =   ts-examine-iommu   + host
run-ts .   =   ts-examine-hostprops-save
 }
 }
diff --git a/ts-examine-hostprops-save b/ts-examine-hostprops-save
index 55d23392..1cb17c09 100755
--- a/ts-examine-hostprops-save
+++ b/ts-examine-hostprops-save
@@ -27,20 +27,29 @@ tsreadconfig();
 
 our $blessing = intended_blessing();
 
-logm("setting host properties");
+logm("setting host properties and flags");
 
 # NB: in order to aid debug only attempt to save the host props on flights
 # with intended real blessing, for the rest just do a dry run.
 our $dry_run = $blessing ne "real";
-logm("not saving host props with intended blessing $blessing != real")
+logm("not saving host props/flags with intended blessing $blessing != real")
 if $dry_run;
 
 foreach my $k (sort keys %r) {
-next unless $k =~ m/^hostprop\/([^\/]*)\/([^\/]*)$/;
-my $ho = selecthost($1);
-my $prop = $2;
-
-logm("recording for $ho->{Name} $prop=$r{$k}");
-
-set_host_property($ho, $prop, $r{$k}) if !$dry_run;
+next unless $k =~ m/^host(prop|flag)\/([^\/]*)\/([^\/]*)$/;
+my $type = $1;
+my $ho = selecthost($2);
+my $prop = $3;
+
+if ($type == "flag") {
+logm("recording flag $prop for $ho->{Name}");
+if ($r{$k} && !$dry_run) {
+set_host_flag($ho, $prop);
+} elsif (!$dry_run) {
+remove_host_flag($ho, $prop);
+}
+} else {
+logm("recording prop for $ho->{Name} $prop=$r{$k}");
+set_host_property($ho, $prop, $r{$k})

[Xen-devel] [PATCH OSSTEST v3 2/2] make-flight: add dom0 PVH test

2020-03-11 Thread Roger Pau Monne

Add a dom0 PVH test, the test to be run is launching a PV guest
(test-debian). Note the PVH dom0 tests are only enabled for Xen >=
4.13.

The runvar difference is:

+test-amd64-amd64-dom0pvh-xl-amd   all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-intel all_host_di_version 2020-02-10
+test-amd64-amd64-dom0pvh-xl-amd   all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-intel all_host_suite  stretch
+test-amd64-amd64-dom0pvh-xl-amd   all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-amd,iommu
+test-amd64-amd64-dom0pvh-xl-intel all_hostflags   
arch-amd64,arch-xen-amd64,suite-stretch,purpose-test,hvm-intel,iommu
+test-amd64-amd64-dom0pvh-xl-amd   archamd64
+test-amd64-amd64-dom0pvh-xl-intel archamd64
+test-amd64-amd64-dom0pvh-xl-amd   buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-intel buildjobbuild-amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-intel debian_arch amd64
+test-amd64-amd64-dom0pvh-xl-amd   debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-intel debian_kernkind pvops
+test-amd64-amd64-dom0pvh-xl-amd   debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-intel debian_suitestretch
+test-amd64-amd64-dom0pvh-xl-amd   kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-intel kernbuildjobbuild-amd64-pvops
+test-amd64-amd64-dom0pvh-xl-amd   kernkindpvops
+test-amd64-amd64-dom0pvh-xl-intel kernkindpvops
+test-amd64-amd64-dom0pvh-xl-amd   toolstack   xl
+test-amd64-amd64-dom0pvh-xl-intel toolstack   xl
+test-amd64-amd64-dom0pvh-xl-amd   xen_boot_append dom0=pvh,verbose
+test-amd64-amd64-dom0pvh-xl-intel xen_boot_append dom0=pvh,verbose

Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Request hosts with iommu flag.
---
 make-flight | 24 
 1 file changed, 24 insertions(+)

diff --git a/make-flight b/make-flight
index b08431dc..48f164cc 100755
--- a/make-flight
+++ b/make-flight
@@ -753,6 +753,16 @@ test_matrix_do_one () {
   *)test_shim=y ;;
   esac
 
+  # PVH dom0 tests for versions >= 4.13 only
+  case "$xenbranch" in
+  xen-3.*-testing)  test_dom0pvh=n ;;
+  xen-4.?-testing)  test_dom0pvh=n ;;
+  xen-4.10-testing) test_dom0pvh=n ;;
+  xen-4.11-testing) test_dom0pvh=n ;;
+  xen-4.12-testing) test_dom0pvh=n ;;
+  *)test_dom0pvh=y ;;
+  esac
+
   # xend PV guest test on x86 only
   if [ x$test_xend = xy -a \( $dom0arch = "i386" -o $dom0arch = "amd64" \) ]; 
then
 job_create_test test-$xenarch$kern-$dom0arch-pv test-debian xend \
@@ -861,6 +871,20 @@ test_matrix_do_one () {
 
   fi
 
+  if [ x$test_dom0pvh = xy -a $xenarch = amd64 -a $dom0arch = amd64 ]; then
+
+for cpuvendor in amd intel; do
+
+  job_create_test test-$xenarch$kern-$dom0arch-dom0pvh-xl-$cpuvendor \
+test-debian xl $xenarch $dom0arch $debian_runvars \
+all_hostflags=$most_hostflags,hvm-$cpuvendor,iommu \
+xen_boot_append='dom0=pvh,verbose'
+
+done
+
+  fi
+
+
   if [ x$test_shim = xy -a $xenarch = amd64 ]; then
 
 job_create_test test-$xenarch$kern-$dom0arch-xl-pvshim \
-- 
2.25.0


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

[Xen-devel] [xen-unstable-coverity test] 148425: all pass - PUSHED

2020-03-11 Thread osstest service owner

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  e19d3a942e4b6f6c5b19287a4a6f5020bdab2936
baseline version:
 xen  6052921da02dd2180b80eb77c7aa115c6834067a

Last test of basis   148255  2020-03-08 09:18:25 Z3 days
Testing same since   148425  2020-03-11 09:26:50 Z0 days1 attempts


People who touched revisions under test:
  George Dunlap 
  Jan Beulich 
  Nick Rosbrook 
  Nick Rosbrook 
  Paul Durrant 
  Roger Pau Monné 
  Ross Lagerwall 
  Tamas K Lengyel 
  Tim Deegan 

jobs:
 coverity-amd64   pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   6052921da0..e19d3a942e  e19d3a942e4b6f6c5b19287a4a6f5020bdab2936 -> 
coverity-tested/smoke

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 12:53, Markus Armbruster wrote:

I think a v9 makes sense now.

If any of the improvement ideas should turn into time sinks for you,
let's talk.  We don't need perfection, we only need to get to the point
where we trust the script to do what we believe it does, understand its
limitations, and know how to compensate for them.

Right now I'm optimistic v9 will be ready for merging, perhaps with some
minor tweaking in my tree.



Good. I hope, I'll resend today.

--
Best regards,
Vladimir

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Markus Armbruster

I think a v9 makes sense now.

If any of the improvement ideas should turn into time sinks for you,
let's talk.  We don't need perfection, we only need to get to the point
where we trust the script to do what we believe it does, understand its
limitations, and know how to compensate for them.

Right now I'm optimistic v9 will be ready for merging, perhaps with some
minor tweaking in my tree.


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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 12:33, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

+
+// Convert error clearing functions

Suggest: Ensure @local_err is cleared on free


But there is no local_err after conversion


True.  Hmm.  What about this:

  // Convert calls to error_free(), possibly indirect
  // In addition to replacing @local_err by *errp, we have to clear *errp
  // to avoid use-after-free in the automatic error propagation.



OK


+(
+-error_free(local_err);
++error_free_errp(errp);
+|
+-error_report_err(local_err);
++error_report_errp(errp);
+|
+-error_reportf_err(local_err, args);
++error_reportf_errp(errp, args);
+|
+-warn_report_err(local_err);
++warn_report_errp(errp);
+|
+-warn_reportf_err(local_err, args);





--
Best regards,
Vladimir

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jürgen Groß


On 11.03.20 10:36, Jan Beulich wrote:

On 11.03.2020 10:27, Jürgen Groß wrote:

On 11.03.20 10:25, Jan Beulich wrote:

On 11.03.2020 07:07, Jürgen Groß wrote:

On 10.03.20 18:02, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, 
int level,
struct amd_iommu_pte *pde = _vaddr[index];

if ( !(index % 2) )

-process_pending_softirqs();
+process_pending_softirqs_norcu();


At the example of this - the property of holding an RCU lock is
entirely invisible here, as it's the generic
iommu_dump_p2m_table() which acquires it. This suggest to me that
going forward breaking this is going to be very likely. Couldn't
process_pending_softirqs() exclude RCU handling when finding
preempt_count() to return non-zero?


This can be done, but then the non-debug build would require to have
non-empty rcu lock functions.


I guess I don't understand - I see only one version of them:

#define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
#define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })

Same for the preempt count adjustment operations.


See patch 5.


Which I haven't looked at yet, and which I also shouldn't need to
look at to understand the patch here. If this is a preparatory
change rather than some form of fix or improvement, then the
description should say so.


This was just meant as an answer to your question regarding considering
preempt_count(). Just changing this patch here and then undoing the
change again in patch 5 is no option IMO, so I gave a hint why using
preempt_count() might be a bad idea.


Juergen

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> 09.03.2020 12:56, Markus Armbruster wrote:
>> Suggest
>>
>>  scripts: Coccinelle script to use auto-propagated errp
>>
>> or
>>
>>  scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()
>>
>> Vladimir Sementsov-Ogievskiy  writes:
[...]
>>> +// Note, that we update everything related to matched by rule1 function 
>>> name
>>> +// and local_err name. We may match something not related to the pattern
>>> +// matched by rule1. For example, local_err may be defined with the same 
>>> name
>>> +// in different blocks inside one function, and in one block follow the
>>> +// propagation pattern and in other block doesn't. Or we may have several
>>> +// functions with the same name (for different configurations).
>>
>> Context: rule1 matches functions that have all three of
>>
>> * an Error **errp parameter
>>
>> * an Error *local_err = NULL variable declaration
>>
>> * an error_propagate(errp, local_err) or error_propagate_prepend(errp,
>>local_err, ...) expression, where @errp is the parameter and
>>@local_err is the variable.
>>
>> If I understand you correctly, you're pointing out two potential issues:
>>
>> 1. This rule can match functions rule1 does not match if there is
>> another function with the same name that rule1 does match.
>>
>> 2. This rule matches in the entire function matched by rule1, even when
>> parts of that function use a different @errp or @local_err.
>>
>> I figure these apply to all rules with identifier rule1.fn, not just
>> this one.  Correct?
>>
>> Regarding 1.  There must be a better way to chain rules together, but I
>> don't know it.
>
> Hmm, what about something like this:
>
> @rule1 disable optional_qualifier exists@
> identifier fn, local_err;
> symbol errp;
> @@
>
>  fn(..., Error **
> - errp
> + ___errp_coccinelle_updating___
> , ...)
>  {
>  ...
>  Error *local_err = NULL;
>  ...
> (
> error_propagate_prepend(errp, local_err, ...);
> |
> error_propagate(errp, local_err);
> )
>  ...
>  }
>
>
> [..]
>
> match symbol ___errp_coccinelle_updating___ in following rules in function 
> header
>
> [..]
>
>
> @ disable optional_qualifier@
> identifier fn, local_err;
> symbol errp;
> @@
>
>  fn(..., Error **
> - ___errp_coccinelle_updating___
> + errp
> , ...)
>  {
>  ...
>  }
>
>
> - hacky, but seems not more hacky than python detection, and should work 
> better

As simple, forceful and unsubtle as a sledgehammer.  I like it :)

[...]


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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jan Beulich

On 11.03.2020 10:27, Jürgen Groß wrote:
> On 11.03.20 10:25, Jan Beulich wrote:
>> On 11.03.2020 07:07, Jürgen Groß wrote:
>>> On 10.03.20 18:02, Jan Beulich wrote:
 On 10.03.2020 08:28, Juergen Gross wrote:
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct 
> page_info* pg, int level,
>struct amd_iommu_pte *pde = _vaddr[index];
>
>if ( !(index % 2) )
> -process_pending_softirqs();
> +process_pending_softirqs_norcu();

 At the example of this - the property of holding an RCU lock is
 entirely invisible here, as it's the generic
 iommu_dump_p2m_table() which acquires it. This suggest to me that
 going forward breaking this is going to be very likely. Couldn't
 process_pending_softirqs() exclude RCU handling when finding
 preempt_count() to return non-zero?
>>>
>>> This can be done, but then the non-debug build would require to have
>>> non-empty rcu lock functions.
>>
>> I guess I don't understand - I see only one version of them:
>>
>> #define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
>> #define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })
>>
>> Same for the preempt count adjustment operations.
> 
> See patch 5.

Which I haven't looked at yet, and which I also shouldn't need to
look at to understand the patch here. If this is a preparatory
change rather than some form of fix or improvement, then the
description should say so.

Jan

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> 09.03.2020 12:56, Markus Armbruster wrote:
>>> +
>>> +// Convert error clearing functions
>> Suggest: Ensure @local_err is cleared on free
>
> But there is no local_err after conversion

True.  Hmm.  What about this:

 // Convert calls to error_free(), possibly indirect
 // In addition to replacing @local_err by *errp, we have to clear *errp
 // to avoid use-after-free in the automatic error propagation.

>>> +(
>>> +-error_free(local_err);
>>> ++error_free_errp(errp);
>>> +|
>>> +-error_report_err(local_err);
>>> ++error_report_errp(errp);
>>> +|
>>> +-error_reportf_err(local_err, args);
>>> ++error_reportf_errp(errp, args);
>>> +|
>>> +-warn_report_err(local_err);
>>> ++warn_report_errp(errp);
>>> +|
>>> +-warn_reportf_err(local_err, args);


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

[Xen-devel] [linux-4.4 test] 148347: regressions - FAIL

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-amd broken in 148277
 test-amd64-amd64-qemuu-nested-intel   broken in 148277
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm   broken in 148277
 test-amd64-amd64-xl-qemuu-ws16-amd64  broken in 148277
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
139698
 test-amd64-i386-libvirt 18 guest-start/debian.repeat fail in 148135 REGR. vs. 
139698
 test-amd64-i386-libvirt-xsm 18 guest-start/debian.repeat fail in 148277 REGR. 
vs. 139698

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken in 148277 
pass in 148347
 test-amd64-amd64-qemuu-nested-amd 4 host-install(4) broken in 148277 pass in 
148347
 test-amd64-amd64-xl-qemuu-ws16-amd64 4 host-install(4) broken in 148277 pass 
in 148347
 test-amd64-amd64-qemuu-nested-intel 4 host-install(4) broken in 148277 pass in 
148347
 test-armhf-armhf-libvirt-raw 11 guest-start  fail in 148203 pass in 148277
 test-armhf-armhf-xl-arndale   7 xen-boot fail in 148203 pass in 148347
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail in 148277 
pass in 148347
 test-amd64-i386-libvirt  12 guest-startfail pass in 148135
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat  fail pass in 148203
 test-amd64-i386-xl-xsm   12 guest-startfail pass in 148277
 test-amd64-i386-libvirt-xsm  12 guest-startfail pass in 148277
 test-amd64-i386-libvirt-pair 21 guest-start/debian fail pass in 148277
 test-amd64-i386-xl   20 guest-start/debian.repeat  fail pass in 148277
 test-armhf-armhf-libvirt-raw  8 host-ping-check-xenfail pass in 148277
 test-armhf-armhf-xl-credit1  16 guest-start/debian.repeat  fail pass in 148277
 test-armhf-armhf-xl-vhd  15 guest-start/debian.repeat  fail pass in 148277

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt 13 migrate-support-check fail in 148135 never pass
 test-amd64-i386-libvirt-xsm 13 migrate-support-check fail in 148277 never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-check fail in 148277 never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-check fail in 148277 never 
pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13

Re: [Xen-devel] [PATCH v4] x86/cpu: Sync any remaining RCU callbacks before CPU up/down

2020-03-11 Thread Jan Beulich

On 10.03.2020 19:06, Igor Druzhinin wrote:
> During CPU down operation RCU callbacks are scheduled to finish
> off some actions later as soon as CPU is fully dead (the same applies
> to CPU up operation in case error path is taken). If in the same grace
> period another CPU up operation is performed on the same CPU, RCU callback
> will be called later on a CPU in a potentially wrong (already up again
> instead of still being down) state leading to eventual state inconsistency
> and/or crash.
> 
> In order to avoid it - flush RCU callbacks explicitly before starting the
> next CPU up/down operation.
> 
> Reviewed-by: Juergen Gross 
> Signed-off-by: Igor Druzhinin 

(Nit: These preferably would come in inverse order.)

Reviewed-by: Jan Beulich 

Jan

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jürgen Groß


On 11.03.20 10:25, Jan Beulich wrote:

On 11.03.2020 07:07, Jürgen Groß wrote:

On 10.03.20 18:02, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, 
int level,
   struct amd_iommu_pte *pde = _vaddr[index];
   
   if ( !(index % 2) )

-process_pending_softirqs();
+process_pending_softirqs_norcu();


At the example of this - the property of holding an RCU lock is
entirely invisible here, as it's the generic
iommu_dump_p2m_table() which acquires it. This suggest to me that
going forward breaking this is going to be very likely. Couldn't
process_pending_softirqs() exclude RCU handling when finding
preempt_count() to return non-zero?


This can be done, but then the non-debug build would require to have
non-empty rcu lock functions.


I guess I don't understand - I see only one version of them:

#define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
#define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })

Same for the preempt count adjustment operations.


See patch 5.


Juergen

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

Re: [Xen-devel] [XEN PATCH v3 13/23] xen/build: include include/config/auto.conf in main Makefile

2020-03-11 Thread Jan Beulich

On 10.03.2020 18:10, Anthony PERARD wrote:
> On Wed, Mar 04, 2020 at 03:29:55PM +0100, Jan Beulich wrote:
>> On 26.02.2020 12:33, Anthony PERARD wrote:
>>> +# The actual configuration files used during the build are stored in
>>> +# include/generated/ and include/config/. Update them if .config is newer 
>>> than
>>> +# include/config/auto.conf (which mirrors .config).
>>> +#
>>> +# This exploits the 'multi-target pattern rule' trick.
>>> +# The syncconfig should be executed only once to make all the targets.
>>> +include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
>>> +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
>>> SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
>>
>> ... this are almost identical, pretty long lines. Can this be macroized,
>> please, with the actual make goal as parameter?
> 
> Sound good, would the following be fine?
> 
> kconfig = -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)"
> config:
> $(MAKE) $(kconfig) $@
> 
> I will put that new `kconfig' macro in Kbuild.include, along the
> shorthand for clean.

Looks okay at the first glance.

Jan

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jan Beulich

On 11.03.2020 07:07, Jürgen Groß wrote:
> On 10.03.20 18:02, Jan Beulich wrote:
>> On 10.03.2020 08:28, Juergen Gross wrote:
>>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> @@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* 
>>> pg, int level,
>>>   struct amd_iommu_pte *pde = _vaddr[index];
>>>   
>>>   if ( !(index % 2) )
>>> -process_pending_softirqs();
>>> +process_pending_softirqs_norcu();
>>
>> At the example of this - the property of holding an RCU lock is
>> entirely invisible here, as it's the generic
>> iommu_dump_p2m_table() which acquires it. This suggest to me that
>> going forward breaking this is going to be very likely. Couldn't
>> process_pending_softirqs() exclude RCU handling when finding
>> preempt_count() to return non-zero?
> 
> This can be done, but then the non-debug build would require to have
> non-empty rcu lock functions.

I guess I don't understand - I see only one version of them:

#define rcu_read_lock(x)   ({ ((void)(x)); preempt_disable(); })
#define rcu_read_unlock(x) ({ ((void)(x)); preempt_enable(); })

Same for the preempt count adjustment operations.

Jan

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 12:04, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

   scripts: Coccinelle script to use auto-propagated errp

or

   scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

include/qapi/error.h  |   3 +
scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
2 files changed, 234 insertions(+)
create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
 * }
 * ...
 * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
 */
  #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]


As our discussion shows, this script is somewhat hard to understand.
That's okay, it solves a somewhat thorny problem, and I don't have
better ideas.  But let me state the intended transformations once more,
so I don't get lost in the details.

Motivation:

1. Make error propagation less error-prone and improve stack backtraces

The "receive error in _error, propagate to @errp" pattern is
tedious, error-prone, and produces unhelpful stack backtraces with
_abort.

ERRP_AUTO_PROPAGATE() removes manual propagation.  It additionally
gives us the stack backtraces we want.

2. Improve error messages with _fatal

Passing @errp to error_append_hint(), error_prepend() or
error_vprepend() is useless when @errp is _fatal.

ERRP_AUTO_PROPAGATE() fixes this by

Re: [Xen-devel] [PATCH v5 6/6] domain: use PGC_extra domheap page for shared_info

2020-03-11 Thread Jan Beulich

On 10.03.2020 18:33, Paul Durrant wrote:
>> -Original Message-
>> From: Jan Beulich 
>> Sent: 09 March 2020 15:56
>>
>> On 09.03.2020 11:23, p...@xen.org wrote:
>>> --- a/xen/common/time.c
>>> +++ b/xen/common/time.c
>>> @@ -99,6 +99,18 @@ void update_domain_wallclock_time(struct domain *d)
>>>  uint32_t *wc_version;
>>>  uint64_t sec;
>>>
>>> +if ( d != current->domain )
>>> +{
>>> +/*
>>> + * We need to check is_dying here as, if it is set, the
>>> + * shared_info may have been freed. To do this safely we need
>>> + * hold the domain lock.
>>> + */
>>> +domain_lock(d);
>>> +if ( d->is_dying )
>>> +goto unlock;
>>> +}
>>
>> This shouldn't happen very often, but it's pretty heavy a lock.
>> It's a fundamental aspect of xenheap pages that their disposal
>> can b e delay until almost the last moment of guest cleanup. I
>> continue to think it's not really a good ideal to have special
>> purpose allocation (and mapping) accompanied by these pages
>> getting taken care of by the generic relinquish-resources logic
>> here (from a more general pov such is of course often nice to
>> have). Instead of freeing these pages there, couldn't they just
>> be taken off d->page_list, with the unmapping and freeing left
>> as it was?
> 
> I don't think this can be achieved without being able de-assign
> pages and I don't really want to have to invent new logic to do
> that (basically re-implementing what happens to xenheap pages).

Where's the connection to being able to de-assign pages here?
There'll be one when the same conversion is to be done for
gnttab code, but I don't see it here - the shared info page is
never to be de-assigned. As to gnttab code, I think it was
noted before that we may be better off not "unpopulating"
status pages when switching back from v2 to v1. At which point
the de-assignment need would go away there, too.

> I really don't think it is that bad to deal with shared info
> and grant table pages as domheap pages. Yes, we have to be
> careful, but in this case the lock is only taken in a
> toolstack update of the wallclock and, apart from start of
> day access, I think this is limited to XEN_DOMCTL_settimeoffset
> and XEN_DOMCTL_setvcpucontext neither of which I believe is
> particularly performance-critical.

It's not, I agree (and hence I had started my previous reply
also with "This shouldn't happen very often"). How all of this
is going to look like with the new extra_page_list I'll have
to see anyway. But for now I remain unconvinced of the want /
need to de-allocate the shared info page early.

Jan

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

Re: [Xen-devel] [PATCH 04/51] drm: Set final_kfree in drm_dev_alloc

2020-03-11 Thread Thomas Zimmermann



Am 02.03.20 um 23:25 schrieb Daniel Vetter:
> I also did a full review of all callers, and only the xen driver
> forgot to call drm_dev_put in the failure path. Fix that up too.
> 
> v2: I noticed that xen has a drm_driver.release hook, and uses
> drm_dev_alloc(). We need to remove the kfree from
> xen_drm_drv_release().
> 
> bochs also has a release hook, but leaked the drm_device ever since
> 
> commit 0a6659bdc5e8221da99eebb176fd9591435e38de
> Author: Gerd Hoffmann 
> Date:   Tue Dec 17 18:04:46 2013 +0100
> 
> drm/bochs: new driver
> 
> This patch here fixes that leak.
> 
> Same for virtio, started leaking with
> 
> commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a
> Author: Gerd Hoffmann 
> Date:   Tue Feb 11 14:58:04 2020 +0100
> 
> drm/virtio: add drm_driver.release callback.
> 
> Cc: Gerd Hoffmann 
> Cc: Oleksandr Andrushchenko 
> Cc: xen-devel@lists.xenproject.org
> 
> Reviewed-by: Oleksandr Andrushchenko 
> Signed-off-by: Daniel Vetter 

Acked-by: Thomas Zimmermann 

> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Thomas Zimmermann 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Oleksandr Andrushchenko 
> Cc: xen-devel@lists.xenproject.org
> ---
>  drivers/gpu/drm/drm_drv.c   | 3 +++
>  drivers/gpu/drm/xen/xen_drm_front.c | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 153050fc926c..7b84ee8a5eb5 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -39,6 +39,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -819,6 +820,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver 
> *driver,
>   return ERR_PTR(ret);
>   }
>  
> + drmm_add_final_kfree(dev, dev);
> +
>   return dev;
>  }
>  EXPORT_SYMBOL(drm_dev_alloc);
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c 
> b/drivers/gpu/drm/xen/xen_drm_front.c
> index 4be49c1aef51..d22b5da38935 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> @@ -461,7 +461,6 @@ static void xen_drm_drv_release(struct drm_device *dev)
>   drm_mode_config_cleanup(dev);
>  
>   drm_dev_fini(dev);
> - kfree(dev);
>  
>   if (front_info->cfg.be_alloc)
>   xenbus_switch_state(front_info->xb_dev,
> @@ -561,6 +560,7 @@ static int xen_drm_drv_init(struct xen_drm_front_info 
> *front_info)
>  fail_modeset:
>   drm_kms_helper_poll_fini(drm_dev);
>   drm_mode_config_cleanup(drm_dev);
> + drm_dev_put(drm_dev);
>  fail:
>   kfree(drm_info);
>   return ret;
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> 10.03.2020 18:47, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  writes:
>>
>>> 09.03.2020 12:56, Markus Armbruster wrote:
 Suggest

   scripts: Coccinelle script to use auto-propagated errp

 or

   scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

 Vladimir Sementsov-Ogievskiy  writes:

> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
> does corresponding changes in code (look for details in
> include/qapi/error.h)
>
> Usage example:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]

 Suggest FILES... instead of a specific set of files.

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> Cc: Eric Blake 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Greg Kurz 
> Cc: Christian Schoenebeck 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paul Durrant 
> Cc: Stefan Hajnoczi 
> Cc: "Philippe Mathieu-Daudé" 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Stefan Berger 
> Cc: Markus Armbruster 
> Cc: Michael Roth 
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-de...@nongnu.org
> Cc: xen-devel@lists.xenproject.org
>
>include/qapi/error.h  |   3 +
>scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
>2 files changed, 234 insertions(+)
>create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index bb9bcf02fb..fbfc6f1c0b 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -211,6 +211,9 @@
> * }
> * ...
> * }
> + *
> + * For mass conversion use script

 mass-conversion (we're not converting mass, we're converting en masse)

> + *   scripts/coccinelle/auto-propagated-errp.cocci
> */
>  #ifndef ERROR_H
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
> b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci

 Preface to my review of this script: may aim isn't to make it
 bullet-proof.  I want to (1) make it good enough (explained in a
 jiffie), and (2) automatically identify the spots where it still isn't
 obviously safe for manual review.

 The latter may involve additional scripting.  That's okay.

 The script is good enough when the number of possibly unsafe spots is
 low enough for careful manual review.

 When I ask for improvements that, in your opinion, go beyond "good
 enough", please push back.  I'm sure we can work it out together.

> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.
> +//
> +// This program is free software; you can redistribute it and/or modify
> +// it under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 2 of the License, or
> +// (at your option) any later version.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License
> +// along with this program.  If not, see .
> +//
> +// Usage example:
> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> +//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
> +//  --max-width 80 blockdev-nbd.c qemu-nbd.c \

 You have --max-width 80 here, but not in the commit message.  Default
 seems to be 78.  Any particular reason to change it to 80?
>>>
>>> Hmm. As I remember, without this parameter, reindenting doesn't work 
>>> correctly.
>>> So, I'm OK with "--max-width 78", but I doubt that it will work without a 
>>> parameter.
>>> Still, may be I'm wrong, we can check it.
>>
>> If you can point to an example where --max-width helps, keep it, and
>> update the commit message to match.  Else, drop it.
>>

> +//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]

As our discussion shows, this script is somewhat hard to understand.
That's okay, it solves a somewhat thorny problem, and I don't have
better ideas.  But let me state the intended transformations once more,
so I

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

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install  fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 
144861
 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install   fail REGR. vs. 144861
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 144861
 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 
144861
 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install  fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail 
REGR. vs. 144861
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install  fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. 
vs. 144861
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail 
REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. 
vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 
144861
 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install   fail REGR. vs. 144861
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail 
REGR. vs. 144861
 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 144861
 test-amd64-i386-freebsd10-amd64 11 guest-start   fail REGR. vs. 144861
 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install   fail REGR. vs. 144861
 test-amd64-amd64-libvirt-pair 21 guest-start/debian  fail REGR. vs. 144861
 test-arm64-arm64-libvirt-xsm 12 guest-start  fail REGR. vs. 144861
 test-arm64-arm64-xl-xsm  12 guest-start  fail REGR. vs. 144861
 test-armhf-armhf-xl-arndale  12 guest-start  fail REGR. vs. 144861
 test-amd64-amd64-xl-xsm 20 guest-start/debian.repeat fail REGR. vs. 144861
 test-amd64-amd64-libvirt18 guest-start/debian.repeat fail REGR. vs. 144861
 test-armhf-armhf-xl  12 guest-start  fail REGR. vs. 144861

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 16 guest-localmigrate   fail REGR. vs. 144861
 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 144861

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 144861
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 144861
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-check

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


09.03.2020 12:56, Markus Armbruster wrote:

+
+// Convert error clearing functions

Suggest: Ensure @local_err is cleared on free


But there is no local_err after conversion




+(
+-error_free(local_err);
++error_free_errp(errp);
+|
+-error_report_err(local_err);
++error_report_errp(errp);
+|
+-error_reportf_err(local_err, args);
++error_reportf_errp(errp, args);
+|
+-warn_report_err(local_err);
++warn_report_errp(errp);
+|
+-warn_reportf_err(local_err, args);



--
Best regards,
Vladimir

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

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


11.03.2020 9:55, Vladimir Sementsov-Ogievskiy wrote:

10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

  scripts: Coccinelle script to use auto-propagated errp

or

  scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
   --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
   blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

   include/qapi/error.h  |   3 +
   scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
   2 files changed, 234 insertions(+)
   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
    * }
    * ...
    * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
    */
 #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

 // Skip functions with "assert(_errp && *_errp)" statement, because that
 // signals unusual semantics, and the parameter name may well

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

 scripts: Coccinelle script to use auto-propagated errp

or

 scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

  include/qapi/error.h  |   3 +
  scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
  2 files changed, 234 insertions(+)
  create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
   * }
   * ...
   * }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
   */
  
  #ifndef ERROR_H

diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

// Skip functions with "assert(_errp && *_errp)" statement, because that
// signals unusual semantics, and the parameter name may well serve a
// purpose.


+// (like nbd_iter_channel_error()).
+//
+// Skip util/error.c to not touch, for example, error_propagate and
+// error_propagate_prepend().


error_propagate()

I much appreciate your meticulous explanation of what you skip and why.


+@ depends on !(file in "util/error.c") disable optional_qualifier@
+identifier fn;
+identifier _errp != errp;
+@@
+
+ fn(...,
+-   Error **_errp
++   Error **errp
+,...)
+ {
+(
+ ... when != assert(_errp && *_errp)
+&

[Xen-devel] [xen-unstable test] 148315: regressions - trouble: broken/fail/pass

2020-03-11 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  broken
 test-amd64-i386-qemut-rhel6hvm-amd broken
 test-amd64-i386-xl-raw   broken
 test-amd64-amd64-xl-xsm  broken
 test-amd64-i386-freebsd10-i386 broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64broken
 test-amd64-amd64-amd64-pvgrub broken
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrictbroken
 test-amd64-i386-xl-qemut-debianhvm-amd64broken
 test-amd64-i386-pair broken
 test-amd64-i386-qemuu-rhel6hvm-intel broken
 test-amd64-amd64-xl-pvshim   broken
 test-xtf-amd64-amd64-2   broken
 test-xtf-amd64-amd64-3   broken
 test-xtf-amd64-amd64-1   broken
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsmbroken
 test-amd64-i386-xl   broken
 test-xtf-amd64-amd64-5   broken
 test-amd64-amd64-pygrub  broken
 test-amd64-i386-freebsd10-amd64 broken
 test-amd64-i386-libvirt  broken
 test-amd64-amd64-xl-multivcpu broken
 test-xtf-amd64-amd64-4   broken
 test-amd64-amd64-xl-qcow2broken
 test-amd64-amd64-xl-credit1  broken
 test-amd64-amd64-xl-qemuu-win7-amd64 broken
 test-amd64-i386-libvirt-xsm  broken
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail in 148236 
REGR. vs. 148098

Tests which are failing intermittently (not blocking):
 test-xtf-amd64-amd64-34 host-install(4)  broken pass in 148236
 test-amd64-amd64-examine  5 host-install broken pass in 148236
 test-amd64-i386-xl4 host-install(4)  broken pass in 148236
 test-amd64-amd64-xl-multivcpu  4 host-install(4) broken pass in 148236
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken pass in 
148236
 test-amd64-amd64-pygrub   4 host-install(4)  broken pass in 148236
 test-amd64-amd64-xl-pvshim4 host-install(4)  broken pass in 148236
 test-amd64-i386-freebsd10-i386  4 host-install(4)broken pass in 148236
 test-xtf-amd64-amd64-44 host-install(4)  broken pass in 148236
 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken pass in 
148236
 test-xtf-amd64-amd64-24 host-install(4)  broken pass in 148236
 test-xtf-amd64-amd64-14 host-install(4)  broken pass in 148236
 test-amd64-i386-freebsd10-amd64  4 host-install(4)   broken pass in 148236
 test-amd64-amd64-xl-xsm   4 host-install(4)  broken pass in 148236
 test-amd64-i386-pair  5 host-install/dst_host(5) broken pass in 148236
 test-amd64-i386-xl-qemut-debianhvm-amd64 4 host-install(4) broken pass in 
148236
 test-xtf-amd64-amd64-54 host-install(4)  broken pass in 148236
 test-amd64-amd64-xl-qemuu-win7-amd64  4 host-install(4)  broken pass in 148236
 test-amd64-amd64-amd64-pvgrub  4 host-install(4) broken pass in 148236
 test-amd64-i386-libvirt-xsm   4 host-install(4)  broken pass in 148236
 test-amd64-i386-libvirt   4 host-install(4)  broken pass in 148236
 test-amd64-i386-qemuu-rhel6hvm-intel  4 host-install(4)  broken pass in 148236
 test-amd64-i386-qemut-rhel6hvm-amd  4 host-install(4)broken pass in 148236
 test-amd64-amd64-xl-qcow2 4 host-install(4)  broken pass in 148236
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken 
pass in 148236
 test-amd64-amd64-xl-credit1   4 host-install(4)  broken pass in 148236
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken 
pass in 148236
 test-amd64-i386-xl-raw4 host-install(4)  broken pass in 148236
 test-amd64-amd64-xl-credit2  20 guest-start/debian.repeat  fail pass in 148236
 test-amd64-i386-xl-xsm   20 guest-start/debian.repeat  fail pass in 148236
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 18 guest-start/debianhvm.repeat 
fail pass in 148236
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 18 guest-start/debianhvm.repeat 
fail pass in 148236
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 18 
guest-start/debianhvm.repeat fail pass in 148236

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds   16 guest-start/debian.repeat fail blocked in 148098
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop  fail in 148236 like 148098
 test-amd64-i386-libvirt 13 migrate-support-check fail in

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

2020-03-11 Thread Vladimir Sementsov-Ogievskiy


10.03.2020 18:47, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


09.03.2020 12:56, Markus Armbruster wrote:

Suggest

  scripts: Coccinelle script to use auto-propagated errp

or

  scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy  writes:


Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
   --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
   blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]


Suggest FILES... instead of a specific set of files.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Stefan Hajnoczi 
Cc: "Philippe Mathieu-Daudé" 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Stefan Berger 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: qemu-bl...@nongnu.org
Cc: qemu-de...@nongnu.org
Cc: xen-devel@lists.xenproject.org

   include/qapi/error.h  |   3 +
   scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
   2 files changed, 234 insertions(+)
   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

diff --git a/include/qapi/error.h b/include/qapi/error.h
index bb9bcf02fb..fbfc6f1c0b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -211,6 +211,9 @@
* }
* ...
* }
+ *
+ * For mass conversion use script


mass-conversion (we're not converting mass, we're converting en masse)


+ *   scripts/coccinelle/auto-propagated-errp.cocci
*/
 #ifndef ERROR_H
diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
b/scripts/coccinelle/auto-propagated-errp.cocci
new file mode 100644
index 00..bff274bd6d
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci


Preface to my review of this script: may aim isn't to make it
bullet-proof.  I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.

The latter may involve additional scripting.  That's okay.

The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.

When I ask for improvements that, in your opinion, go beyond "good
enough", please push back.  I'm sure we can work it out together.


@@ -0,0 +1,231 @@
+// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see .
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
+//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
+//  --max-width 80 blockdev-nbd.c qemu-nbd.c \


You have --max-width 80 here, but not in the commit message.  Default
seems to be 78.  Any particular reason to change it to 80?


Hmm. As I remember, without this parameter, reindenting doesn't work correctly.
So, I'm OK with "--max-width 78", but I doubt that it will work without a 
parameter.
Still, may be I'm wrong, we can check it.


If you can point to an example where --max-width helps, keep it, and
update the commit message to match.  Else, drop it.




+//  {block/nbd*,nbd/*,include/block/nbd*}.[hc]
+
+// Switch unusual (Error **) parameter names to errp


Let's drop the parenthesis around Error **


+// (this is necessary to use ERRP_AUTO_PROPAGATE).


Perhaps ERRP_AUTO_PROPAGATE() should be ERRP_AUTO_PROPAGATE(errp) to
make the fact we're messing with @errp more obvious.  Too late; I
shouldn't rock the boat that much now.


+//
+// Disable optional_qualifier to skip functions with "Error *const *errp"
+// parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, as they have
+// non generic semantics and may have unusual Error ** argument name for 
purpose


non-generic

for a purpose

Wrap comment lines around column 70, please.  It's easier to read.

Maybe

 // Skip functions with "assert(_errp && *_errp)" statement, because that
 // signals unusual semantics, and the parameter name may well serve a
 // purpose.


Sounds good.




+// (like

Re: [Xen-devel] [PATCH -next 020/491] XEN HYPERVISOR INTERFACE: Use fallthrough;

2020-03-11 Thread Jürgen Groß


On 11.03.20 05:51, Joe Perches wrote:

Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches 


Reviewed-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

2020-03-11 Thread Jürgen Groß


On 10.03.20 18:02, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

--- a/xen/common/softirq.c
+++ b/xen/common/softirq.c
@@ -25,7 +25,7 @@ static softirq_handler softirq_handlers[NR_SOFTIRQS];
  static DEFINE_PER_CPU(cpumask_t, batch_mask);
  static DEFINE_PER_CPU(unsigned int, batching);
  
-static void __do_softirq(unsigned long ignore_mask)

+static void __do_softirq(unsigned long ignore_mask, bool rcu_allowed)


Why the separate bool? Can't you ...


@@ -38,7 +38,7 @@ static void __do_softirq(unsigned long ignore_mask)
   */
  cpu = smp_processor_id();
  
-if ( rcu_pending(cpu) )

+if ( rcu_allowed && rcu_pending(cpu) )


... check !(ignore_mask & RCU_SOFTIRQ) here?


Good idea.




@@ -55,13 +55,22 @@ void process_pending_softirqs(void)
  {
  ASSERT(!in_irq() && local_irq_is_enabled());
  /* Do not enter scheduler as it can preempt the calling context. */
-__do_softirq((1ul << SCHEDULE_SOFTIRQ) | (1ul << SCHED_SLAVE_SOFTIRQ));
+__do_softirq((1ul << SCHEDULE_SOFTIRQ) | (1ul << SCHED_SLAVE_SOFTIRQ),
+ true);
+}
+
+void process_pending_softirqs_norcu(void)
+{
+ASSERT(!in_irq() && local_irq_is_enabled());
+/* Do not enter scheduler as it can preempt the calling context. */
+__do_softirq((1ul << SCHEDULE_SOFTIRQ) | (1ul << SCHED_SLAVE_SOFTIRQ) |
+ (1ul << RCU_SOFTIRQ), false);


I guess the comment here also wants to mention RCU?


Yes.




--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, 
int level,
  struct amd_iommu_pte *pde = _vaddr[index];
  
  if ( !(index % 2) )

-process_pending_softirqs();
+process_pending_softirqs_norcu();


At the example of this - the property of holding an RCU lock is
entirely invisible here, as it's the generic
iommu_dump_p2m_table() which acquires it. This suggest to me that
going forward breaking this is going to be very likely. Couldn't
process_pending_softirqs() exclude RCU handling when finding
preempt_count() to return non-zero?


This can be done, but then the non-debug build would require to have
non-empty rcu lock functions.

An alternative would be to ASSERT() no rcu lock being held in
process_pending_softirqs() or rcu_check_callbacks() which would catch
the problematic cases in debug builds.


Juergen

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

Re: [Xen-devel] [PATCH v4 2/6] xen/rcu: don't use stop_machine_run() for rcu_barrier()

2020-03-11 Thread Jürgen Groß


On 10.03.20 17:37, Jan Beulich wrote:

On 10.03.2020 17:34, Jürgen Groß wrote:

On 10.03.20 17:29, Jan Beulich wrote:

On 10.03.2020 08:28, Juergen Gross wrote:

+void rcu_barrier(void)
   {
-atomic_t cpu_count = ATOMIC_INIT(0);
-return stop_machine_run(rcu_barrier_action, _count, NR_CPUS);
+unsigned int n_cpus;
+
+while ( !get_cpu_maps() )
+{
+process_pending_softirqs();
+if ( !atomic_read(_count) )
+return;
+
+cpu_relax();
+}
+
+n_cpus = num_online_cpus();
+
+if ( atomic_cmpxchg(_count, 0, n_cpus) == 0 )
+{
+atomic_add(n_cpus, _count);
+cpumask_raise_softirq(_online_map, RCU_SOFTIRQ);
+}
+
+while ( atomic_read(_count) )


Don't you leave a window for races here, in that done_count
gets set to non-zero only after setting cpu_count? A CPU
losing the cmpxchg attempt above may observe done_count
still being zero, and hence exit without waiting for the
count to actually _drop_ to zero.


This can only be a cpu not having joined the barrier handling, so it
will do that later.


I'm afraid I don't understand - if two CPUs independently call
rcu_barrier(), neither should fall through here without waiting
at all, I would think?


Oh, good catch!

I have thought more about this problem and I think using counters only
for doing rendezvous accounting is rather risky. I'll have a try using
a cpumask instead.


Juergen

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

84 matches

Mail list logo