Re: [Xen-devel] [PATCH 00/12 v3] PL011 emulation support in Xen

2017-05-11 Thread Julien Grall



On 11/05/2017 03:44, Bhupinder Thakur wrote:

Hi Julien,


Hi Bhupinder,


I will resend the series keeping you and Stefano in cc.


No need to series your series :).

Cheers,

--
Julien Grall

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


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

2017-05-11 Thread osstest service owner
flight 109300 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109300/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf ef810bc807188224a752ffbcf5e7f4b651291cee
baseline version:
 ovmf bcd999d2377d1a34bb3b5c02e4206a8d14a2

Last test of basis   109291  2017-05-11 01:48:21 Z0 days
Testing same since   109300  2017-05-11 04:00:33 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 
  Jiaxin Wu 
  Wu Jiaxin 

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



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

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

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

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


Pushing revision :

+ branch=ovmf
+ revision=ef810bc807188224a752ffbcf5e7f4b651291cee
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
ef810bc807188224a752ffbcf5e7f4b651291cee
+ branch=ovmf
+ revision=ef810bc807188224a752ffbcf5e7f4b651291cee
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' xef810bc807188224a752ffbcf5e7f4b651291cee = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/h

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

2017-05-11 Thread osstest service owner
flight 109269 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109269/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-winxpsp3 16 guest-stop  fail REGR. vs. 109165

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 109112
 test-amd64-i386-xl-qemut-winxpsp3 16 guest-stop   fail like 109112
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 109165
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail like 109165
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail like 109165
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 109165
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 109165
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 109165
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 109165
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 109165
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 16 guest-stopfail like 109165
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-arm64-arm64-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-arm64-arm64-libvirt 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-rtds 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-rtds 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 12 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  0d1a96043a75b067498a33619e99df1276f4c1b1
baseline version:
 xen  8839be5c1fe339a1310b4e05e88c5a0230b7959d

Last test of basis   109165  2017-05-08 07:17:46 Z3 days
Failing since109186  2017-05-08 19:20:10 Z2 days5 attempts
Testing

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

2017-05-11 Thread osstest service owner
flight 109297 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109297/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   5 xen-buildfail REGR. vs. 107636
 build-arm64   5 xen-buildfail REGR. vs. 107636
 build-i3865 xen-buildfail REGR. vs. 107636
 build-amd64-xsm   5 xen-buildfail REGR. vs. 107636
 build-i386-xsm5 xen-buildfail REGR. vs. 107636
 build-amd64   5 xen-buildfail REGR. vs. 107636
 build-armhf   5 xen-buildfail REGR. vs. 107636
 build-armhf-xsm   5 xen-buildfail REGR. vs. 107636

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-intel  1 build-check(1)  blocked n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-winxpsp3  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-pvh-intel  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-

Re: [Xen-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec

2017-05-11 Thread Paolo Bonzini


On 09/05/2017 21:04, Stefano Stabellini wrote:
> Assert that the return value is not an error. This issue was found by
> Coverity.
> 
> CID: 1374831
> 
> Signed-off-by: Stefano Stabellini 
> CC: gr...@kaod.org
> CC: pbonz...@redhat.com
> CC: Eric Blake 

Queued, thanks.

Paolo

> ---
>  util/oslib-posix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 4d9189e..16894ad 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -182,7 +182,9 @@ void qemu_set_cloexec(int fd)
>  {
>  int f;
>  f = fcntl(fd, F_GETFD);
> -fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
> +f = fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
>  }
>  
>  /*
> 

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


Re: [Xen-devel] Is there any limitation on the firmware size in Xen?

2017-05-11 Thread Jan Beulich
>>> On 11.05.17 at 06:10,  wrote:
> On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
>> On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
>> > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
>> > > I was asked the following question which I am posting to the list.
>> > > 
>> > > "
>> > > My name is Gary Lin, and I am the maintainer of the OVMF package, a UEFI
>> > > implementation for the virtual machines.
>> > > 
>> > > Recently, I was testing an upstream patchset[*] and encountered some
>> > > problems in Xen. Maybe you can help me or give me some hints.
>> > > 
>> > > To be short, the edk2/ovmf upstream is going to increase the firmware
>> > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
>> > > different types of VM to make sure the patchset really work. When I was
>> > > using the 2MB build, my Xen HVM worked as expected and showed the boot
>> > > menu. However, if I use the 4MB build, I got something like this from
>> > > "xl dmesg":
>> > > 
>> > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... 
> done.
>> > > (d32) Testing HVM environment:
>> > > (d32)  - REP INSB across page boundaries ... passed
>> > > (d32)  - GS base MSRs and SWAPGS ... passed
>> > > (d32) Passed 2 of 2 tests
>> > > (d32) Writing SMBIOS tables ...
>> > > (d32) Loading OVMF ...
>> > > (d32) no BIOS ROM image found
>> > > (d32) *** HVMLoader bug at hvmloader.c:389
>> > > (d32) *** HVMLoader crashed.
>> > > 
>> > > I tried to trace the code and found that in 
> libxl__load_hvm_firmware_module()
>> > > in tools/libxl/libxl_dom.c actully loaded the file and 
> add_module_to_list()
>> > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 4194304
>> > > bytes. However, when hvmloader loaded "bios_module" with 
> get_module_entry(),
>> > > modlist is NULL. It seems the firmware data was removed for some reason.
>> > > 
>> > > Here are my questions:
>> > > 
>> > > 1. Is there any limitation on the firmware size in Xen?
>> > > 
>> > 
>> > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
>> > that regard. HVMloader should be happy with that address range.
>> 
>> Oh wait, it hasn't gone that far into loading OVMF.
>> 
>> IT would be useful, as a starting point, to go through modlist, print
>> out and compare module loading addresses and lengths from both libxc and
>> hvmloader.
>> 
> 
> I printed the address and contents of hvm_start_info in get_module_entry().
> Here is the result.
> 
> 4MB build:
> 
> (d39) get_module_entry info 0x588000
> (d39) info->magic 0x0
> (d39) info->version   0x0
> (d39) info->flags 0x0
> (d39) info->nr_modules0x0
> (d39) info->modlist_paddr 0x0
> (d39) info->cmdline_paddr 0x0
> (d39) info->rsdp_paddr0x0
> 
> Obviously, it's corrupted since magic is 0.

Note that hvmloader's main() has

BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);

very early, so you having got past this means the corruption
occurred inside hvmloader (or at least while it was already
running). Could you comment out the call to perform_tests()
and try again?

Jan


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


Re: [Xen-devel] [PATCH 2/2] libxl/devd: prevent adding spurious domains to the list

2017-05-11 Thread Roger Pau Monne
On Wed, May 10, 2017 at 05:29:12PM +0100, Wei Liu wrote:
> On Wed, May 10, 2017 at 11:12:39AM +0100, Roger Pau Monne wrote:
> > Current code in backend_watch_callback forgets to remove a 
> > libxl__ddomain_guest
> > from the list of tracked domains when the related data is freed, causing
> > dereferences later on when the list is traversed. Make sure that a domain is
> > always removed from the list when freed.
> 
> There is another bug: dguest was freed by a spurious event. That is also
> fixed by this patch.
> 
> I would suggest changing the tite to "correctly manipulate the dguest
> list" and also extend the commit message to include that other bug.

What about the following commit message:

libxl/devd: correctly manipulate the dguest list

Current code in backend_watch_callback has two issues when manipulating the
dgest list:

1. backend_watch_callback forgets to remove a libxl__ddomain_guest from the
list of tracked domains when the related data is freed, causing dereferences
later on when the list is traversed. Make sure that a domain is always removed
from the list when freed.

2. A spurious device state change can cause a dguest to be freed, with active
devices and without being removed from the list. Fix this by always checking if
a dguest has active devices before freeing and removing it.

Let me know if you want me to resend the patch or if you will fix the message
while committing.

> Code-wise:
> 
> Reviewed-by: Wei Liu 

Thanks.

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


[Xen-devel] [distros-debian-wheezy test] 71286: tolerable trouble: broken/pass

2017-05-11 Thread Platform Team regression test user
flight 71286 distros-debian-wheezy real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71286/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 build-arm64   2 hosts-allocate   broken never pass
 build-arm64-pvops 2 hosts-allocate   broken never pass
 build-arm64   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass

baseline version:
 flight   71255

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-wheezy-netboot-pvgrub pass
 test-amd64-i386-i386-wheezy-netboot-pvgrub   pass
 test-amd64-i386-amd64-wheezy-netboot-pygrub  pass
 test-amd64-amd64-i386-wheezy-netboot-pygrub  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


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


Re: [Xen-devel] [block-xen-blkback] question about pontential null pointer dereference

2017-05-11 Thread Juergen Gross
On 10/05/17 18:49, Gustavo A. R. Silva wrote:
> 
> Hello everybody,
> 
> While looking into Coverity ID 1350942 I ran into the following piece of
> code at drivers/block/xen-blkback/xenbus.c:490:
> 
> 490static int xen_blkbk_remove(struct xenbus_device *dev)
> 491{
> 492struct backend_info *be = dev_get_drvdata(&dev->dev);
> 493
> 494pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
> 495
> 496if (be->major || be->minor)
> 497xenvbd_sysfs_delif(dev);
> 498
> 499if (be->backend_watch.node) {
> 500unregister_xenbus_watch(&be->backend_watch);
> 501kfree(be->backend_watch.node);
> 502be->backend_watch.node = NULL;
> 503}
> 504
> 505dev_set_drvdata(&dev->dev, NULL);
> 506
> 507if (be->blkif)
> 508xen_blkif_disconnect(be->blkif);
> 509
> 510/* Put the reference we set in xen_blkif_alloc(). */
> 511xen_blkif_put(be->blkif);
> 512kfree(be->mode);
> 513kfree(be);
> 514return 0;
> 515}
> 
> The issue here is that line 507 implies that be->blkif might be NULL. If
> this is the case, there is a NULL pointer dereference when executing
> line 511 once macro xen_blkif_put() dereference be->blkif
> 
> Is there any chance for be->blkif to be NULL at line 511?

Yes. xen_blkbk_probe() will call xen_blkbk_remove() with be->blkif being
NULL in the failure path.

The call to xen_blkif_put() should be guarded by the "if (be->blkif)" of
line 507, too.


Juergen

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


Re: [Xen-devel] Is there any limitation on the firmware size in Xen?

2017-05-11 Thread Gary Lin
On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
> >>> On 11.05.17 at 06:10,  wrote:
> > On Wed, May 10, 2017 at 04:47:49PM +0100, Wei Liu wrote:
> >> On Wed, May 10, 2017 at 04:39:32PM +0100, Wei Liu wrote:
> >> > On Wed, May 10, 2017 at 08:29:35AM -0600, Charles Arnold wrote:
> >> > > I was asked the following question which I am posting to the list.
> >> > > 
> >> > > "
> >> > > My name is Gary Lin, and I am the maintainer of the OVMF package, a 
> >> > > UEFI
> >> > > implementation for the virtual machines.
> >> > > 
> >> > > Recently, I was testing an upstream patchset[*] and encountered some
> >> > > problems in Xen. Maybe you can help me or give me some hints.
> >> > > 
> >> > > To be short, the edk2/ovmf upstream is going to increase the firmware
> >> > > size from 2MB to 4MB to fulfill windows HCK, and we have to test
> >> > > different types of VM to make sure the patchset really work. When I was
> >> > > using the 2MB build, my Xen HVM worked as expected and showed the boot
> >> > > menu. However, if I use the 4MB build, I got something like this from
> >> > > "xl dmesg":
> >> > > 
> >> > > (d32)  - CPU0 ... 39-bit phys ... fixed MTRRs ... var MTRRs [1/8] ... 
> > done.
> >> > > (d32) Testing HVM environment:
> >> > > (d32)  - REP INSB across page boundaries ... passed
> >> > > (d32)  - GS base MSRs and SWAPGS ... passed
> >> > > (d32) Passed 2 of 2 tests
> >> > > (d32) Writing SMBIOS tables ...
> >> > > (d32) Loading OVMF ...
> >> > > (d32) no BIOS ROM image found
> >> > > (d32) *** HVMLoader bug at hvmloader.c:389
> >> > > (d32) *** HVMLoader crashed.
> >> > > 
> >> > > I tried to trace the code and found that in 
> > libxl__load_hvm_firmware_module()
> >> > > in tools/libxl/libxl_dom.c actully loaded the file and 
> > add_module_to_list()
> >> > > in tools/libxc/xc_dom_x86.c was loading a firmware "module" with 
> >> > > 4194304
> >> > > bytes. However, when hvmloader loaded "bios_module" with 
> > get_module_entry(),
> >> > > modlist is NULL. It seems the firmware data was removed for some 
> >> > > reason.
> >> > > 
> >> > > Here are my questions:
> >> > > 
> >> > > 1. Is there any limitation on the firmware size in Xen?
> >> > > 
> >> > 
> >> > OVMF is loaded into 4GB - ovmf_size. There shouldn't be limitation in
> >> > that regard. HVMloader should be happy with that address range.
> >> 
> >> Oh wait, it hasn't gone that far into loading OVMF.
> >> 
> >> IT would be useful, as a starting point, to go through modlist, print
> >> out and compare module loading addresses and lengths from both libxc and
> >> hvmloader.
> >> 
> > 
> > I printed the address and contents of hvm_start_info in get_module_entry().
> > Here is the result.
> > 
> > 4MB build:
> > 
> > (d39) get_module_entry info 0x588000
> > (d39) info->magic 0x0
> > (d39) info->version   0x0
> > (d39) info->flags 0x0
> > (d39) info->nr_modules0x0
> > (d39) info->modlist_paddr 0x0
> > (d39) info->cmdline_paddr 0x0
> > (d39) info->rsdp_paddr0x0
> > 
> > Obviously, it's corrupted since magic is 0.
> 
> Note that hvmloader's main() has
> 
> BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
> 
> very early, so you having got past this means the corruption
> occurred inside hvmloader (or at least while it was already
> running). Could you comment out the call to perform_tests()
> and try again?
> 
You got it. After commenting out perform_tests(), the grub2 menu showed
and the system booted.

It seems that perform_tests() cleared 0x40~0x80, and that's why 
the members of hvm_start_info became 0 in my test.

Thanks!

Gary Lin


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


[Xen-devel] [ovmf baseline-only test] 71287: tolerable FAIL

2017-05-11 Thread Platform Team regression test user
This run is configured for baseline tests only.

flight 71287 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71287/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-libvirt   5 libvirt-buildfail   like 71285
 build-i386-libvirt5 libvirt-buildfail   like 71285

version targeted for testing:
 ovmf ef810bc807188224a752ffbcf5e7f4b651291cee
baseline version:
 ovmf bcd999d2377d1a34bb3b5c02e4206a8d14a2

Last test of basis71285  2017-05-11 04:16:37 Z0 days
Testing same since71287  2017-05-11 07:48:51 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 
  Jiaxin Wu 
  Wu Jiaxin 

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



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


commit ef810bc807188224a752ffbcf5e7f4b651291cee
Author: Jiaxin Wu 
Date:   Wed May 10 23:30:57 2017 +0800

NetworkPkg/IScsiDxe: Switch IP4 configuration policy to Static before DHCP

DHCP4 service allows only one of its children to be configured in the active
state. If the DHCP4 D.O.R.A started by IP4 auto configuration and has not
been completed, the Dhcp4 state machine will not be in the right state for
the iSCSI to start a new round D.O.R.A. So, we need to switch it's policy to
static.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
Reviewed-by: Ye Ting 
Reviewed-by: Fu Siyuan 

commit df5914993c9d4db87e4132bcc3b88efb48db5c6e
Author: Dandan Bi 
Date:   Wed May 10 09:47:06 2017 +0800

MdeModulePkg/FormDisplay: Make the LineWidth of option consistent

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=529

LineWidth of option in funcrion UpdateSkipInfoForMenu and DisplayOneMenu
are inconsistent. Now fix this issue to avoid incorrect UI display.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 

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


Re: [Xen-devel] [ARM] Native application design and discussion (I hope)

2017-05-11 Thread Julien Grall

Hello,

On 10/05/17 20:04, Julien Grall wrote:

On 05/10/2017 06:37 PM, Volodymyr Babchuk wrote:

Hi Julien,


Hi Volodymyr,


Returning back to Native apps, I think we can make ctx switch even
faster by dropping p2m code. Imagine that we already created stage 1
MMU for native application. Then to switch to app it we need only:

1. Enable TGE bit in HCR
2. Disable VM bit in HCR
3. Save/Program EL1_TTBR and friends
3.5 (optionally) save/restore FPU state
4. Save/Restore general purpose registers + SP + CSR + PC to jump to
an app in EL0 state.

This can be done in "real" vcpu or in idle vcpu context. No
differences there.

Exception handling in hypervisor would became tricky because of vcpu
absence for native app. Current implementation of entry.S always says
general purpose registers to a vcpu structure. Basically, we should
teach entry.S and traps.c about native apps.
Am I missing something?


HCR_EL2.VM is allowed to be cached in the TLBs so for correctness you
have to flush the TLBs everytime you change this bit (see D4.8.3 in ARM
DDI 0487A.k_iss10775).

Furthermore, as I mentioned earlier (see [1]) there are dependencies on
the VMID even when stage-2 is disabled (see D4-1823 in ARM DDI
0487A.k_iss10775) so you have to program correctly VTTBR_EL2.VMID. This
also means that if you use a different EL0 app, you have to ther use a
different VMID or flush the TLBs.

Bottom line, if you don't use stage-2 page table you have to flush the
TLBs. Likely this will have an higher impact on the platform than using
stage-2 page table.

Virtual memory is quite tricky, someone needs to look at the ARM ARM and
check all the behaviors when disabling either stage-1 or stage-2. There
are memory attribute implications that may make tricky to move an EL0
app between pCPU.


Looking again at the documentation and chatting with other ARM folks. I 
was wrong on some part, sorry for the confusion.


It turns out that if you don't need to flush the TLBs when disabling the 
HCR_EL2.VM (this is what Linux does for KVM). So disabling stage-2 for 
EL0 app would be ok.


But you still need to allocate a VMID per EL0 app as TLBs will still 
depend on it even with stage-2 disabled.


Even if we keep stage-2 enabled, we would have to create dummy page 
tables of stage-1 because the memory attribute would impact performance 
and at least not allow the EL0 app to move (see D4.2.8 in ARM DDI 
0487A.k_iss10775). In this case, 1:1 page tables with a block map (e.g 
1GB) would be sufficient and rely on stage-2 page tables.


Lastly, can you remind me with platform you are using for testing?

I hope this helps.

Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2 04/18] x86/traps: move all PV emulation code to pv/emulate_ops.h

2017-05-11 Thread Andrew Cooper
On 05/05/17 15:48, Wei Liu wrote:
> Move the following emulation code from traps.c:
>
> 1. invalid op
> 2. rdtsc
> 3. privilege instructions
> 4. gate operation
> 5. pv cpuid emulation
>
> Export the emulate_* functions via pv/traps.h.
>
> No functional change.
>
> Signed-off-by: Wei Liu 

As you are moving all emulation code, arch/x86/x86_64/gpr_switch.S
should move along as well.

> ---
>  xen/arch/x86/pv/Makefile   |1 +
>  xen/arch/x86/pv/emulate_ops.c  | 1929 
> 
>  xen/arch/x86/traps.c   | 1880 +--
>  xen/include/asm-x86/pv/traps.h |   54 ++
>  4 files changed, 1985 insertions(+), 1879 deletions(-)
>  create mode 100644 xen/arch/x86/pv/emulate_ops.c
>  create mode 100644 xen/include/asm-x86/pv/traps.h
>
> diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile
> index 489a9f59cb..ef3cecc463 100644
> --- a/xen/arch/x86/pv/Makefile
> +++ b/xen/arch/x86/pv/Makefile
> @@ -3,3 +3,4 @@ obj-y += traps.o
>  
>  obj-bin-y += dom0_build.init.o
>  obj-y += domain.o
> +obj-y += emulate_ops.o
> diff --git a/xen/arch/x86/pv/emulate_ops.c b/xen/arch/x86/pv/emulate_ops.c
> new file mode 100644
> index 00..5f0965e05b
> --- /dev/null
> +++ b/xen/arch/x86/pv/emulate_ops.c

Why emulate_ops?  What about just plain emulate.c ?

>
> 
>
> +return X86EMUL_UNHANDLEABLE;
> +}
> +
> +#include "../x86_64/mmconfig.h"

Please can this be moved sensibly to the head of the file?

~Andrew

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


Re: [Xen-devel] [PATCH v3 4/9] mm: Scrub memory from idle loop

2017-05-11 Thread Dario Faggioli
On Fri, 2017-04-14 at 11:37 -0400, Boris Ostrovsky wrote:
> Instead of scrubbing pages during guest destruction (from
> free_heap_pages()) do this opportunistically, from the idle loop.
> 
> Signed-off-by: Boris Ostrovsky 
> ---
> Changes in v3:
> * If memory-only nodes exist, select the closest one for scrubbing
> * Don't scrub from idle loop until we reach SYS_STATE_active.
> 
>  xen/arch/arm/domain.c   |   13 --
>  xen/arch/x86/domain.c   |3 +-
>  xen/common/page_alloc.c |   98
> +-
>  xen/include/xen/mm.h|1 +
>  4 files changed, 98 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 76310ed..38d6331 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -46,13 +46,16 @@ void idle_loop(void)
>  if ( cpu_is_offline(smp_processor_id()) )
>  stop_cpu();
>  
> -local_irq_disable();
> -if ( cpu_is_haltable(smp_processor_id()) )
> +if ( !scrub_free_pages() )
>  {
> -dsb(sy);
> -wfi();
> +local_irq_disable();
> +if ( cpu_is_haltable(smp_processor_id()) )
> +{
> +dsb(sy);
> +wfi();
> +}
> +local_irq_enable();
>  }
> -local_irq_enable();
>  
>  do_tasklet();
>  do_softirq();
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 90e2b1f..a5f62b5 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -118,7 +118,8 @@ static void idle_loop(void)
>  {
>  if ( cpu_is_offline(smp_processor_id()) )
>  play_dead();
> -(*pm_idle)();
> +if ( !scrub_free_pages() )
> +(*pm_idle)();
>  do_tasklet();
>
This means that, if we got here to run a tasklet (as in, if the idle
vCPU has been forced into execution, because there were a vCPU context
tasklet wanting to run), we will (potentially) do some scrubbing first.

Is this on purpose, and, in any case, ideal? vCPU context tasklets are
not terribly common, but I still don't think it is (ideal).

Not sure how to address this, though. What (the variants of) pm_idle()
uses for deciding whether or not to actually go to sleep is
cpu_is_haltable(), which checks per_cpu(tasklet_work_to_do, cpu):

/*
 * Used by idle loop to decide whether there is work to do:
 *  (1) Run softirqs; or (2) Play dead; or (3) Run tasklets.
 */
#define cpu_is_haltable(cpu)\
(!softirq_pending(cpu) &&   \
 cpu_online(cpu) && \
 !per_cpu(tasklet_work_to_do, cpu))

Pulling it out/adding a call to it (cpu_is_haltable()) is ugly, and
probably not what we want (e.g., it's always called with IRQs disabled,
while they're on here).

Maybe we can test tasklet_work_to_do, before calling scrub_free_pages()
(also ugly, IMO).
Or, if scrub_free_pages() is, and always will be, called only from
here, within the idle loop, test tasklet_work_to_do inside, similarly
to what it does already for pending softirqs...

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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


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

2017-05-11 Thread osstest service owner
flight 109308 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109308/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf a6c31c6d6349a51041d8b77df375c340043e36bd
baseline version:
 ovmf ef810bc807188224a752ffbcf5e7f4b651291cee

Last test of basis   109300  2017-05-11 04:00:33 Z0 days
Testing same since   109308  2017-05-11 07:57:17 Z0 days1 attempts


People who touched revisions under test:
  Star Zeng 
  Yonghong Zhu 

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



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

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

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

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


Pushing revision :

+ branch=ovmf
+ revision=a6c31c6d6349a51041d8b77df375c340043e36bd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
a6c31c6d6349a51041d8b77df375c340043e36bd
+ branch=ovmf
+ revision=a6c31c6d6349a51041d8b77df375c340043e36bd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' xa6c31c6d6349a51041d8b77df375c340043e36bd = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/gi

Re: [Xen-devel] [PATCH v3] x86/amd: don't set X86_BUG_SYSRET_SS_ATTRS when running under Xen

2017-05-11 Thread Juergen Gross
On 27/04/17 07:01, Juergen Gross wrote:
> When running as Xen pv guest X86_BUG_SYSRET_SS_ATTRS must not be set
> on AMD cpus.
> 
> This bug/feature bit is kind of special as it will be used very early
> when switching threads. Setting the bit and clearing it a little bit
> later leaves a critical window where things can go wrong. This time
> window has enlarged a little bit by using setup_clear_cpu_cap() instead
> of the hypervisor's set_cpu_features callback. It seems this larger
> window now makes it rather easy to hit the problem.
> 
> The proper solution is to never set the bit in case of Xen.
> 
> Signed-off-by: Juergen Gross 

Any objections for carrying this through the Xen tree?


Juergen

> ---
>  arch/x86/kernel/cpu/amd.c   | 5 +++--
>  arch/x86/xen/enlighten_pv.c | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c36140d788fe..b6da6e75e3a8 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -799,8 +799,9 @@ static void init_amd(struct cpuinfo_x86 *c)
>   if (cpu_has(c, X86_FEATURE_3DNOW) || cpu_has(c, X86_FEATURE_LM))
>   set_cpu_cap(c, X86_FEATURE_3DNOWPREFETCH);
>  
> - /* AMD CPUs don't reset SS attributes on SYSRET */
> - set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
> + /* AMD CPUs don't reset SS attributes on SYSRET, Xen does. */
> + if (!cpu_has(c, X86_FEATURE_XENPV))
> + set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index a1af4f68278f..dcfd07faf1c3 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -290,7 +290,6 @@ static bool __init xen_check_xsave(void)
>  
>  static void __init xen_init_capabilities(void)
>  {
> - setup_clear_cpu_cap(X86_BUG_SYSRET_SS_ATTRS);
>   setup_force_cpu_cap(X86_FEATURE_XENPV);
>   setup_clear_cpu_cap(X86_FEATURE_DCA);
>   setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
> 


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


Re: [Xen-devel] [PATCH 00/10 v2] pl011 emulation support in Xen

2017-05-11 Thread Wei Liu
On Fri, Apr 28, 2017 at 09:31:14PM +0530, Bhupinder Thakur wrote:
> 
> url: ssh://g...@git.linaro.org:/people/bhupinder.thakur/xen.git

I'm not sure if a ssh:// url is helpful...

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


Re: [Xen-devel] [PATCH 03/10 v2] xen/arm: vpl011: Enable pl011 emulation for a guest domain in Xen

2017-05-11 Thread Wei Liu
On Fri, May 05, 2017 at 02:43:40PM +0100, Julien Grall wrote:
> On 05/05/17 08:10, Bhupinder Thakur wrote:
> > Hi Julien,
> 
> Hi Bhupinder,
> 
> > > > > Hi Jan,
> > > > > 
> > > > > > > @@ -631,6 +632,9 @@ int arch_domain_create(struct domain *d,
> > > > > > > unsigned int domcr_flags,
> > > > > > >  if ( (rc = domain_vtimer_init(d, config)) != 0 )
> > > > > > >  goto fail;
> > > > > > > 
> > > > > > > +if ( domcr_flags & DOMCRF_vuart )
> > > > > > > +if ( (rc = domain_vpl011_init(d, config)) != 0 )
> > > > > > > +goto fail;
> > > > > > >  update_domain_wallclock_time(d);
> > > > > > 
> > > > > > 
> > > > > I am planning to remove the usage of domain creation flag to check
> > > > > whether vuart is enabled/disabled. Please see my next comment. With
> > > > > that change, domain_vpl011_init() will be called always. The
> > > > > domain_vpl011_init() will check whether vuart is enabled or disabled
> > > > > in the config structure passed. If vuart is enabled then it will go
> > > > > ahead with vpl011 initialization else it will return without
> > > > > initializing vpl011.
> > > > 
> > > > 
> > > > Please don't do that. The arch code decides whether domain_vpl011_init
> > > > not the invert.
> > > 
> > > 
> > > I was wondering whether it would be better to defer the PL011 creation to 
> > > a
> > > domctl. This could be called after the domain is created with all the
> > > information required (MMIO region, Console PFN...).
> > > 
> > > This would also make the migration support more trivial as the we will not
> > > need to know in advance whether a UART is been used.
> > > 
> > > Any opinions?
> > 
> > Would there be race condition where the guest tries to access the
> > pl011 mmio region (as the domain has been created) but pl011 is not
> > initialized yet as domctl is not called? What could be an appropriate
> > place to call this domctl? It should be before xenstore is populated
> > with vuart ring-ref/port information.
> 
> There are no race condition. The domain will only be started when everything
> has been created by calling XEN_DOMCTL_unpausedomain.
> 
> The DOMCTL createdomain only initialize the basic structure for the domain,
> after the hypercall the domain is not in state to be run because, for
> instance, the vCPUs were not allocated (see XEN_DOMCTL_max_vcpus) and the
> guest RAM were not populated.
> 
> I am not very familiar with the libxl code, but I think
> libxl__arch_domain_create should be a good candidate. I will let Ian and Wei
> confirm that.
> 

That sounds reasonable.

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


[Xen-devel] [PATCH V4] x86/ioreq_server: Make p2m_finish_type_change actually work

2017-05-11 Thread Xiong Zhang
Commit 6d774a951696 ("x86/ioreq server: synchronously reset outstanding
p2m_ioreq_server entries when an ioreq server unmaps") introduced
p2m_finish_type_change(), which was meant to synchronously finish a
previously initiated type change over a gpfn range.  It did this by
calling get_entry(), checking if it was the appropriate type, and then
calling set_entry().

Unfortunately, a previous commit (1679e0df3df6 "x86/ioreq server:
asynchronously reset outstanding p2m_ioreq_server entries") modified
get_entry() to always return the new type after the type change, meaning
that p2m_finish_type_change() never changed any entries.  Which means
when an ioreq server was detached and then re-attached (as happens in
XenGT on reboot) the re-attach failed.

Fix this by using the existing p2m-specific recalculation logic instead
of doing a read-check-write loop.

Fix: 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
  outstanding p2m_ioreq_server entries when an ioreq server unmaps")'

Signed-off-by: Xiong Zhang 
Signed-off-by: Yu Zhang 
Reviewed-by: George Dunlap 
---
v1: Add ioreq_pre_recalc query flag to get the old p2m_type.(Jan)
v2: Add p2m->recalc() hook to change gfn p2m_type. (George)
v3: Make commit message clearer. (George)
Keep the name of p2m-specific recal function unchanged. (Jan)
v4: Move version info below S-o-B and handle return value of
p2m->recalc. (Jan)
---
 xen/arch/x86/hvm/dm.c |  5 +++--
 xen/arch/x86/mm/p2m-ept.c |  1 +
 xen/arch/x86/mm/p2m-pt.c  |  1 +
 xen/arch/x86/mm/p2m.c | 35 +++
 xen/include/asm-x86/p2m.h |  7 ---
 5 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index d72b7bd..99bf66a 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -412,8 +412,9 @@ static int dm_op(domid_t domid,
 first_gfn <= p2m->max_mapped_pfn )
 {
 /* Iterate p2m table for 256 gfns each time. */
-p2m_finish_type_change(d, _gfn(first_gfn), 256,
-   p2m_ioreq_server, p2m_ram_rw);
+rc = p2m_finish_type_change(d, _gfn(first_gfn), 256);
+if ( rc < 0 )
+break;
 
 first_gfn += 256;
 
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f37a1f2..09efba7 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1238,6 +1238,7 @@ int ept_p2m_init(struct p2m_domain *p2m)
 
 p2m->set_entry = ept_set_entry;
 p2m->get_entry = ept_get_entry;
+p2m->recalc = resolve_misconfig;
 p2m->change_entry_type_global = ept_change_entry_type_global;
 p2m->change_entry_type_range = ept_change_entry_type_range;
 p2m->memory_type_changed = ept_memory_type_changed;
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 5079b59..2eddeee 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -1153,6 +1153,7 @@ void p2m_pt_init(struct p2m_domain *p2m)
 {
 p2m->set_entry = p2m_pt_set_entry;
 p2m->get_entry = p2m_pt_get_entry;
+p2m->recalc = do_recalc;
 p2m->change_entry_type_global = p2m_pt_change_entry_type_global;
 p2m->change_entry_type_range = p2m_pt_change_entry_type_range;
 p2m->write_p2m_entry = paging_write_p2m_entry;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 1d57e5c..668c5a6 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1011,33 +1011,44 @@ void p2m_change_type_range(struct domain *d,
 p2m_unlock(p2m);
 }
 
-/* Synchronously modify the p2m type for a range of gfns from ot to nt. */
-void p2m_finish_type_change(struct domain *d,
-gfn_t first_gfn, unsigned long max_nr,
-p2m_type_t ot, p2m_type_t nt)
+/*
+ * Finish p2m type change for gfns which are marked as need_recalc in a range.
+ * Returns: 0/1 for success, negative for failure
+ */
+int p2m_finish_type_change(struct domain *d,
+   gfn_t first_gfn, unsigned long max_nr)
 {
 struct p2m_domain *p2m = p2m_get_hostp2m(d);
-p2m_type_t t;
 unsigned long gfn = gfn_x(first_gfn);
 unsigned long last_gfn = gfn + max_nr - 1;
-
-ASSERT(ot != nt);
-ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
+int rc = 0;
 
 p2m_lock(p2m);
 
 last_gfn = min(last_gfn, p2m->max_mapped_pfn);
 while ( gfn <= last_gfn )
 {
-get_gfn_query_unlocked(d, gfn, &t);
-
-if ( t == ot )
-p2m_change_type_one(d, gfn, t, nt);
+rc = p2m->recalc(p2m, gfn);
+/* 
+ * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return
+ * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
+ * gfn here.
+ */
+if ( rc == -ENOENT)
+rc = 0;
+else if ( rc < 0 )
+{
+gdprintk(XENLOG_ERR, "p2m->recalc failed! Dom%d gfn=%lx\n

Re: [Xen-devel] [PATCH v8 23/27] ARM: vITS: handle INV command

2017-05-11 Thread Julien Grall



On 10/05/17 16:11, Andre Przywara wrote:

Hi,


Hi Andre,


On 12/04/17 18:20, Julien Grall wrote:

On 12/04/17 01:44, Andre Przywara wrote:


+{
+ASSERT(spin_is_locked(&v->arch.vgic.lock));


The locking is likely to wrong here too (see patch #2). For instance
with a MOVI then INV on interrupt enabled.


+
+if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
+{
+if ( !list_empty(&p->inflight) &&
+ !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
+gic_raise_guest_irq(v, vlpi, p->lpi_priority);
+}
+else
+{
+clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
+list_del_init(&p->lr_queue);
+}
+}
+
+static int its_handle_inv(struct virt_its *its, uint64_t *cmdptr)
+{
+struct domain *d = its->d;
+uint32_t devid = its_cmd_get_deviceid(cmdptr);
+uint32_t eventid = its_cmd_get_id(cmdptr);
+struct pending_irq *p;
+unsigned long flags;
+struct vcpu *vcpu;
+uint32_t vlpi;
+int ret = -1;
+
+/* Translate the event into a vCPU/vLPI pair. */
+if ( !read_itte(its, devid, eventid, &vcpu, &vlpi) )
+return -1;
+
+if ( vlpi == INVALID_LPI )
+return -1;
+
+spin_lock_irqsave(&vcpu->arch.vgic.lock, flags);
+
+p = d->arch.vgic.handler->lpi_to_pending(d, vlpi);
+if ( !p )
+goto out_unlock;


As said on v5, this could be simpler and use the pending_irqs in the
device. That would be an improvement though. So a would be good.


Originally I found it more straight-forward to use the one existing
interface (the rbtree) we also use in the VGIC part, which would allow
us to handle locking or ref-counting in one central place.
But indeed the ITS command handling has all the data we need to find the
pending_irq directly from the virtual device.
So I replaced all lpi_to_pending() calls in those handlers with a new
function gicv3_its_get_event_pending_irq(), which looks up the struct
from an ITS/device/event triple.
I take and keep the its->lock for the runtime of these functions, so
those events and their memory will not vanish meanwhile.

Does that make sense?


It makes sense to keep the ref-counting in one central place. But it is 
better to avoid reading guest memory and therefore avoid most of 
checking and overhead to translate the IPA to PA.


That's why I suggested to use pending_irqs :).

Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 2/2] libxl/devd: prevent adding spurious domains to the list

2017-05-11 Thread Wei Liu
On Thu, May 11, 2017 at 09:24:40AM +0100, Roger Pau Monne wrote:
> On Wed, May 10, 2017 at 05:29:12PM +0100, Wei Liu wrote:
> > On Wed, May 10, 2017 at 11:12:39AM +0100, Roger Pau Monne wrote:
> > > Current code in backend_watch_callback forgets to remove a 
> > > libxl__ddomain_guest
> > > from the list of tracked domains when the related data is freed, causing
> > > dereferences later on when the list is traversed. Make sure that a domain 
> > > is
> > > always removed from the list when freed.
> > 
> > There is another bug: dguest was freed by a spurious event. That is also
> > fixed by this patch.
> > 
> > I would suggest changing the tite to "correctly manipulate the dguest
> > list" and also extend the commit message to include that other bug.
> 
> What about the following commit message:
> 
> libxl/devd: correctly manipulate the dguest list
> 
> Current code in backend_watch_callback has two issues when manipulating the
> dgest list:
> 
> 1. backend_watch_callback forgets to remove a libxl__ddomain_guest from the
> list of tracked domains when the related data is freed, causing dereferences
> later on when the list is traversed. Make sure that a domain is always removed
> from the list when freed.
> 
> 2. A spurious device state change can cause a dguest to be freed, with active
> devices and without being removed from the list. Fix this by always checking 
> if
> a dguest has active devices before freeing and removing it.
> 
> Let me know if you want me to resend the patch or if you will fix the message
> while committing.
> 

Resending and CC Julien would be best.

> > Code-wise:
> > 
> > Reviewed-by: Wei Liu 
> 
> Thanks.


Wei.

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


[Xen-devel] [PATCH v2 1/2] libxl/devd: fix a race with concurrent device addition/removal

2017-05-11 Thread Roger Pau Monne
Current code can free the libxl__device inside of the libxl__ddomain_device
before the addition has finished if a removal happens while an addition is
still in process:

  backend_watch_callback
|
v
   add_device
| backend_watch_callback
(async operation)   |
|   v
| remove_device
|   |
|   V
|device_complete
| (free libxl__device)
v
 device_complete
  (deref libxl__device)

Fix this by creating a temporary copy of the libxl__device, that's tracked by
the GC of the nested async operation. This ensures that the libxl__device used
by the async operations cannot be freed while being used.

Signed-off-by: Roger Pau Monné 
Reported-by: Ian Jackson 
Reviewed-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Julien Grall 
---
 tools/libxl/libxl_device.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 5e966762c6..cd4ad05a6f 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1415,9 +1415,6 @@ static void device_complete(libxl__egc *egc, 
libxl__ao_device *aodev)
libxl__device_action_to_string(aodev->action),
aodev->rc ? "failed" : "succeed");
 
-if (aodev->action == LIBXL__DEVICE_ACTION_REMOVE)
-free(aodev->dev);
-
 libxl__nested_ao_free(aodev->ao);
 }
 
@@ -1521,7 +1518,12 @@ static int add_device(libxl__egc *egc, libxl__ao *ao,
 
 GCNEW(aodev);
 libxl__prepare_ao_device(ao, aodev);
-aodev->dev = dev;
+/*
+ * Clone the libxl__device to avoid races if remove_device is called
+ * before the device addition has finished.
+ */
+GCNEW(aodev->dev);
+*aodev->dev = *dev;
 aodev->action = LIBXL__DEVICE_ACTION_ADD;
 aodev->callback = device_complete;
 libxl__wait_device_connection(egc, aodev);
@@ -1564,7 +1566,12 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
 
 GCNEW(aodev);
 libxl__prepare_ao_device(ao, aodev);
-aodev->dev = dev;
+/*
+ * Clone the libxl__device to avoid races if there's a add_device
+ * running in parallel.
+ */
+GCNEW(aodev->dev);
+*aodev->dev = *dev;
 aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
 aodev->callback = device_complete;
 libxl__initiate_device_generic_remove(egc, aodev);
@@ -1576,7 +1583,6 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
 goto out;
 }
 libxl__device_destroy(gc, dev);
-free(dev);
 /* Fall through to return > 0, no ao has been dispatched */
 default:
 rc = 1;
@@ -1597,7 +1603,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 char *p, *path;
 const char *sstate, *sonline;
 int state, online, rc, num_devs;
-libxl__device *dev = NULL;
+libxl__device *dev;
 libxl__ddomain_device *ddev = NULL;
 libxl__ddomain_guest *dguest = NULL;
 bool free_ao = false;
@@ -1625,7 +1631,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 goto skip;
 online = atoi(sonline);
 
-dev = libxl__zalloc(NOGC, sizeof(*dev));
+GCNEW(dev);
 rc = libxl__parse_backend_path(gc, path, dev);
 if (rc)
 goto skip;
@@ -1659,7 +1665,8 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
  * to the list of active devices for a given guest.
  */
 ddev = libxl__zalloc(NOGC, sizeof(*ddev));
-ddev->dev = dev;
+ddev->dev = libxl__zalloc(NOGC, sizeof(*ddev->dev));
+*ddev->dev = *dev;
 LIBXL_SLIST_INSERT_HEAD(&dguest->devices, ddev, next);
 LOGD(DEBUG, dev->domid, "Added device %s to the list of active 
devices",
  path);
@@ -1670,9 +1677,6 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 /*
  * Removal of an active device, remove it from the list and
  * free it's data structures if they are no longer needed.
- *
- * The free of the associated libxl__device is left to the
- * helper remove_device function.
  */
 LIBXL_SLIST_REMOVE(&dguest->devices, ddev, libxl__ddomain_device,
next);
@@ -1682,6 +1686,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 if (rc > 0)
 free_ao = true;
 
+free(ddev->dev);
 free(ddev);
 /* If this was the last device in the domain, remove it from the list 
*/
 num_devs = dguest->num_vifs + dguest->num_vbds + dg

[Xen-devel] [PATCH v2 2/2] libxl/devd: correctly manipulate the dguest list

2017-05-11 Thread Roger Pau Monne
Current code in backend_watch_callback has two issues when manipulating the
dguest list:

1. backend_watch_callback forgets to remove a libxl__ddomain_guest from the
list of tracked domains when the related data is freed, causing dereferences
later on when the list is traversed. Make sure that a domain is always removed
from the list when freed.

2. A spurious device state change can cause a dguest to be freed, with active
devices and without being removed from the list. Fix this by always checking if
a dguest has active devices before freeing and removing it.

Let me know if you want me to resend the patch or if you will fix the message
while committing.

Signed-off-by: Roger Pau Monné 
Reported-by: Reinis Martinsons 
Suggested-by: Ian Jackson 
Reviewed-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Julien Grall 

Changes since v1:
 - Fix commit message
---
 tools/libxl/libxl_device.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index cd4ad05a6f..8417198081 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1602,7 +1602,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 STATE_AO_GC(nested_ao);
 char *p, *path;
 const char *sstate, *sonline;
-int state, online, rc, num_devs;
+int state, online, rc;
 libxl__device *dev;
 libxl__ddomain_device *ddev = NULL;
 libxl__ddomain_guest *dguest = NULL;
@@ -1684,21 +1684,9 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
  path);
 rc = remove_device(egc, nested_ao, dguest, ddev);
 if (rc > 0)
-free_ao = true;
+libxl__nested_ao_free(nested_ao);
 
-free(ddev->dev);
-free(ddev);
-/* If this was the last device in the domain, remove it from the list 
*/
-num_devs = dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks;
-if (num_devs == 0) {
-LIBXL_SLIST_REMOVE(&ddomain->guests, dguest, libxl__ddomain_guest,
-   next);
-LOGD(DEBUG, dguest->domid, "Removed domain from the list of active 
guests");
-/* Clear any leftovers in libxl/ */
-libxl__xs_rm_checked(gc, XBT_NULL,
- GCSPRINTF("libxl/%u", dguest->domid));
-free(dguest);
-}
+goto clean;
 }
 
 if (free_ao)
@@ -1708,10 +1696,20 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
 
 skip:
 libxl__nested_ao_free(nested_ao);
+clean:
 if (ddev)
 free(ddev->dev);
 free(ddev);
-free(dguest);
+if (dguest != NULL &&
+dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks == 0) {
+LIBXL_SLIST_REMOVE(&ddomain->guests, dguest, libxl__ddomain_guest,
+   next);
+LOGD(DEBUG, dguest->domid, "Removed domain from the list of active 
guests");
+/* Clear any leftovers in libxl/ */
+libxl__xs_rm_checked(gc, XBT_NULL,
+ GCSPRINTF("libxl/%u", dguest->domid));
+free(dguest);
+}
 return;
 }
 
-- 
2.11.0 (Apple Git-81)


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


[Xen-devel] [PATCH v2 0/2] libxl/devd: bufixes

2017-05-11 Thread Roger Pau Monne
Hello,

The following two patches fix a race with concurrent device addition/removal
and two bugs related to manipulation of the list of active domains in the devd
subcommand.

IMHO they should be part of 4.9 because they are confined to devd code, and
without them devd is unusable (it's trivial to segfault it), so the risk is
low. Worse thing that could happen is that devd crashes, which is already the
case without them.

Thanks, Roger.


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


Re: [Xen-devel] [PATCH 05/12 v3] xen/arm: vpl011: Add new domctl APIs to initialize/de-initialize vpl011

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:01:18PM +0530, Bhupinder Thakur wrote:
> Add two new domctl APIs to initialize and de-initialize vpl011. It takes the 
> GFN and console
> backend domid as input and returns an event channel to be used for
> sending and receiving events from Xen.
> 
> Xen will communicate with xenconsole using GFN as the ring buffer and
> the event channel to transmit and receive pl011 data on guest domain's behalf.
> 
> Signed-off-by: Bhupinder Thakur 
[...]
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5d914a5..e7489d9 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t 
> domid,
>  goto out;
>  }
>  
> +if ( info->vuart &&
> + (ret = xc_dom_vpl011_init(CTX->xch,
> +   domid,
> +   state->console_domid,
> +   dom->vuart_gfn,
> +   &state->vuart_port)) != 0 ) {
> +LOGE(ERROR, "xc_dom_vpl011_init failed");
> +goto out;
> +}

Please push this to arch-specific function.

>  out:
>  return ret != 0 ? ERROR_FAIL : 0;
>  }
> @@ -788,6 +797,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>  if (xc_dom_translated(dom)) {
>  state->console_mfn = dom->console_pfn;
>  state->store_mfn = dom->xenstore_pfn;
> +state->vuart_gfn = dom->vuart_gfn;
>  } else {
>  state->console_mfn = xc_dom_p2m(dom, dom->console_pfn);
>  state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
> diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
> index 08eccd0..1d2c65a 100644
> --- a/tools/libxl/libxl_domain.c
> +++ b/tools/libxl/libxl_domain.c
> @@ -1028,6 +1028,8 @@ void libxl__destroy_domid(libxl__egc *egc, 
> libxl__destroy_domid_state *dis)
>  goto out;
>  }
>  
> +xc_dom_vpl011_deinit(ctx->xch, domid);
> +

Again, arch-specific function please.

>  if (libxl__device_pci_destroy_all(gc, domid) < 0)
>  LOGD(ERROR, domid, "Pci shutdown failed");
>  rc = xc_domain_pause(ctx->xch, domid);
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index 971caec..11707db 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

Please order the header files alphabetically.

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


Re: [Xen-devel] [PATCH v2 1/2] libxl/devd: fix a race with concurrent device addition/removal

2017-05-11 Thread Ian Jackson
Roger Pau Monne writes ("[PATCH v2 1/2] libxl/devd: fix a race with concurrent 
device addition/removal"):
> Current code can free the libxl__device inside of the libxl__ddomain_device
> before the addition has finished if a removal happens while an addition is
> still in process:
...
> Fix this by creating a temporary copy of the libxl__device, that's
> tracked by the GC of the nested async operation. This ensures that
> the libxl__device used by the async operations cannot be freed while
> being used.
...
>  GCNEW(aodev);
>  libxl__prepare_ao_device(ao, aodev);
> -aodev->dev = dev;
> +/*
> + * Clone the libxl__device to avoid races if remove_device is called
> + * before the device addition has finished.
> + */
> +GCNEW(aodev->dev);
> +*aodev->dev = *dev;

This does conveniently disentangle the memory management, so I think
it's a good approach.

But it reads kind of oddly to me.  I think it is not buggy, but can
you add a comment to the definition of libxl__device, saying that it
is a transparent structure containing no external memory references ?

Otherwise this copy is not really justifiable, because in C, in
general, structs might contain private fields, or memory references or
linked list entries or something.

Thanks,
Ian.

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


Re: [Xen-devel] [PATCH 06/12 v3] xen/arm: vpl011: Add a new vuart node in the xenstore

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:12PM +0530, Bhupinder Thakur wrote:
>  libxl__device_console_add(gc, domid, &console, state, &device);
> @@ -1369,14 +1377,22 @@ static void domcreate_launch_dm(libxl__egc *egc, 
> libxl__multidev *multidev,
>  }
>  case LIBXL_DOMAIN_TYPE_PV:
>  {
> -libxl__device_console console;
> -libxl__device device;
> +libxl__device_console console, vuart;
> +libxl__device device, vuart_device;
>  
>  for (i = 0; i < d_config->num_vfbs; i++) {
>  libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
>  libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
>  }
>  
> +if (d_config->b_info.vuart)
> +{
> +init_console_info(gc, &vuart, 0);
> +vuart.backend_domid = state->console_domid;
> +libxl__device_vuart_add(gc, domid, &vuart, state, &vuart_device);
> +libxl__device_console_dispose(&vuart);
> +}
> +

This is strange. You have code snippet for both PV and HVM. Why?

>  init_console_info(gc, &console, 0);
>  console.backend_domid = state->console_domid;
>  libxl__device_console_add(gc, domid, &console, state, &device);
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 5e96676..bb25672 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -26,6 +26,9 @@ static char *libxl__device_frontend_path(libxl__gc *gc, 
> libxl__device *device)
>  if (device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0)
>  return GCSPRINTF("%s/console", dom_path);
>  
> +if (device->kind == LIBXL__DEVICE_KIND_VUART)
> +return GCSPRINTF("%s/vuart/%d", dom_path, device->devid);
> +
>  return GCSPRINTF("%s/device/%s/%d", dom_path,
>   libxl__device_kind_to_string(device->kind),
>   device->devid);
> @@ -151,13 +154,19 @@ retry_transaction:
>  if (rc) goto out;
>  
>  if (!libxl_only) {
> -rc = libxl__xs_write_checked(gc, t, 
> GCSPRINTF("%s/frontend",libxl_path),
> - frontend_path);
> -if (rc) goto out;
> +if (fents || ro_fents)
> +{
> +rc = libxl__xs_write_checked(gc, t, 
> GCSPRINTF("%s/frontend",libxl_path),
> + frontend_path);
> +if (rc) goto out;
> +}
>  
> -rc = libxl__xs_write_checked(gc, t, 
> GCSPRINTF("%s/backend",libxl_path),
> - backend_path);
> -if (rc) goto out;
> +if (bents)
> +{
> +rc = libxl__xs_write_checked(gc, t, 
> GCSPRINTF("%s/backend",libxl_path),
> + backend_path);
> +if (rc) goto out;
> +}

What is this for?

If there is no fe or be entries you skip the path creation altogether.
But why? This doesn't seem to be related to your patch.

At least explain this a bit in the commit message?

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


Re: [Xen-devel] [PATCH 07/12 v3] xen/arm: vpl011: Allocate a new GFN in the toolstack for vuart

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:13PM +0530, Bhupinder Thakur wrote:
> Allocate a new gfn to be used as a ring buffer between xenconsole
> and Xen for sending/receiving pl011 data.
> 
> Signed-off-by: Bhupinder Thakur 

Acked-by: Wei Liu 

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


Re: [Xen-devel] [PATCH V4] x86/ioreq_server: Make p2m_finish_type_change actually work

2017-05-11 Thread Jan Beulich
>>> On 12.05.17 at 04:42,  wrote:
> Commit 6d774a951696 ("x86/ioreq server: synchronously reset outstanding
> p2m_ioreq_server entries when an ioreq server unmaps") introduced
> p2m_finish_type_change(), which was meant to synchronously finish a
> previously initiated type change over a gpfn range.  It did this by
> calling get_entry(), checking if it was the appropriate type, and then
> calling set_entry().
> 
> Unfortunately, a previous commit (1679e0df3df6 "x86/ioreq server:
> asynchronously reset outstanding p2m_ioreq_server entries") modified
> get_entry() to always return the new type after the type change, meaning
> that p2m_finish_type_change() never changed any entries.  Which means
> when an ioreq server was detached and then re-attached (as happens in
> XenGT on reboot) the re-attach failed.
> 
> Fix this by using the existing p2m-specific recalculation logic instead
> of doing a read-check-write loop.
> 
> Fix: 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
>   outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
> 
> Signed-off-by: Xiong Zhang 
> Signed-off-by: Yu Zhang 
> Reviewed-by: George Dunlap 

Reviewed-by: Jan Beulich 
albeit I'm not overly happy with ...

> +int p2m_finish_type_change(struct domain *d,
> +   gfn_t first_gfn, unsigned long max_nr)
>  {
>  struct p2m_domain *p2m = p2m_get_hostp2m(d);
> -p2m_type_t t;
>  unsigned long gfn = gfn_x(first_gfn);
>  unsigned long last_gfn = gfn + max_nr - 1;
> -
> -ASSERT(ot != nt);
> -ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
> +int rc = 0;
>  
>  p2m_lock(p2m);
>  
>  last_gfn = min(last_gfn, p2m->max_mapped_pfn);
>  while ( gfn <= last_gfn )
>  {
> -get_gfn_query_unlocked(d, gfn, &t);
> -
> -if ( t == ot )
> -p2m_change_type_one(d, gfn, t, nt);
> +rc = p2m->recalc(p2m, gfn);
> +/* 
> + * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return
> + * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
> + * gfn here.
> + */
> +if ( rc == -ENOENT)
> +rc = 0;
> +else if ( rc < 0 )
> +{
> +gdprintk(XENLOG_ERR, "p2m->recalc failed! Dom%d gfn=%lx\n",
> + d->domain_id, gfn);

... a message being logged here.

Also I'm sure it was pointed out before that if this is meant for
4.9 (which I assume it is) you should have Cc-ed Julien (now
added).

Jan


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


Re: [Xen-devel] [PATCH v2 2/2] libxl/devd: correctly manipulate the dguest list

2017-05-11 Thread Ian Jackson
Roger Pau Monne writes ("[PATCH v2 2/2] libxl/devd: correctly manipulate the 
dguest list"):
> Current code in backend_watch_callback has two issues when manipulating the
> dguest list:
...
>  skip:
>  libxl__nested_ao_free(nested_ao);
> +clean:
>  if (ddev)
>  free(ddev->dev);

This is starting to be quite goto-rich, and the memory ownership rules
become less clear.  Rather than try to analyse this in detail, I
wonder if it would be better to try to rework this so that it fits
CODING_STYLE better.

Wei, what do you think ?

>  free(ddev);
> -free(dguest);
> +if (dguest != NULL &&
> +dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks == 0) {

Perhaps this cleanup functionality could become a function if its own.
check_maybe_free_dguest or something ?

I haven't gone through the code in detail trying to convince myself
it's OK.  Even if there are to be no significant code changes, I would
like to see the memory ownership and lifetime rules here written down
(in comments in the code).  That way readers wouldn't have to
reverse-engineer them, and bugs will be clearer.

Ian.

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


Re: [Xen-devel] [PATCH 08/12 v3] xen/arm: vpl011: Modify xenconsole to support multiple consoles

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:14PM +0530, Bhupinder Thakur wrote:
> Xenconsole supports only PV console currently. This patch adds support
> for supporting multiple consoles.
> 
> This patch modifies different data structures and APIs used
> in xenconsole to support multiple consoles.
> 
> Change summary:
> 
> 1. Split the domain structure into a console structure and the
>domain structure, where each console structure represents one
>console.
> 
> 2. Modify different APIs such as buffer_append() etc. to take
>console structure as input and perform per console specific
>operations.
> 
> 3. Define a generic console_create_ring(), which sets up the
>ring buffer and event channel for each console.
> 
> 3. Modify domain_create_ring() to use console_create_ring().
> 
> 4. Modifications in handle_ring_read() to read ring buffer data
>from multiple consoles.
> 
> 5. Add log file support for multiple consoles.

I feel that it would be far easier to review if you spilt this patch
into 5. Can you do that?

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


Re: [Xen-devel] [PATCH 09/12 v3] xen/arm: vpl011: Add support for vuart in xenconsole

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:15PM +0530, Bhupinder Thakur wrote:
> Xenconsole supports only PV console currently. This patch adds support
> for vuart console, which allows emulated pl011 UART to be accessed
> as a console.
> 
> Signed-off-by: Bhupinder Thakur 
> ---
> 
> One review comment was to keep the vuart code under CONFIG_ARM64 && 
> CONFIG_ACPI flags.
> This code review could not be incorporated as I could not find out the 
> appropriate flags
> unders which this code can be kept. Are the CONFIG* flags exported to 
> xenconsole?

IIRC they are exported.

> 
>  tools/console/daemon/io.c | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 9bb14de..19a2f35 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -115,6 +115,7 @@ struct console_data {
>  };
>  
>  static int map_pvcon_ring_ref(struct console *, int );
> +static int map_vuartcon_ring_ref(struct console *, int );
>  
>  static struct console_data console_data[] = {
>  
> @@ -124,6 +125,12 @@ static struct console_data console_data[] = {
>   .mapfunc = map_pvcon_ring_ref,
>   .mandatory = true
>   },
> + {
> + .xsname = "/vuart/0",
> + .ttyname = "tty",
> + .mapfunc = map_vuartcon_ring_ref,
> + .mandatory = false
> + }
>  };
>  
>  #define MAX_CONSOLE (sizeof(console_data)/sizeof(struct console_data))
> @@ -751,6 +758,28 @@ out:
>   return err;
>  }
>  
> +static int map_vuartcon_ring_ref(struct console *con, int ring_ref)
> +{
> + int err = 0;
> + struct domain *dom = con->d;
> +
> + if (!con->interface) {
> + con->interface = xc_map_foreign_range(xc,
> + 
>   dom->domid,
> + 
>   XC_PAGE_SIZE,
> + 
>   PROT_READ|PROT_WRITE,
> + 
>   (unsigned long)ring_ref);

Indentation.

> + if (con->interface == NULL) {
> + err = EINVAL;
> + goto out;
> + }
> + con->ring_ref = ring_ref;
> + }
> +
> +out:
> + return err;
> +}
> +
>  static int console_create_ring(struct console *con)
>  {
>   int err, remote_port, ring_ref;
> -- 
> 2.7.4
> 

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


Re: [Xen-devel] [PATCH 10/12 v3] xen/arm: vpl011: Add a new vuart console type to xenconsole client

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:16PM +0530, Bhupinder Thakur wrote:
> Add a new console type VUART to connect to guest's emualated vuart
> console.
> 
> Signed-off-by: Bhupinder Thakur 
> ---
>  tools/console/client/main.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/console/client/main.c b/tools/console/client/main.c
> index 99f..6f4405f 100644
> --- a/tools/console/client/main.c
> +++ b/tools/console/client/main.c
> @@ -76,7 +76,7 @@ static void usage(const char *program) {
>  "\n"
>  "  -h, --help   display this help and exit\n"
>  "  -n, --num N  use console number N\n"
> -"  --type TYPE  console type. must be 'pv' or 'serial'\n"
> +"  --type TYPE  console type. must be 'pv', 'serial' or 
> 'vuart'\n"
>  "  --start-notify-fd N file descriptor used to notify parent\n"
>  , program);
>  }
> @@ -264,6 +264,7 @@ typedef enum {
> CONSOLE_INVAL,
> CONSOLE_PV,
> CONSOLE_SERIAL,
> +   CONSOLE_VUART,
>  } console_type;
>  
>  static struct termios stdin_old_attr;
> @@ -361,6 +362,8 @@ int main(int argc, char **argv)
>   type = CONSOLE_SERIAL;
>   else if (!strcmp(optarg, "pv"))
>   type = CONSOLE_PV;
> + else if (!strcmp(optarg, "vuart"))
> + type = CONSOLE_VUART;
>   else {
>   fprintf(stderr, "Invalid type argument\n");
>   fprintf(stderr, "Console types supported are: 
> serial, pv\n");
> @@ -436,6 +439,9 @@ int main(int argc, char **argv)
>   else
>   snprintf(path, strlen(dom_path) + 
> strlen("/device/console/%d/tty") + 5, "%s/device/console/%d/tty", dom_path, 
> num);
>   }
> + if (type == CONSOLE_VUART) {
> + snprintf(path, strlen(dom_path) + strlen("/vuart/0/tty") + 1, 
> +  "%s/vuart/0/tty", dom_path);

Indentation.

With this fixed:

Acked-by: Wei Liu 

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


Re: [Xen-devel] [PATCH 11/12 v3] xen/arm: vpl011: Add a pl011 uart DT node in the guest device tree

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:17PM +0530, Bhupinder Thakur wrote:
> The SBSA uart node format is as specified in
> Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt and given below:
> 
> ARM SBSA defined generic UART
> --
> This UART uses a subset of the PL011 registers and consequently lives
> in the PL011 driver. It's baudrate and other communication parameters
> cannot be adjusted at runtime, so it lacks a clock specifier here.
> 
> Required properties:
> - compatible: must be "arm,sbsa-uart"
> - reg: exactly one register range
> - interrupts: exactly one interrupt specifier
> - current-speed: the (fixed) baud rate set by the firmware
> 
> Signed-off-by: Bhupinder Thakur 

I will leave this to arm folks.

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


Re: [Xen-devel] [PATCH 04/12 v3] xen/arm: vpl011: Add support for vuart in libxl

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 07:58:01PM +0530, Bhupinder Thakur wrote:
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 856a304..504ca7c 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -916,6 +916,14 @@ void parse_config_data(const char *config_source,
>  if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>  b_info->max_vcpus = l;
>  
> +if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) {
> +if (libxl_vuart_type_from_string(buf, &b_info->vuart)) {
> +fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n",
> +buf);
> +exit (1);

Extraneous space.

With this fixed:

Acked-by: Wei Liu 

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


Re: [Xen-devel] [PATCH 12/12 v3] xen/arm: vpl011: Update documentation for vuart console support

2017-05-11 Thread Wei Liu
On Wed, May 10, 2017 at 08:05:18PM +0530, Bhupinder Thakur wrote:
> 1. Update documentation for a new vuart option added.
> 2. Update documentation about SPI irq reserved for vpl011.
> 
> Signed-off-by: Bhupinder Thakur 

Acked-by: Wei Liu 

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


[Xen-devel] [PATCH v2 05/18] x86/pv: clean up emulate_ops.c

2017-05-11 Thread Andrew Cooper
Please can you fold this following delta?
---
 xen/arch/x86/pv/emulate_ops.c | 101 +-
 1 file changed, 51 insertions(+), 50 deletions(-)

diff --git a/xen/arch/x86/pv/emulate_ops.c b/xen/arch/x86/pv/emulate_ops.c
index 97c8d14..9341dec 100644
--- a/xen/arch/x86/pv/emulate_ops.c
+++ b/xen/arch/x86/pv/emulate_ops.c
@@ -39,7 +39,7 @@
 
 #include 
 
-/* I/O emulation support. Helper routines for, and type of, the stack stub.*/
+/* I/O emulation support. Helper routines for, and type of, the stack stub. */
 void host_to_guest_gpr_switch(struct cpu_user_regs *);
 unsigned long guest_to_host_gpr_switch(unsigned long);
 
@@ -318,15 +318,14 @@ static io_emul_stub_t *io_emul_stub_setup(struct 
priv_op_ctxt *ctxt, u8 opcode,
 }
 
 /* Has the guest requested sufficient permission for this I/O access? */
-static int guest_io_okay(unsigned int port, unsigned int bytes,
- struct vcpu *v, struct cpu_user_regs *regs)
+static bool guest_io_okay(unsigned int port, unsigned int bytes,
+  struct vcpu *v, struct cpu_user_regs *regs)
 {
 /* If in user mode, switch to kernel mode just to read I/O bitmap. */
-int user_mode = !(v->arch.flags & TF_kernel_mode);
-#define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v)
+const bool user_mode = !(v->arch.flags & TF_kernel_mode);
 
 if ( iopl_ok(v, regs) )
-return 1;
+return true;
 
 if ( v->arch.pv_vcpu.iobmp_limit > (port + bytes) )
 {
@@ -336,9 +335,11 @@ static int guest_io_okay(unsigned int port, unsigned int 
bytes,
  * Grab permission bytes from guest space. Inaccessible bytes are
  * read as 0xff (no access allowed).
  */
-TOGGLE_MODE();
+if ( user_mode )
+toggle_guest_mode(v);
+
 switch ( __copy_from_guest_offset(x.bytes, v->arch.pv_vcpu.iobmp,
-  port>>3, 2) )
+  port >> 3, 2) )
 {
 default: x.bytes[0] = ~0;
 /* fallthrough */
@@ -346,14 +347,15 @@ static int guest_io_okay(unsigned int port, unsigned int 
bytes,
 /* fallthrough */
 case 0:  break;
 }
-TOGGLE_MODE();
 
-if ( (x.mask & (((1< port) )
-match |= 1 << i;
+match |= 1u << i;
 }
 
 return match;
@@ -401,11 +403,11 @@ static bool admin_io_okay(unsigned int port, unsigned int 
bytes,
  * We never permit direct access to that register.
  */
 if ( (port == 0xcf8) && (bytes == 4) )
-return 0;
+return false;
 
 /* We also never permit direct access to the RTC/CMOS registers. */
 if ( ((port & ~1) == RTC_PORT(0)) )
-return 0;
+return false;
 
 return ioports_access_permitted(d, port, port + bytes - 1);
 }
@@ -416,10 +418,10 @@ static bool pci_cfg_ok(struct domain *currd, unsigned int 
start,
 uint32_t machine_bdf;
 
 if ( !is_hardware_domain(currd) )
-return 0;
+return false;
 
 if ( !CF8_ENABLED(currd->arch.pci_cf8) )
-return 1;
+return true;
 
 machine_bdf = CF8_BDF(currd->arch.pci_cf8);
 if ( write )
@@ -427,7 +429,7 @@ static bool pci_cfg_ok(struct domain *currd, unsigned int 
start,
 const unsigned long *ro_map = pci_get_ro_map(0);
 
 if ( ro_map && test_bit(machine_bdf, ro_map) )
-return 0;
+return false;
 }
 start |= CF8_ADDR_LO(currd->arch.pci_cf8);
 /* AMD extended configuration space access? */
@@ -438,7 +440,7 @@ static bool pci_cfg_ok(struct domain *currd, unsigned int 
start,
 uint64_t msr_val;
 
 if ( rdmsr_safe(MSR_AMD64_NB_CFG, msr_val) )
-return 0;
+return false;
 if ( msr_val & (1ULL << AMD64_NB_CFG_CF8_EXT_ENABLE_BIT) )
 start |= CF8_ADDR_HI(currd->arch.pci_cf8);
 }
@@ -835,7 +837,7 @@ static int priv_op_write_cr(unsigned int reg, unsigned long 
val,
 if ( (val ^ read_cr0()) & ~X86_CR0_TS )
 {
 gdprintk(XENLOG_WARNING,
-"Attempt to change unmodifiable CR0 flags\n");
+ "Attempt to change unmodifiable CR0 flags\n");
 break;
 }
 do_fpu_taskswitch(!!(val & X86_CR0_TS));
@@ -948,11 +950,11 @@ static int priv_op_read_msr(unsigned int reg, uint64_t 
*val,
 *val = curr->arch.pv_vcpu.gs_base_user;
 return X86EMUL_OKAY;
 
-/*
- * In order to fully retain original behavior, defer calling
- * pv_soft_rdtsc() until after emulation. This may want/need to be
- * reconsidered.
- */
+/*
+ * In order to fully retain original behavior, defer calling
+ * pv_soft_rdtsc() until after emulation. This may want/need to be
+ * reconsidered.
+ */
 case MSR_IA32_TSC:
 poc->tsc |= TSC_BASE;
 goto normal;
@@ -1042,16 +1044,16 @@ static int priv_op_read_msr(unsi

Re: [Xen-devel] [PATCH v1 04/10] xen/arm: p2m: Update IOMMU mapping whenever possible if page table is not shared

2017-05-11 Thread Julien Grall

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

Update IOMMU mapping if the IOMMU doesn't share page table with the CPU.
The best place to do so on ARM is __p2m_set_entry(). Use mfn as an indicator
of the required action. If mfn is valid call iommu_map_pages(),
otherwise - iommu_unmap_pages().

Signed-off-by: Oleksandr Tyshchenko 
CC: Julien Grall 


Acked-by: Julien Grall 



---
   Changes in v1:
  - Update IOMMU mapping in __p2m_set_entry() instead of p2m_set_entry().
  - Pass order argument to IOMMU APIs instead of page_count.
---
 xen/arch/arm/p2m.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 34d5776..9ca491b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -984,7 +984,15 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
 p2m_free_entry(p2m, orig_pte, level);

 if ( need_iommu(p2m->domain) && (p2m_valid(orig_pte) || p2m_valid(*entry)) 
)
-rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL << page_order);
+{
+if ( iommu_use_hap_pt(p2m->domain) )
+rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL << 
page_order);
+else if ( !mfn_eq(smfn, INVALID_MFN) )
+rc = iommu_map_pages(p2m->domain, gfn_x(sgfn), mfn_x(smfn),
+ page_order, p2m_get_iommu_flags(t));
+else
+rc = iommu_unmap_pages(p2m->domain, gfn_x(sgfn), page_order);
+}
 else
 rc = 0;




Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v1 05/10] iommu/arm: Re-define iommu_use_hap_pt(d) as iommu_hap_pt_share

2017-05-11 Thread Julien Grall

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

Not every integrated into ARM SoCs IOMMU can share page tables
with the CPU and as result the iommu_use_hap_pt(d) is not always true.
Reuse x86's iommu_hap_pt_share flag to indicate whether the IOMMU
page table is shared or not.

Now all IOMMU drivers on ARM are able to change this flag
according to their possibilities like x86-variants do.
Therefore set iommu_hap_pt_share flag for SMMU because it always shares
page table with the CPU.

Signed-off-by: Oleksandr Tyshchenko 
---
 xen/drivers/passthrough/arm/smmu.c | 3 +++
 xen/include/asm-arm/iommu.h| 7 +--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 527a592..86ee12a 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2870,6 +2870,9 @@ static __init int arm_smmu_dt_init(struct dt_device_node 
*dev,

platform_features &= smmu->features;

+   /* Always share P2M table between the CPU and the SMMU */
+   iommu_hap_pt_share = true;
+


I would prefer to bail-out if someone try to unshare the page-table 
rather than overriding. This would help us to know if someone are try to 
do that.


So I would do:

if ( !iommu_hap_pt_share )
{
printk()
return -EINVAL;
}


return 0;
 }

diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
index 57d9b1e..10a6f23 100644
--- a/xen/include/asm-arm/iommu.h
+++ b/xen/include/asm-arm/iommu.h
@@ -20,8 +20,11 @@ struct arch_iommu
 void *priv;
 };

-/* Always share P2M Table between the CPU and the IOMMU */
-#define iommu_use_hap_pt(d) (1)
+/*
+ * The ARM domain always has a P2M table, but not every integrated into
+ * ARM SoCs IOMMU can use it as page table.


The first part: "ARM domain has a P2M table" is pretty obvious. I would 
instead say: "Not every ARM SoCs IOMMU use the same page-table format as 
the processor.".



+ */
+#define iommu_use_hap_pt(d) (iommu_hap_pt_share)

 const struct iommu_ops *iommu_get_ops(void);
 void __init iommu_set_ops(const struct iommu_ops *ops);



Cheers,

--
Julien Grall

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


Re: [Xen-devel] [ARM] Native application design and discussion (I hope)

2017-05-11 Thread Volodymyr Babchuk
Hi Julien,



On 11 May 2017 at 13:07, Julien Grall  wrote:
> Looking again at the documentation and chatting with other ARM folks. I was
> wrong on some part, sorry for the confusion.
Thank you for this investigation. One can't fit whole ARMv8 TRM in one's head :)

> It turns out that if you don't need to flush the TLBs when disabling the
> HCR_EL2.VM (this is what Linux does for KVM). So disabling stage-2 for EL0
> app would be ok.
Aha, these are good news.

> But you still need to allocate a VMID per EL0 app as TLBs will still depend
> on it even with stage-2 disabled.
I see. But I will need to allocate VMID in any case, right?

> Even if we keep stage-2 enabled, we would have to create dummly page tables
> of stage-1 because the memory attribute would impact performance and at
> least not allow the EL0 app to move (see D4.2.8 in ARM DDI
> 0487A.k_iss10775). In this case, 1:1 page tables with a block map (e.g 1GB)
> would be sufficient and rely on stage-2 page tables.
Yes, I did exactly this in my PoC.  I'm was curious on disabling
stage-2 because I don't want to mess with p2m context save/restore
functions. Good to know that it is possible.

> Lastly, can you remind me with platform you are using for testing?
It is Renesas Rcar Gen3. It is Big-Little platform, but currently we
use only four A57 cores.

-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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


Re: [Xen-devel] [PATCH v2 2/5] vcpu: track hvm vcpu number on the system

2017-05-11 Thread Wei Liu
On Thu, May 11, 2017 at 02:04:09PM +0800, Chao Gao wrote:
> This number is used to calculate how many hvm vcpu on a pcpu on average.
> 
> Signed-off-by: Chao Gao 
> ---
>  xen/common/domain.c | 8 
>  xen/include/xen/sched.h | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index b22aacc..d433d9e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -71,6 +71,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
>  
>  vcpu_info_t dummy_vcpu_info;
>  
> +/* how many hvm vcpu on this system? */
> +atomic_t num_hvm_vcpus;
> +

This is x86 specific and should go to x86/domain.c

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


Re: [Xen-devel] [PATCH v2 2/5] vcpu: track hvm vcpu number on the system

2017-05-11 Thread Wei Liu
On Thu, May 11, 2017 at 12:35:11PM +0100, Wei Liu wrote:
> On Thu, May 11, 2017 at 02:04:09PM +0800, Chao Gao wrote:
> > This number is used to calculate how many hvm vcpu on a pcpu on average.
> > 
> > Signed-off-by: Chao Gao 
> > ---
> >  xen/common/domain.c | 8 
> >  xen/include/xen/sched.h | 2 ++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index b22aacc..d433d9e 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -71,6 +71,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;
> >  
> >  vcpu_info_t dummy_vcpu_info;
> >  
> > +/* how many hvm vcpu on this system? */
> > +atomic_t num_hvm_vcpus;
> > +
> 
> This is x86 specific and should go to x86/domain.c

... as with all the code that manipulates it. I'm sure you can find the
appropriate places like arch_initialise/destroy_vcpu.

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


Re: [Xen-devel] [PATCH v1 07/10] iommu/arm: Add alloc_page_table platform callback

2017-05-11 Thread Julien Grall

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

The alloc_page_table callback is a mandatory thing
for the IOMMUs that don't share page table with the CPU on ARM.
The non-shared IOMMUs have to perform all required actions here
to be ready to handle IOMMU mapping updates right after completing it.

The arch_iommu_populate_page_table() seems an appropriate place
to call newly created callback.
Since we will only be here for the non-shared IOMMUs always
return error if the callback wasn't implemented.


Why do you need a specific callback and not doing it directly in 
iommu_domain_init?


My take here is in the unshare case, we may want to have multiple set of 
page tables (e.g one per device). So this should be left at the 
discretion of the IOMMU itself.


Am I missing something?



Signed-off-by: Oleksandr Tyshchenko 
CC: Jan Beulich 

---
   Changes in V1:
  - Wrap callback in #ifdef CONFIG_ARM.
---
 xen/drivers/passthrough/arm/iommu.c | 5 +++--
 xen/include/xen/iommu.h | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/arm/iommu.c 
b/xen/drivers/passthrough/arm/iommu.c
index 95b1abb..f132032 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -70,6 +70,7 @@ void arch_iommu_domain_destroy(struct domain *d)

 int arch_iommu_populate_page_table(struct domain *d)
 {
-/* The IOMMU shares the p2m with the CPU */
-return -ENOSYS;
+const struct iommu_ops *ops = iommu_get_ops();
+
+return ops->alloc_page_table ? ops->alloc_page_table(d) : -ENOSYS;
 }
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 3afbc3b..f5914db 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -175,6 +175,9 @@ struct iommu_ops {
   unsigned int flags);
 int __must_check (*unmap_pages)(struct domain *d, unsigned long gfn,
 unsigned int order);
+#ifdef CONFIG_ARM
+int (*alloc_page_table)(struct domain *d);
+#endif /* CONFIG_ARM */
 void (*free_page_table)(struct page_info *);
 #ifdef CONFIG_X86
 void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned 
int value);



--
Julien Grall

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


Re: [Xen-devel] [PATCH v2 2/2] libxl/devd: correctly manipulate the dguest list

2017-05-11 Thread Wei Liu
On Thu, May 11, 2017 at 12:13:00PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 2/2] libxl/devd: correctly manipulate the 
> dguest list"):
> > Current code in backend_watch_callback has two issues when manipulating the
> > dguest list:
> ...
> >  skip:
> >  libxl__nested_ao_free(nested_ao);
> > +clean:
> >  if (ddev)
> >  free(ddev->dev);
> 
> This is starting to be quite goto-rich, and the memory ownership rules
> become less clear.  Rather than try to analyse this in detail, I
> wonder if it would be better to try to rework this so that it fits
> CODING_STYLE better.
> 
> Wei, what do you think ?
> 

No objection from me, of course.

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


Re: [Xen-devel] [dpdk-dev] [PATCH] maintainers: claim responsability for xen

2017-05-11 Thread Tan, Jianfeng
Hi  Thomas and all,

Apologize for being an unqualified maintainer.

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Friday, May 5, 2017 6:04 AM
> To: Joao Martins; Konrad Rzeszutek Wilk; Tan, Jianfeng
> Cc: Konrad Rzeszutek Wilk; d...@dpdk.org; Xen-devel
> Subject: Re: [dpdk-dev] [PATCH] maintainers: claim responsability for xen
> 
> Ping
> 
> The Xen dom0 support in DPDK seems dead.
> 
> Reminder:
> Last time we talked about, it was because of a severe bug which is not
> fixed yet:
> http://dpdk.org/ml/archives/dev/2016-July/044207.html

For this bug, we removed the userspace memset(0) and suppose it has been done 
by kernel, however, xen0 uses __get_free_pages() kernel API to map hugepages 
and reseve memseg, I think it makes sense to zero the hugepage for xen0 in 
rte_dom0_mm kernel module (instead of some special code for xen0 in userspace) 
to keep aligned behavior.

> http://dpdk.org/ml/archives/dev/2016-July/044376.html

It does not make any sense to upstream a netfront PMD before we have a netback 
PMD, as the legacy netback driver would be the bottleneck. Anyone has plan on 
this? And a question mark keeps in my mind that is it a must to implement 
netback in dom0?

From another perspective, instead of using netfront/netback, we can also use 
virtio/vhost as the device model; however, xl tool in xen only supports 
vhost-kernel backend instead of vhost-user backend. So anyone has plan to 
enhance xl tool so that we can accelerate dom0 just using vswitch like OVS-DPDK?

A third solution is to use xenvirtio as the frontend, and vhost_xen as the 
backend. This solution is to use virtio ring on grant table mechanism of xen. 
Honestly, I don't even know if it still work now. And to make it more usable, 
better to upstream vhost_xen inside popular vswitch like OVS-DPDK.

> The request (9 months ago) was to give more time for feedbacks:
> http://dpdk.org/ml/archives/dev/2016-July/044847.html

Apologize again that I volunteer to maintain these files, but spend very few 
time on this.

Thanks,
Jianfeng

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


Re: [Xen-devel] [PATCH v1 09/10] xen/arm: Add use_iommu flag to xen_arch_domainconfig

2017-05-11 Thread Julien Grall

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

This flag is intended to let Xen know that the guest has devices
which will most likely be used for passthrough and as the result
the use of IOMMU is expected for this domain.
The primary aim of this knowledge is to help the IOMMUs that don't
share page tables with the CPU on ARM be ready before P2M code starts
updating IOMMU mapping.
So, if this flag is set the non-shared IOMMUs will populate
their page tables at the domain creation time and thereby will be able
to handle IOMMU mapping updates from *the very beginning*.

In order to retain the current behavior for x86 still call
iommu_domain_init() with use_iommu flag being forced to false.

Signed-off-by: Oleksandr Tyshchenko 
CC: Jan Beulich 
CC: Julien Grall 
CC: Ian Jackson 
CC: Wei Liu 

---
   Changes in V1:
  - Treat use_iommu flag as the ARM decision only. Don't use
common domain creation flag for it, use ARM config instead.
  - Clarify patch subject/description.
---
 tools/libxl/libxl_arm.c   | 10 ++
 xen/arch/arm/domain.c |  2 +-
 xen/include/public/arch-arm.h |  5 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index d842d88..9c4705e 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -78,6 +78,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
 return ERROR_FAIL;
 }

+/* TODO Are these assumptions enough to make decision about using IOMMU? */
+if ((d_config->num_dtdevs && d_config->dtdevs) ||
+(d_config->num_pcidevs && d_config->pcidevs))


Checking num_dtdevs and num_pcidevs is enough. It would be a bug if 
dtdevs and pcidevs are not null.



+xc_config->use_iommu = 1;
+else
+xc_config->use_iommu = 0;
+
+LOG(DEBUG, "The use of IOMMU %s expected for this domain",
+xc_config->use_iommu ? "is" : "isn't");
+
 return 0;
 }

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ec19310..81c4b90 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -569,7 +569,7 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,
 ASSERT(config != NULL);

 /* p2m_init relies on some value initialized by the IOMMU subsystem */
-if ( (rc = iommu_domain_init(d, false)) != 0 )
+if ( (rc = iommu_domain_init(d, config->use_iommu ? true : false)) != 0 )


!!config->use_iommu is enough.


 goto fail;

 if ( (rc = p2m_init(d)) != 0 )
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index bd974fb..cb33f75 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -322,6 +322,11 @@ struct xen_arch_domainconfig {
  *
  */
 uint32_t clock_frequency;
+/*
+ * IN
+ * Inform the hypervisor that the use of IOMMU is expected for this domain.


I would simplify to : "IOMMU is expected to be used for this domain".


+ */
+uint8_t use_iommu;
 };
 #endif /* __XEN__ || __XEN_TOOLS__ */




Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2 1/2] libxl/devd: fix a race with concurrent device addition/removal

2017-05-11 Thread Roger Pau Monne
On Thu, May 11, 2017 at 12:06:08PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 1/2] libxl/devd: fix a race with 
> concurrent device addition/removal"):
> > Current code can free the libxl__device inside of the libxl__ddomain_device
> > before the addition has finished if a removal happens while an addition is
> > still in process:
> ...
> > Fix this by creating a temporary copy of the libxl__device, that's
> > tracked by the GC of the nested async operation. This ensures that
> > the libxl__device used by the async operations cannot be freed while
> > being used.
> ...
> >  GCNEW(aodev);
> >  libxl__prepare_ao_device(ao, aodev);
> > -aodev->dev = dev;
> > +/*
> > + * Clone the libxl__device to avoid races if remove_device is 
> > called
> > + * before the device addition has finished.
> > + */
> > +GCNEW(aodev->dev);
> > +*aodev->dev = *dev;
> 
> This does conveniently disentangle the memory management, so I think
> it's a good approach.
> 
> But it reads kind of oddly to me.  I think it is not buggy, but can
> you add a comment to the definition of libxl__device, saying that it
> is a transparent structure containing no external memory references ?

Sure, before implementing this I already took a look at the contents of the
libxl__device struct, but I agree that a comment is in place in case someone
expands the fields of the struct later on.

> Otherwise this copy is not really justifiable, because in C, in
> general, structs might contain private fields, or memory references or
> linked list entries or something.

Thanks, Roger.

NB: FWIW, I'm planning to keep Wei's RB since this is a cosmetic change.


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


[Xen-devel] [PATCH v2] xen-netfront: avoid crashing on resume after a failure in talk_to_netback()

2017-05-11 Thread Vitaly Kuznetsov
Unavoidable crashes in netfront_resume() and netback_changed() after a
previous fail in talk_to_netback() (e.g. when we fail to read MAC from
xenstore) were discovered. The failure path in talk_to_netback() does
unregister/free for netdev but we don't reset drvdata and we try accessing
it after resume.

Fix the bug by removing the whole xen device completely with
device_unregister(), this guarantees we won't have any calls into netfront
after a failure.

Signed-off-by: Vitaly Kuznetsov 
---
Changes since v1: instead of cleaning drvdata and checking for it in
netfront_resume() and netback_changed() remove the device completely with
device_unregister() [David Miller]
---
 drivers/net/xen-netfront.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 6ffc482..7b61adb 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1934,8 +1934,7 @@ static int talk_to_netback(struct xenbus_device *dev,
xennet_disconnect_backend(info);
xennet_destroy_queues(info);
  out:
-   unregister_netdev(info->netdev);
-   xennet_free_netdev(info->netdev);
+   device_unregister(&dev->dev);
return err;
 }
 
-- 
2.9.3


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


Re: [Xen-devel] [PATCH v1 10/10] xen/arm: domain_build: Don't expose the "iommus" property to the guest

2017-05-11 Thread Julien Grall

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

We don't passthrough IOMMU device to DOM0 even if it is not used by
Xen. Therefore exposing the property that describes the IOMMU
master interfaces of the device does not make any sense.

Signed-off-by: Oleksandr Tyshchenko 
---
 xen/arch/arm/domain_build.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3abacc0..2defb60 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -432,6 +432,10 @@ static int write_properties(struct domain *d, struct 
kernel_info *kinfo,
 continue;
 }

+/* Don't expose the property "iommus" to the guest */
+if ( dt_property_name_is_equal(prop, "iommus") )
+continue;


It would be useful to have a link to the bindings associated 
(Documentation/devicetree/bindings/iommu/iommu.txt).


Also, whilst you are at it, you likely want to remove all the other 
iommu properties such as iommu-map.



+
 res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);

 if ( res )



Cheers,

--
Julien Grall

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


Re: [Xen-devel] [stable-4.10: PATCH] xen: revert commits 72a9b186292 and da72ff5bfcb0

2017-05-11 Thread Greg KH
On Mon, May 08, 2017 at 07:12:13AM +0200, Juergen Gross wrote:
> Revert commit 72a9b186292 ("xen: Remove event channel notification
> through Xen PCI platform device") as the original analysis was wrong
> that all the removed code isn't in use any more. As commit da72ff5bfcb0
> ("partially revert xen: Remove event channel notification through Xen
> PCI platform device") reverted already some parts of it revert this
> commit, too.
> 
> It is still necessary for old Xen versions (< 4.0) and for being able
> to run the Linux kernel as dom0 in a nested Xen environment.
> 
> This is a backport of upstream commit 84d582d236dc1f9085e741affc72e9ba061a67c2

Now applied, thanks.

greg k-h

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


Re: [Xen-devel] [stable-4.11: PATCH] xen: revert commits 72a9b186292 and da72ff5bfcb0

2017-05-11 Thread Greg KH
On Mon, May 08, 2017 at 08:12:47AM +0200, Juergen Gross wrote:
> Revert commit 72a9b186292 ("xen: Remove event channel notification
> through Xen PCI platform device") as the original analysis was wrong
> that all the removed code isn't in use any more. As commit da72ff5bfcb0
> ("partially revert xen: Remove event channel notification through Xen
> PCI platform device") reverted already some parts of it revert this
> commit, too.
> 
> It is still necessary for old Xen versions (< 4.0) and for being able
> to run the Linux kernel as dom0 in a nested Xen environment.
> 
> This is a backport of upstream commit 84d582d236dc1f9085e741affc72e9ba061a67c2

Now applied, thanks.

greg k-h

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


Re: [Xen-devel] [stable-4.9: PATCH] xen: revert commit 72a9b186292

2017-05-11 Thread Greg KH
On Mon, May 08, 2017 at 07:11:56AM +0200, Juergen Gross wrote:
> Revert commit 72a9b186292 ("xen: Remove event channel notification
> through Xen PCI platform device") as the original analysis was wrong
> that all the removed code isn't in use any more.
> 
> It is still necessary for old Xen versions (< 4.0) and for being able
> to run the Linux kernel as dom0 in a nested Xen environment.
> 
> This is a backport of upstream commit 84d582d236dc1f9085e741affc72e9ba061a67c2

Now applied, thanks.

greg k-h

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


Re: [Xen-devel] Please apply "partially revert "xen: Remove event channel..."

2017-05-11 Thread Juergen Gross
On 12/04/17 15:44, Ian Jackson wrote:
> Greg KH writes ("Re: Please apply "partially revert "xen: Remove event 
> channel...""):
>> On Wed, Apr 12, 2017 at 03:26:53PM +0200, Juergen Gross wrote:
>>> On 12/04/17 15:13, Greg KH wrote:
 Is this still true?  This long thread is totally confusing, is that what
 you really want to have happen?  Anything else?
> 
> Quite!
> 
>>> Please ignore this request. We will send another one to fully revert the
>>> original patch leading to the current situation.
> 
> Thanks for clarifying this.  (In future, perhaps kernel folks could
> take charge of requesting backports as appropriate, when the stable
> kernel branches break, so that I don't have to blunder about making
> inappropriate requests...)
> 
> Can someone please let me know when this is fixed ?  This is blocking
> switching osstest's default kernel to something more modern.

Greg just sent the mail that he has applied the patch to the
linux-4.9.y stable tree.


Juergen

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


Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu operations in libxc

2017-05-11 Thread Wei Liu
On Mon, Apr 17, 2017 at 08:01:56PM +0800, Lan Tianyu wrote:
> On 2017年04月17日 19:08, Wei Liu wrote:
> > On Fri, Apr 14, 2017 at 11:38:15PM +0800, Lan, Tianyu wrote:
> >> Hi Paul:
> >>Sorry for later response.
> >>
> >> On 3/31/2017 3:57 AM, Chao Gao wrote:
> >>> On Wed, Mar 29, 2017 at 09:08:06AM +, Paul Durrant wrote:
> > -Original Message-
> > From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> > Chao Gao
> > Sent: 29 March 2017 01:40
> > To: Wei Liu 
> > Cc: Lan Tianyu ; Kevin Tian 
> > ;
> > Ian Jackson ; xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu
> > operations in libxc
> >
> > Tianyu is on vacation this two weeks, so I will try to address
> > some comments on this series.
> >
> > On Tue, Mar 28, 2017 at 05:24:03PM +0100, Wei Liu wrote:
> >> On Fri, Mar 17, 2017 at 07:27:05PM +0800, Lan Tianyu wrote:
> >>> From: Chao Gao 
> >>>
> >>> In previous patch, we introduce a common vIOMMU layer. In our design,
> >>> we create/destroy vIOMMU through DMOP interface instead of creating
> > it
> >>> according to a config flag of domain. It makes it is possible
> >>> to create vIOMMU in device model or in tool stack.
> >>>
> 
>  I've not been following this closely so apologies if this has already 
>  been asked...
> 
>  Why would you need to create a vIOMMU instance in an external device 
>  model.
>  Since the toolstack should be in control of the device model 
>  configuration why would it not know in advance that one was required?
> >>>
> >>> I assume your question is why we don't create a vIOMMU instance via 
> >>> hypercall in toolstack.
> >>> I think creating in toolstack is also ok and is easier to be reused by 
> >>> pvh.
> >>>
> >>> If Tianyu has no concern about this, will move this part to toolstack.
> >>
> >> We can move create/destroy vIOMMU in the tool stack but we still need to 
> >> add
> >> such dummy vIOMMU device model in Qemu to pass virtual device's DMA request
> >> into Xen hypervisor. Qemu is required to use DMOP hypercall and tool stack
> >> may use domctl hyercall. vIOMMU hypercalls will be divided into two part.
> >>
> >> Domctl:
> >>create, destroy and query.
> >> DMOP:
> >>vDev's DMA related operations.
> >>
> >> Is this OK?
> >>
> > 
> > Why are they divided into two libraries? Can't they be in DMOP at the
> > same time?
> 
> Yes, we can use DMOP for all vIOMMU hyercalls if it's necessary to keep
> unified vIOMMU hyercall type. In theory, DMOP dedicates to be used by
> Qemu but we also can use it in tool stack. If we move create, destroy
> and query operation to tool stack, it isn't necessary to use DMOP for
> them since only tool stack will call them. This is why I said we could
> use domctl for these operations. Both two ways will not affect function
> implementation. Which one it's better from your view? :)
> 


After reading the subthread I think I agree with Paul. I.e. please
separate them.

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


Re: [Xen-devel] [PATCH v11 20/23] tools: L2 CAT: support get HW info for L2 CAT.

2017-05-11 Thread Wei Liu
On Wed, May 03, 2017 at 04:44:20PM +0800, Yi Sun wrote:
> This patch implements xl/xc changes to support get HW info
> for L2 CAT.
> 
> 'xl psr-hwinfo' is updated to show both L3 CAT and L2 CAT
> info.
> 
> Example(on machine which only supports L2 CAT):
> Cache Monitoring Technology (CMT):
> Enabled : 0
> Cache Allocation Technology (CAT): L2
> Socket ID   : 0
> Maximum COS : 3
> CBM length  : 8
> Default CBM : 0xff
> 
> Signed-off-by: He Chen 
> Signed-off-by: Yi Sun 
> Acked-by: Jan Beulich 

Acked-by: Wei Liu 

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


Re: [Xen-devel] [PATCH v11 21/23] tools: L2 CAT: support show cbm for L2 CAT.

2017-05-11 Thread Wei Liu
On Wed, May 03, 2017 at 04:44:21PM +0800, Yi Sun wrote:
> This patch implements changes in xl/xc changes to support
> showing CBM of L2 CAT.
> 
> The new level option is introduced to original CAT showing
> command in order to show CBM for specified level CAT.
> - 'xl psr-cat-show' is updated to show CBM of a domain
>   according to input cache level.
> 
> Examples:
> root@:~$ xl psr-cat-show -l2 1
> Socket ID   : 0
> Default CBM : 0xff
>ID NAME CBM
> 1 ubuntu140x7f
> 
> Signed-off-by: He Chen 
> Signed-off-by: Yi Sun 
> ---

Acked-by: Wei Liu 

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


Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu operations in libxc

2017-05-11 Thread Lan Tianyu
On 2017年05月11日 20:35, Wei Liu wrote:
> On Mon, Apr 17, 2017 at 08:01:56PM +0800, Lan Tianyu wrote:
>> On 2017年04月17日 19:08, Wei Liu wrote:
>>> On Fri, Apr 14, 2017 at 11:38:15PM +0800, Lan, Tianyu wrote:
 Hi Paul:
Sorry for later response.

 On 3/31/2017 3:57 AM, Chao Gao wrote:
> On Wed, Mar 29, 2017 at 09:08:06AM +, Paul Durrant wrote:
>>> -Original Message-
>>> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
>>> Chao Gao
>>> Sent: 29 March 2017 01:40
>>> To: Wei Liu 
>>> Cc: Lan Tianyu ; Kevin Tian 
>>> ;
>>> Ian Jackson ; xen-devel@lists.xen.org
>>> Subject: Re: [Xen-devel] [RFC PATCH 5/23] Tools/libxc: Add viommu
>>> operations in libxc
>>>
>>> Tianyu is on vacation this two weeks, so I will try to address
>>> some comments on this series.
>>>
>>> On Tue, Mar 28, 2017 at 05:24:03PM +0100, Wei Liu wrote:
 On Fri, Mar 17, 2017 at 07:27:05PM +0800, Lan Tianyu wrote:
> From: Chao Gao 
>
> In previous patch, we introduce a common vIOMMU layer. In our design,
> we create/destroy vIOMMU through DMOP interface instead of creating
>>> it
> according to a config flag of domain. It makes it is possible
> to create vIOMMU in device model or in tool stack.
>
>>
>> I've not been following this closely so apologies if this has already 
>> been asked...
>>
>> Why would you need to create a vIOMMU instance in an external device 
>> model.
>> Since the toolstack should be in control of the device model 
>> configuration why would it not know in advance that one was required?
>
> I assume your question is why we don't create a vIOMMU instance via 
> hypercall in toolstack.
> I think creating in toolstack is also ok and is easier to be reused by 
> pvh.
>
> If Tianyu has no concern about this, will move this part to toolstack.

 We can move create/destroy vIOMMU in the tool stack but we still need to 
 add
 such dummy vIOMMU device model in Qemu to pass virtual device's DMA request
 into Xen hypervisor. Qemu is required to use DMOP hypercall and tool stack
 may use domctl hyercall. vIOMMU hypercalls will be divided into two part.

 Domctl:
create, destroy and query.
 DMOP:
vDev's DMA related operations.

 Is this OK?

>>>
>>> Why are they divided into two libraries? Can't they be in DMOP at the
>>> same time?
>>
>> Yes, we can use DMOP for all vIOMMU hyercalls if it's necessary to keep
>> unified vIOMMU hyercall type. In theory, DMOP dedicates to be used by
>> Qemu but we also can use it in tool stack. If we move create, destroy
>> and query operation to tool stack, it isn't necessary to use DMOP for
>> them since only tool stack will call them. This is why I said we could
>> use domctl for these operations. Both two ways will not affect function
>> implementation. Which one it's better from your view? :)
>>
> 
> 
> After reading the subthread I think I agree with Paul. I.e. please
> separate them.
> 

Sure. Will update.

-- 
Best regards
Tianyu Lan

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


[Xen-devel] [ovmf baseline-only test] 71288: tolerable FAIL

2017-05-11 Thread Platform Team regression test user
This run is configured for baseline tests only.

flight 71288 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71288/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-libvirt   5 libvirt-buildfail   like 71287
 build-i386-libvirt5 libvirt-buildfail   like 71287

version targeted for testing:
 ovmf a6c31c6d6349a51041d8b77df375c340043e36bd
baseline version:
 ovmf ef810bc807188224a752ffbcf5e7f4b651291cee

Last test of basis71287  2017-05-11 07:48:51 Z0 days
Testing same since71288  2017-05-11 10:48:55 Z0 days1 attempts


People who touched revisions under test:
  Star Zeng 
  Yonghong Zhu 

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



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


commit a6c31c6d6349a51041d8b77df375c340043e36bd
Author: Yonghong Zhu 
Date:   Wed May 10 16:35:30 2017 +0800

BaseTools: Correct VOID* PatchPcd Size in Library Autogen

This patch correct the VOID* PatchPcd Size info generated in the
Library's autogen file. Update it to use the MaxDatumSize.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
Reviewed-by: Liming Gao 

commit 8ecb1e9befbf9a049f74486274288c1bc677ebf3
Author: Star Zeng 
Date:   Thu May 11 12:57:47 2017 +0800

MdeModulePkg CapsuleApp: Fix mixed EOL format issue

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 

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


Re: [Xen-devel] [PATCH v11 22/23] tools: L2 CAT: support set cbm for L2 CAT.

2017-05-11 Thread Wei Liu
On Wed, May 03, 2017 at 04:44:22PM +0800, Yi Sun wrote:
> This patch implements the xl/xc changes to support set CBM
> for L2 CAT.
> 
> The new level option is introduced to original CAT setting
> command in order to set CBM for specified level CAT.
> - 'xl psr-cat-set' is updated to set cache capacity bitmasks(CBM)
>   for a domain according to input cache level.
> 
> root@:~$ xl psr-cat-set -l2 1 0x7f
> 
> root@:~$ xl psr-cat-show -l2 1
> Socket ID   : 0
> Default CBM : 0xff
>ID NAME CBM
> 1 ubuntu140x7f
> 
> Signed-off-by: He Chen 
> Signed-off-by: Yi Sun 

Acked-by: Wei Liu 

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


Re: [Xen-devel] [PATCH v3] x86/amd: don't set X86_BUG_SYSRET_SS_ATTRS when running under Xen

2017-05-11 Thread Thomas Gleixner
On Thu, 11 May 2017, Juergen Gross wrote:

> On 27/04/17 07:01, Juergen Gross wrote:
> > When running as Xen pv guest X86_BUG_SYSRET_SS_ATTRS must not be set
> > on AMD cpus.
> > 
> > This bug/feature bit is kind of special as it will be used very early
> > when switching threads. Setting the bit and clearing it a little bit
> > later leaves a critical window where things can go wrong. This time
> > window has enlarged a little bit by using setup_clear_cpu_cap() instead
> > of the hypervisor's set_cpu_features callback. It seems this larger
> > window now makes it rather easy to hit the problem.
> > 
> > The proper solution is to never set the bit in case of Xen.
> > 
> > Signed-off-by: Juergen Gross 
> 
> Any objections for carrying this through the Xen tree?


Acked-by: Thomas Gleixner 

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


[Xen-devel] [PATCH for-4.9] xl: don't ignore return value from libxl_device_events_handler

2017-05-11 Thread Wei Liu
That function can return a whole slew of error codes. Translate them
to EXIT_FAILURE.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Julien Grall 
Cc: Roger Pau Monn??  
---
 tools/xl/xl_misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 9037e2b2f0..9c6227af23 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -182,7 +182,7 @@ int main_devd(int argc, char **argv)
 }
 }
 
-libxl_device_events_handler(ctx, 0);
+ret = libxl_device_events_handler(ctx, 0) ? EXIT_FAILURE : EXIT_SUCCESS;
 
 out:
 return ret;
-- 
2.11.0


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


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

2017-05-11 Thread osstest service owner
flight 109292 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109292/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-pvh-intel  6 xen-boot fail REGR. vs. 59254
 test-amd64-i386-pair  9 xen-boot/src_host fail REGR. vs. 59254
 test-amd64-amd64-pair 9 xen-boot/src_host fail REGR. vs. 59254
 test-amd64-i386-pair 10 xen-boot/dst_host fail REGR. vs. 59254
 test-amd64-amd64-pair10 xen-boot/dst_host fail REGR. vs. 59254
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-libvirt   6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-credit2   6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail REGR. 
vs. 59254
 test-amd64-i386-xl-qemut-win7-amd64  6 xen-boot   fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-winxpsp3  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail REGR. vs. 59254
 test-amd64-amd64-libvirt-xsm  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-debianhvm-amd64  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-bootfail REGR. vs. 59254
 test-amd64-amd64-xl-xsm   6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
59254
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-ovmf-amd64  6 xen-boot   fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-debianhvm-amd64  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-libvirt-xsm   6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-winxpsp3  6 xen-boot fail REGR. vs. 59254
 test-amd64-i386-xl6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-multivcpu  6 xen-boot fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-win7-amd64  6 xen-boot   fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-qemuu-rhel6hvm-amd  6 xen-bootfail REGR. vs. 59254
 test-amd64-amd64-xl-pvh-amd   6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl   6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-freebsd10-i386  6 xen-bootfail REGR. vs. 59254
 test-amd64-amd64-xl-qemuu-win7-amd64  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-xl-xsm6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-libvirt  6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-qemut-rhel6hvm-amd  6 xen-bootfail REGR. vs. 59254
 test-amd64-i386-freebsd10-amd64  6 xen-boot   fail REGR. vs. 59254

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  6 xen-boot  fail REGR. vs. 59254

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qcow2 6 xen-bootfail baseline untested
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot fail baseline untested
 test-amd64-amd64-i386-pvgrub  6 xen-bootfail baseline untested
 test-amd64-amd64-libvirt-pair  9 xen-boot/src_host  fail baseline untested
 test-amd64-amd64-libvirt-pair 10 xen-boot/dst_host  fail baseline untested
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline 
untested
 test-amd64-amd64-pygrub   6 xen-bootfail baseline untested
 test-amd64-amd64-libvirt-vhd  6 xen-bootfail baseline untested
 test-amd64-i386-rumprun-i386  6 xen-bootfail baseline untested
 test-amd64-i386-xl-raw6 xen-bootfail baseline untested
 test-amd64-amd64-amd64-pvgrub  6 xen-boot   fail baseline untested
 test-amd64-amd64-qemuu-nested-amd  6 xen-boot   fail baseline untested
 test-amd64-i386-libvirt-pair  9 xen-boot/src_host   fail baseline untested
 test-amd64-i386-libvirt-p

Re: [Xen-devel] [PATCH] public/elfnote: document non-alignment of relocated init-P2M

2017-05-11 Thread Andrew Cooper
On 09/05/17 17:02, Jan Beulich wrote:
 On 09.05.17 at 17:33,  wrote:
>> On 09/05/17 17:32, Jan Beulich wrote:
>> On 09.05.17 at 17:25,  wrote:
 On 09/05/17 17:06, Jan Beulich wrote:
 On 09.05.17 at 16:36,  wrote:
>> On 09/05/17 16:23, Jan Beulich wrote:
>>> Since PV kernels can't use large pages anywa, when the init-P2M support
>>> was added it was decided to keep the implementation simple and not
>>> align large pages in PFN space. Document this.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/include/public/elfnote.h
>>> +++ b/xen/include/public/elfnote.h
>>> @@ -173,7 +173,9 @@
>>>   * The (non-default) location the initial phys-to-machine map should be
>>>   * placed at by the hypervisor (Dom0) or the tools (DomU).
>>>   * The kernel must be prepared for this mapping to be established using
>>> - * large pages, despite such otherwise not being available to guests.
>>> + * large pages, despite such otherwise not being available to guests. 
>>> Note
>> Shouldn't the large page usage be limited to dom0?
> Why? Even if the tools right now don't use large pages here, why
> should we preclude them wanting to at some point?
 Those could be of temporary nature only in order not to break migration.
 So the guest would be forced to split the big pages up anyway. Why
 create them in the beginning then?
>>> As long as the migration stream doesn't represent them as 1Gb or
>>> 2Mb pages, I don't see how they would get in the way of migration.
>> How would the receiving side allocate the additional page tables?
> Oh, indeed. Yet still, guests not intended to be migrated could be
> built that way. I don't think we should allow kernel side relaxation
> here.

The save side of PV migrate will cleanly abort if it finds any
superpages, precisely because we have no idea whether the destination
can allocate the frames.

~Andrew

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


Re: [Xen-devel] [PATCH] public/elfnote: document non-alignment of relocated init-P2M

2017-05-11 Thread Andrew Cooper
On 09/05/17 16:23, Jan Beulich wrote:
 On 09.05.17 at 17:10,  wrote:
>> On 09/05/17 15:23, Jan Beulich wrote:
>>> Since PV kernels can't use large pages anywa, when the init-P2M support
>> anyway
> Already fixed after Alan pointed this out. Do I need to send v2
> because of this typo?

No.  Acked-by: Andrew Cooper 

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


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

2017-05-11 Thread osstest service owner
flight 109312 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109312/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 29dc8aa861fac78c6d62391dff312db934b755e3
baseline version:
 ovmf a6c31c6d6349a51041d8b77df375c340043e36bd

Last test of basis   109308  2017-05-11 07:57:17 Z0 days
Testing same since   109312  2017-05-11 10:45:18 Z0 days1 attempts


People who touched revisions under test:
  Chao Zhang 
  Jeff Fan 
  Zhang, Chao B 

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



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

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

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

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


Pushing revision :

+ branch=ovmf
+ revision=29dc8aa861fac78c6d62391dff312db934b755e3
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
29dc8aa861fac78c6d62391dff312db934b755e3
+ branch=ovmf
+ revision=29dc8aa861fac78c6d62391dff312db934b755e3
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' x29dc8aa861fac78c6d62391dff312db934b755e3 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.or

Re: [Xen-devel] [PATCH v1 07/10] iommu/arm: Add alloc_page_table platform callback

2017-05-11 Thread Oleksandr Tyshchenko
On Thu, May 11, 2017 at 2:38 PM, Julien Grall  wrote:
> Hi Oleksandr,
Hi, Julien

>
> On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> The alloc_page_table callback is a mandatory thing
>> for the IOMMUs that don't share page table with the CPU on ARM.
>> The non-shared IOMMUs have to perform all required actions here
>> to be ready to handle IOMMU mapping updates right after completing it.
>>
>> The arch_iommu_populate_page_table() seems an appropriate place
>> to call newly created callback.
>> Since we will only be here for the non-shared IOMMUs always
>> return error if the callback wasn't implemented.
>
>
> Why do you need a specific callback and not doing it directly in
> iommu_domain_init?
>
> My take here is in the unshare case, we may want to have multiple set of
> page tables (e.g one per device). So this should be left at the discretion
> of the IOMMU itself.
>
> Am I missing something?
I was thinking about extra need_iommu argument for init platform
callback as I had done for iommu_domain_init API.
But I had doubts regarding hw_domain. During iommu_domain_init
execution we haven't known yet is the IOMMU expected for domain 0
or not.

Taking into account that I needed to:
- populate page table followed by setting need_iommu flag.
- implement arch_iommu_populate_page_table() on ARM because of
!iommu_use_hap_pt(d).
- find a solution for hw_domain.

I decided to use iommu_construct() and implement alloc_page_table
callback to be called for populating page table.
I thought that it would allow us to keep all required actions in a
single place rather than spreading.

>
>
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> CC: Jan Beulich 
>>
>> ---
>>Changes in V1:
>>   - Wrap callback in #ifdef CONFIG_ARM.
>> ---
>>  xen/drivers/passthrough/arm/iommu.c | 5 +++--
>>  xen/include/xen/iommu.h | 3 +++
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/arm/iommu.c
>> b/xen/drivers/passthrough/arm/iommu.c
>> index 95b1abb..f132032 100644
>> --- a/xen/drivers/passthrough/arm/iommu.c
>> +++ b/xen/drivers/passthrough/arm/iommu.c
>> @@ -70,6 +70,7 @@ void arch_iommu_domain_destroy(struct domain *d)
>>
>>  int arch_iommu_populate_page_table(struct domain *d)
>>  {
>> -/* The IOMMU shares the p2m with the CPU */
>> -return -ENOSYS;
>> +const struct iommu_ops *ops = iommu_get_ops();
>> +
>> +return ops->alloc_page_table ? ops->alloc_page_table(d) : -ENOSYS;
>>  }
>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>> index 3afbc3b..f5914db 100644
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -175,6 +175,9 @@ struct iommu_ops {
>>unsigned int flags);
>>  int __must_check (*unmap_pages)(struct domain *d, unsigned long gfn,
>>  unsigned int order);
>> +#ifdef CONFIG_ARM
>> +int (*alloc_page_table)(struct domain *d);
>> +#endif /* CONFIG_ARM */
>>  void (*free_page_table)(struct page_info *);
>>  #ifdef CONFIG_X86
>>  void (*update_ire_from_apic)(unsigned int apic, unsigned int reg,
>> unsigned int value);
>>
>
> --
> Julien Grall



-- 
Regards,

Oleksandr Tyshchenko

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


[Xen-devel] [linux-4.9 test] 109296: regressions - FAIL

2017-05-11 Thread osstest service owner
flight 109296 linux-4.9 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109296/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit2   6 xen-boot fail REGR. vs. 107358

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail in 109240 pass in 
109260
 test-amd64-amd64-xl-qemuu-winxpsp3 16 guest-stop fail in 109240 pass in 109296
 test-amd64-i386-xl-qemuu-winxpsp3 16 guest-stop  fail in 109240 pass in 109296
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail in 109260 
pass in 109296
 test-armhf-armhf-libvirt-xsm  5 xen-install  fail in 109260 pass in 109296
 test-amd64-amd64-xl-qemuu-winxpsp3 17 guest-start/win.repeat fail in 109260 
pass in 109296
 test-amd64-i386-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail pass in 
109240
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail pass in 
109260
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail pass in 
109260

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stopfail REGR. vs. 107358
 test-amd64-amd64-xl-rtds  9 debian-install   fail REGR. vs. 107358

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop  fail in 109240 like 107358
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stopfail in 109240 never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-start/win.repeat fail in 109260 
like 107358
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail like 107358
 test-armhf-armhf-libvirt-raw  6 xen-boot fail  like 107358
 test-armhf-armhf-xl-rtds  6 xen-boot fail  like 107358
 test-armhf-armhf-xl-xsm   6 xen-boot fail  like 107358
 test-armhf-armhf-libvirt-xsm  6 xen-boot fail  like 107358
 test-armhf-armhf-libvirt  6 xen-boot fail  like 107358
 test-armhf-armhf-xl-vhd   6 xen-boot fail  like 107358
 test-armhf-armhf-xl   6 xen-boot fail  like 107358
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-rtds 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-rtds 13 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 12 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1   fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-arndale   6 xen-boot fail   never pass
 test-arm64-arm64-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-arm64-arm64-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-arm64-arm64-xl  12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass

version targeted for testing:
 linux89f3b8d5f264d5dab9818c6667c71e3cc55b13f5
baseline version:
 linux37feaf8095d352014555b82adb4a04609ca17d3f

Last test of basis   107358  2017-04-10 19:42:52 Z   30 days
Failing since107396  2017-04-12 11:15:19 Z   29 days   49 attempts
Testing same since   109171  2017-05-08 09:58:59 Z3 days6 attempts


334 people touched re

Re: [Xen-devel] [PATCH v1 09/10] xen/arm: Add use_iommu flag to xen_arch_domainconfig

2017-05-11 Thread Oleksandr Tyshchenko
On Thu, May 11, 2017 at 2:42 PM, Julien Grall  wrote:
> Hi Oleksandr,
Hi Julien

>
>
> On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> This flag is intended to let Xen know that the guest has devices
>> which will most likely be used for passthrough and as the result
>> the use of IOMMU is expected for this domain.
>> The primary aim of this knowledge is to help the IOMMUs that don't
>> share page tables with the CPU on ARM be ready before P2M code starts
>> updating IOMMU mapping.
>> So, if this flag is set the non-shared IOMMUs will populate
>> their page tables at the domain creation time and thereby will be able
>> to handle IOMMU mapping updates from *the very beginning*.
>>
>> In order to retain the current behavior for x86 still call
>> iommu_domain_init() with use_iommu flag being forced to false.
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> CC: Jan Beulich 
>> CC: Julien Grall 
>> CC: Ian Jackson 
>> CC: Wei Liu 
>>
>> ---
>>Changes in V1:
>>   - Treat use_iommu flag as the ARM decision only. Don't use
>> common domain creation flag for it, use ARM config instead.
>>   - Clarify patch subject/description.
>> ---
>>  tools/libxl/libxl_arm.c   | 10 ++
>>  xen/arch/arm/domain.c |  2 +-
>>  xen/include/public/arch-arm.h |  5 +
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index d842d88..9c4705e 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -78,6 +78,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>>  return ERROR_FAIL;
>>  }
>>
>> +/* TODO Are these assumptions enough to make decision about using
>> IOMMU? */
>> +if ((d_config->num_dtdevs && d_config->dtdevs) ||
>> +(d_config->num_pcidevs && d_config->pcidevs))
>
>
> Checking num_dtdevs and num_pcidevs is enough. It would be a bug if dtdevs
> and pcidevs are not null.
ok

>
>> +xc_config->use_iommu = 1;
>> +else
>> +xc_config->use_iommu = 0;
>> +
>> +LOG(DEBUG, "The use of IOMMU %s expected for this domain",
>> +xc_config->use_iommu ? "is" : "isn't");
>> +
>>  return 0;
>>  }
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index ec19310..81c4b90 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -569,7 +569,7 @@ int arch_domain_create(struct domain *d, unsigned int
>> domcr_flags,
>>  ASSERT(config != NULL);
>>
>>  /* p2m_init relies on some value initialized by the IOMMU subsystem
>> */
>> -if ( (rc = iommu_domain_init(d, false)) != 0 )
>> +if ( (rc = iommu_domain_init(d, config->use_iommu ? true : false)) !=
>> 0 )
>
>
> !!config->use_iommu is enough.
ok

>
>>  goto fail;
>>
>>  if ( (rc = p2m_init(d)) != 0 )
>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>> index bd974fb..cb33f75 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -322,6 +322,11 @@ struct xen_arch_domainconfig {
>>   *
>>   */
>>  uint32_t clock_frequency;
>> +/*
>> + * IN
>> + * Inform the hypervisor that the use of IOMMU is expected for this
>> domain.
>
>
> I would simplify to : "IOMMU is expected to be used for this domain".
ok

>
>> + */
>> +uint8_t use_iommu;
>>  };
>>  #endif /* __XEN__ || __XEN_TOOLS__ */
>>
>>
>
> Cheers,
>
> --
> Julien Grall



-- 
Regards,

Oleksandr Tyshchenko

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


[Xen-devel] [PATCH 4.11 27/28] xen: Revert commits da72ff5bfcb0 and 72a9b186292d

2017-05-11 Thread Greg Kroah-Hartman
4.11-stable review patch.  If anyone has any objections, please let me know.

--

From: Boris Ostrovsky 

commit 84d582d236dc1f9085e741affc72e9ba061a67c2 upstream.

Recent discussion (http://marc.info/?l=xen-devel&m=149192184523741)
established that commit 72a9b186292d ("xen: Remove event channel
notification through Xen PCI platform device") (and thus commit
da72ff5bfcb0 ("partially revert "xen: Remove event channel
notification through Xen PCI platform device"")) are unnecessary and,
in fact, prevent HVM guests from booting on Xen releases prior to 4.0

Therefore we revert both of those commits.

The summary of that discussion is below:

  Here is the brief summary of the current situation:

  Before the offending commit (72a9b186292):

  1) INTx does not work because of the reset_watches path.
  2) The reset_watches path is only taken if you have Xen > 4.0
  3) The Linux Kernel by default will use vector inject if the hypervisor
 support. So even INTx does not work no body running the kernel with
 Xen > 4.0 would notice. Unless he explicitly disabled this feature
 either in the kernel or in Xen (and this can only be disabled by
 modifying the code, not user-supported way to do it).

  After the offending commit (+ partial revert):

  1) INTx is no longer support for HVM (only for PV guests).
  2) Any HVM guest The kernel will not boot on Xen < 4.0 which does
 not have vector injection support. Since the only other mode
 supported is INTx which.

  So based on this summary, I think before commit (72a9b186292) we were
  in much better position from a user point of view.

Signed-off-by: Boris Ostrovsky 
Reviewed-by: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk 
Cc: Bjorn Helgaas 
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Vitaly Kuznetsov 
Cc: Paul Gortmaker 
Cc: Ross Lagerwall 
Cc: xen-de...@lists.xenproject.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: Anthony Liguori 
Cc: KarimAllah Ahmed 
Signed-off-by: Juergen Gross 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/include/asm/xen/events.h |   11 +++
 arch/x86/pci/xen.c|2 +-
 arch/x86/xen/enlighten.c  |   16 +++-
 arch/x86/xen/smp.c|2 ++
 arch/x86/xen/time.c   |5 +
 drivers/xen/events/events_base.c  |   26 +-
 drivers/xen/platform-pci.c|   13 +++--
 7 files changed, 50 insertions(+), 25 deletions(-)

--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -20,4 +20,15 @@ static inline int xen_irqs_disabled(stru
 /* No need for a barrier -- XCHG is a barrier on x86. */
 #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
 
+extern int xen_have_vector_callback;
+
+/*
+ * Events delivered via platform PCI interrupts are always
+ * routed to vcpu 0 and hence cannot be rebound.
+ */
+static inline bool xen_support_evtchn_rebind(void)
+{
+   return (!xen_hvm_domain() || xen_have_vector_callback);
+}
+
 #endif /* _ASM_X86_XEN_EVENTS_H */
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -447,7 +447,7 @@ void __init xen_msi_init(void)
 
 int __init pci_xen_hvm_init(void)
 {
-   if (!xen_feature(XENFEAT_hvm_pirqs))
+   if (!xen_have_vector_callback || !xen_feature(XENFEAT_hvm_pirqs))
return 0;
 
 #ifdef CONFIG_ACPI
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -138,6 +138,8 @@ struct shared_info xen_dummy_shared_info
 void *xen_initial_gdt;
 
 RESERVE_BRK(shared_info_page_brk, PAGE_SIZE);
+__read_mostly int xen_have_vector_callback;
+EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 
 static int xen_cpu_up_prepare(unsigned int cpu);
 static int xen_cpu_up_online(unsigned int cpu);
@@ -1861,7 +1863,9 @@ static int xen_cpu_up_prepare(unsigned i
xen_vcpu_setup(cpu);
}
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
xen_setup_timer(cpu);
 
rc = xen_smp_intr_init(cpu);
@@ -1877,7 +1881,9 @@ static int xen_cpu_dead(unsigned int cpu
 {
xen_smp_intr_free(cpu);
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
xen_teardown_timer(cpu);
 
return 0;
@@ -1916,8 +1922,8 @@ static void __init xen_hvm_guest_init(vo
 
xen_panic_handler_init();
 
-   BUG_ON(!xen_feature(XENFEAT_hvm_callback_vector));
-
+   if (xen_feature(XENFEAT_hvm_callback_vector))
+   xen_have_vector_callback = 1;
xen_hvm_smp_init();
WARN_ON(xen_cpuhp_setup());
xen_unplug_emulated_devices();
@@ -1958,7 +1964,7 @@ bool xen_hvm_n

Re: [Xen-devel] [PATCH for-4.9] xl: don't ignore return value from libxl_device_events_handler

2017-05-11 Thread Roger Pau Monné
On Thu, May 11, 2017 at 01:57:19PM +0100, Wei Liu wrote:
> That function can return a whole slew of error codes. Translate them
> to EXIT_FAILURE.
> 
> Signed-off-by: Wei Liu 

Acked-by: Roger Pau Monné 


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


Re: [Xen-devel] [PATCH v1 10/10] xen/arm: domain_build: Don't expose the "iommus" property to the guest

2017-05-11 Thread Oleksandr Tyshchenko
On Thu, May 11, 2017 at 2:58 PM, Julien Grall  wrote:
> Hi Oleksandr,
Hi Julien

>
> On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> We don't passthrough IOMMU device to DOM0 even if it is not used by
>> Xen. Therefore exposing the property that describes the IOMMU
>> master interfaces of the device does not make any sense.
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> ---
>>  xen/arch/arm/domain_build.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 3abacc0..2defb60 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -432,6 +432,10 @@ static int write_properties(struct domain *d, struct
>> kernel_info *kinfo,
>>  continue;
>>  }
>>
>> +/* Don't expose the property "iommus" to the guest */
>> +if ( dt_property_name_is_equal(prop, "iommus") )
>> +continue;
>
>
> It would be useful to have a link to the bindings associated
> (Documentation/devicetree/bindings/iommu/iommu.txt).
Agree. I will mention it in commit description.

>
> Also, whilst you are at it, you likely want to remove all the other iommu
> properties such as iommu-map.
Excuse me, I have never heard about it. Is it a required property?

>
>> +
>>  res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
>>
>>  if ( res )
>>
>
> Cheers,
>
> --
> Julien Grall


-- 
Regards,

Oleksandr Tyshchenko

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


[Xen-devel] [PATCH 4.10 125/129] xen: Revert commits da72ff5bfcb0 and 72a9b186292d

2017-05-11 Thread Greg Kroah-Hartman
4.10-stable review patch.  If anyone has any objections, please let me know.

--

From: Boris Ostrovsky 

commit 84d582d236dc1f9085e741affc72e9ba061a67c2 upstream.

Recent discussion (http://marc.info/?l=xen-devel&m=149192184523741)
established that commit 72a9b186292d ("xen: Remove event channel
notification through Xen PCI platform device") (and thus commit
da72ff5bfcb0 ("partially revert "xen: Remove event channel
notification through Xen PCI platform device"")) are unnecessary and,
in fact, prevent HVM guests from booting on Xen releases prior to 4.0

Therefore we revert both of those commits.

The summary of that discussion is below:

  Here is the brief summary of the current situation:

  Before the offending commit (72a9b186292):

  1) INTx does not work because of the reset_watches path.
  2) The reset_watches path is only taken if you have Xen > 4.0
  3) The Linux Kernel by default will use vector inject if the hypervisor
 support. So even INTx does not work no body running the kernel with
 Xen > 4.0 would notice. Unless he explicitly disabled this feature
 either in the kernel or in Xen (and this can only be disabled by
 modifying the code, not user-supported way to do it).

  After the offending commit (+ partial revert):

  1) INTx is no longer support for HVM (only for PV guests).
  2) Any HVM guest The kernel will not boot on Xen < 4.0 which does
 not have vector injection support. Since the only other mode
 supported is INTx which.

  So based on this summary, I think before commit (72a9b186292) we were
  in much better position from a user point of view.

Signed-off-by: Boris Ostrovsky 
Reviewed-by: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk 
Cc: Bjorn Helgaas 
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Vitaly Kuznetsov 
Cc: Paul Gortmaker 
Cc: Ross Lagerwall 
Cc: xen-de...@lists.xenproject.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: Anthony Liguori 
Cc: KarimAllah Ahmed 
Signed-off-by: Juergen Gross 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/include/asm/xen/events.h |   11 +++
 arch/x86/pci/xen.c|2 +-
 arch/x86/xen/enlighten.c  |   21 +++--
 arch/x86/xen/smp.c|2 ++
 arch/x86/xen/time.c   |5 +
 drivers/xen/events/events_base.c  |   26 +-
 drivers/xen/platform-pci.c|   13 +++--
 include/xen/xen.h |3 ++-
 8 files changed, 56 insertions(+), 27 deletions(-)

--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -20,4 +20,15 @@ static inline int xen_irqs_disabled(stru
 /* No need for a barrier -- XCHG is a barrier on x86. */
 #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
 
+extern int xen_have_vector_callback;
+
+/*
+ * Events delivered via platform PCI interrupts are always
+ * routed to vcpu 0 and hence cannot be rebound.
+ */
+static inline bool xen_support_evtchn_rebind(void)
+{
+   return (!xen_hvm_domain() || xen_have_vector_callback);
+}
+
 #endif /* _ASM_X86_XEN_EVENTS_H */
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -447,7 +447,7 @@ void __init xen_msi_init(void)
 
 int __init pci_xen_hvm_init(void)
 {
-   if (!xen_feature(XENFEAT_hvm_pirqs))
+   if (!xen_have_vector_callback || !xen_feature(XENFEAT_hvm_pirqs))
return 0;
 
 #ifdef CONFIG_ACPI
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -137,6 +137,8 @@ struct shared_info xen_dummy_shared_info
 void *xen_initial_gdt;
 
 RESERVE_BRK(shared_info_page_brk, PAGE_SIZE);
+__read_mostly int xen_have_vector_callback;
+EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 
 static int xen_cpu_up_prepare(unsigned int cpu);
 static int xen_cpu_up_online(unsigned int cpu);
@@ -1508,7 +1510,10 @@ static void __init xen_pvh_early_guest_i
if (!xen_feature(XENFEAT_auto_translated_physmap))
return;
 
-   BUG_ON(!xen_feature(XENFEAT_hvm_callback_vector));
+   if (!xen_feature(XENFEAT_hvm_callback_vector))
+   return;
+
+   xen_have_vector_callback = 1;
 
xen_pvh_early_cpu_init(0, false);
xen_pvh_set_cr_flags(0);
@@ -1847,7 +1852,9 @@ static int xen_cpu_up_prepare(unsigned i
xen_vcpu_setup(cpu);
}
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
xen_setup_timer(cpu);
 
rc = xen_smp_intr_init(cpu);
@@ -1863,7 +1870,9 @@ static int xen_cpu_dead(unsigned int cpu
 {
xen_smp_intr_free(cpu);
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
   

Re: [Xen-devel] [PATCH v1 04/10] xen/arm: p2m: Update IOMMU mapping whenever possible if page table is not shared

2017-05-11 Thread Oleksandr Tyshchenko
On Thu, May 11, 2017 at 2:24 PM, Julien Grall  wrote:
> Hi Oleksandr,
Hi Julien

>
> On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> Update IOMMU mapping if the IOMMU doesn't share page table with the CPU.
>> The best place to do so on ARM is __p2m_set_entry(). Use mfn as an
>> indicator
>> of the required action. If mfn is valid call iommu_map_pages(),
>> otherwise - iommu_unmap_pages().
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> CC: Julien Grall 
>
>
> Acked-by: Julien Grall 
Great. Thank you.

>
>
>>
>> ---
>>Changes in v1:
>>   - Update IOMMU mapping in __p2m_set_entry() instead of
>> p2m_set_entry().
>>   - Pass order argument to IOMMU APIs instead of page_count.
>> ---
>>  xen/arch/arm/p2m.c | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 34d5776..9ca491b 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -984,7 +984,15 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>>  p2m_free_entry(p2m, orig_pte, level);
>>
>>  if ( need_iommu(p2m->domain) && (p2m_valid(orig_pte) ||
>> p2m_valid(*entry)) )
>> -rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL <<
>> page_order);
>> +{
>> +if ( iommu_use_hap_pt(p2m->domain) )
>> +rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL <<
>> page_order);
>> +else if ( !mfn_eq(smfn, INVALID_MFN) )
>> +rc = iommu_map_pages(p2m->domain, gfn_x(sgfn), mfn_x(smfn),
>> + page_order, p2m_get_iommu_flags(t));
>> +else
>> +rc = iommu_unmap_pages(p2m->domain, gfn_x(sgfn), page_order);
>> +}
>>  else
>>  rc = 0;
>>
>>
>
> Cheers,
>
> --
> Julien Grall



-- 
Regards,

Oleksandr Tyshchenko

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


Re: [Xen-devel] [PATCH v3 4/9] mm: Scrub memory from idle loop

2017-05-11 Thread Boris Ostrovsky

>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 90e2b1f..a5f62b5 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -118,7 +118,8 @@ static void idle_loop(void)
>>  {
>>  if ( cpu_is_offline(smp_processor_id()) )
>>  play_dead();
>> -(*pm_idle)();
>> +if ( !scrub_free_pages() )
>> +(*pm_idle)();
>>  do_tasklet();
>>
> This means that, if we got here to run a tasklet (as in, if the idle
> vCPU has been forced into execution, because there were a vCPU context
> tasklet wanting to run), we will (potentially) do some scrubbing first.
>
> Is this on purpose, and, in any case, ideal? vCPU context tasklets are
> not terribly common, but I still don't think it is (ideal).
>
> Not sure how to address this, though. What (the variants of) pm_idle()
> uses for deciding whether or not to actually go to sleep is
> cpu_is_haltable(), which checks per_cpu(tasklet_work_to_do, cpu):
>
> /*
>  * Used by idle loop to decide whether there is work to do:
>  *  (1) Run softirqs; or (2) Play dead; or (3) Run tasklets.
>  */
> #define cpu_is_haltable(cpu)\
> (!softirq_pending(cpu) &&   \
>  cpu_online(cpu) && \
>  !per_cpu(tasklet_work_to_do, cpu))
>
> Pulling it out/adding a call to it (cpu_is_haltable()) is ugly, and
> probably not what we want (e.g., it's always called with IRQs disabled,
> while they're on here).
>
> Maybe we can test tasklet_work_to_do, before calling scrub_free_pages()
> (also ugly, IMO).
> Or, if scrub_free_pages() is, and always will be, called only from
> here, within the idle loop, test tasklet_work_to_do inside, similarly
> to what it does already for pending softirqs...

We can move do_tasklet() above scrub_free_pages(). And new tasklet after
that would result in a softirq being set so we'd do an early exit from
scrub_free_pages().

OTOH since, as you say, we only get to idle loop() if no tasklet is
pending (cpu_is_haltable() test) then would even that be needed?


-boris




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


[Xen-devel] [PATCH 4.9 099/103] xen: Revert commits da72ff5bfcb0 and 72a9b186292d

2017-05-11 Thread Greg Kroah-Hartman
4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Boris Ostrovsky 

commit 84d582d236dc1f9085e741affc72e9ba061a67c2 upstream.

Recent discussion (http://marc.info/?l=xen-devel&m=149192184523741)
established that commit 72a9b186292d ("xen: Remove event channel
notification through Xen PCI platform device") (and thus commit
da72ff5bfcb0 ("partially revert "xen: Remove event channel
notification through Xen PCI platform device"")) are unnecessary and,
in fact, prevent HVM guests from booting on Xen releases prior to 4.0

Therefore we revert both of those commits.

The summary of that discussion is below:

  Here is the brief summary of the current situation:

  Before the offending commit (72a9b186292):

  1) INTx does not work because of the reset_watches path.
  2) The reset_watches path is only taken if you have Xen > 4.0
  3) The Linux Kernel by default will use vector inject if the hypervisor
 support. So even INTx does not work no body running the kernel with
 Xen > 4.0 would notice. Unless he explicitly disabled this feature
 either in the kernel or in Xen (and this can only be disabled by
 modifying the code, not user-supported way to do it).

  After the offending commit (+ partial revert):

  1) INTx is no longer support for HVM (only for PV guests).
  2) Any HVM guest The kernel will not boot on Xen < 4.0 which does
 not have vector injection support. Since the only other mode
 supported is INTx which.

  So based on this summary, I think before commit (72a9b186292) we were
  in much better position from a user point of view.

Signed-off-by: Boris Ostrovsky 
Reviewed-by: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk 
Cc: Bjorn Helgaas 
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Vitaly Kuznetsov 
Cc: Paul Gortmaker 
Cc: Ross Lagerwall 
Cc: xen-de...@lists.xenproject.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: Anthony Liguori 
Cc: KarimAllah Ahmed 
Signed-off-by: Juergen Gross 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/include/asm/xen/events.h |   11 ++
 arch/x86/pci/xen.c|2 -
 arch/x86/xen/enlighten.c  |   21 
 arch/x86/xen/smp.c|2 +
 arch/x86/xen/time.c   |5 ++
 drivers/xen/events/events_base.c  |   26 ++-
 drivers/xen/platform-pci.c|   64 ++
 include/xen/xen.h |3 +
 8 files changed, 117 insertions(+), 17 deletions(-)

--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -20,4 +20,15 @@ static inline int xen_irqs_disabled(stru
 /* No need for a barrier -- XCHG is a barrier on x86. */
 #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
 
+extern int xen_have_vector_callback;
+
+/*
+ * Events delivered via platform PCI interrupts are always
+ * routed to vcpu 0 and hence cannot be rebound.
+ */
+static inline bool xen_support_evtchn_rebind(void)
+{
+   return (!xen_hvm_domain() || xen_have_vector_callback);
+}
+
 #endif /* _ASM_X86_XEN_EVENTS_H */
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -447,7 +447,7 @@ void __init xen_msi_init(void)
 
 int __init pci_xen_hvm_init(void)
 {
-   if (!xen_feature(XENFEAT_hvm_pirqs))
+   if (!xen_have_vector_callback || !xen_feature(XENFEAT_hvm_pirqs))
return 0;
 
 #ifdef CONFIG_ACPI
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -137,6 +137,8 @@ struct shared_info xen_dummy_shared_info
 void *xen_initial_gdt;
 
 RESERVE_BRK(shared_info_page_brk, PAGE_SIZE);
+__read_mostly int xen_have_vector_callback;
+EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 
 static int xen_cpu_up_prepare(unsigned int cpu);
 static int xen_cpu_up_online(unsigned int cpu);
@@ -1521,7 +1523,10 @@ static void __init xen_pvh_early_guest_i
if (!xen_feature(XENFEAT_auto_translated_physmap))
return;
 
-   BUG_ON(!xen_feature(XENFEAT_hvm_callback_vector));
+   if (!xen_feature(XENFEAT_hvm_callback_vector))
+   return;
+
+   xen_have_vector_callback = 1;
 
xen_pvh_early_cpu_init(0, false);
xen_pvh_set_cr_flags(0);
@@ -1860,7 +1865,9 @@ static int xen_cpu_up_prepare(unsigned i
xen_vcpu_setup(cpu);
}
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
xen_setup_timer(cpu);
 
rc = xen_smp_intr_init(cpu);
@@ -1876,7 +1883,9 @@ static int xen_cpu_dead(unsigned int cpu
 {
xen_smp_intr_free(cpu);
 
-   if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
  

Re: [Xen-devel] [PATCH v7 2/3] * util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64

2017-05-11 Thread Fu Wei
Hi Vladimir,

On 11 May 2017 at 06:01, Vladimir 'phcoder' Serbinenko
 wrote:
>
>
> On Tue, May 9, 2017, 11:02 Fu Wei  wrote:
>>
>> Hi Vladimir
>>
>> On 9 May 2017 at 14:59, Vladimir 'phcoder' Serbinenko 
>> wrote:
>> >
>> >
>> > Le Tue, May 2, 2017 à 9:06 AM,  a écrit :
>> >>
>> >> From: Fu Wei 
>> >>
>> >> This patch adds the support of xen_boot command for aarch64:
>> >> xen_hypervisor
>> >> xen_module
>> >> These two commands are only for aarch64, since it has its own protocol
>> >> and
>> >> commands to boot xen hypervisor and Dom0, but not multiboot.
>> >>
>> >> For other architectures, they are still using multiboot and module
>> >> commands.
>> >>
>> >> Signed-off-by: Fu Wei 
>> >> ---
>> >>  util/grub.d/20_linux_xen.in | 13 ++---
>> >>  1 file changed, 10 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
>> >> index c48af94..919 100644
>> >> --- a/util/grub.d/20_linux_xen.in
>> >> +++ b/util/grub.d/20_linux_xen.in
>> >> @@ -122,16 +122,16 @@ linux_entry ()
>> >>  else
>> >>  xen_rm_opts="no-real-mode edd=off"
>> >>  fi
>> >> -   multiboot   ${rel_xen_dirname}/${xen_basename} placeholder
>> >> ${xen_args} \${xen_rm_opts}
>> >> +   ${xen_loader}   ${rel_xen_dirname}/${xen_basename} placeholder
>> >> ${xen_args} \${xen_rm_opts}
>> >> echo'$(echo "$lmessage" | grub_quote)'
>> >> -   module  ${rel_dirname}/${basename} placeholder
>> >> root=${linux_root_device_thisversion} ro ${args}
>> >> +   ${module_loader}${rel_dirname}/${basename} placeholder
>> >> root=${linux_root_device_thisversion} ro ${args}
>> >>  EOF
>> >>if test -n "${initrd}" ; then
>> >>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>> >>  message="$(gettext_printf "Loading initial ramdisk ...")"
>> >>  sed "s/^/$submenu_indentation/" << EOF
>> >> echo'$(echo "$message" | grub_quote)'
>> >> -   module  --nounzip   ${rel_dirname}/${initrd}
>> >> +   ${module_loader}--nounzip   ${rel_dirname}/${initrd}
>> >>  EOF
>> >>fi
>> >>sed "s/^/$submenu_indentation/" << EOF
>> >> @@ -206,6 +206,13 @@ while [ "x${xen_list}" != "x" ] ; do
>> >>  if [ "x$is_top_level" != xtrue ]; then
>> >> echo "  submenu '$(gettext_printf "Xen hypervisor, version %s"
>> >> "${xen_version}" | grub_quote)' \$menuentry_id_option
>> >> 'xen-hypervisor-$xen_version-$boot_device_id' {"
>> >>  fi
>> >> +if [ "x$machine" != xaarch64 ]; then
>> >
>> > Machine of grub-mkconfig doesn't necessarily match the kernel. Think of
>> > chroot or of having 32-bit userspace with 64-bit kernel. Better to do
>> > this
>> > on runtime. I know, it's not very nice but the whole grub-mkconfig is
>> > trouble that needs redesign that I'm working on.
>>
>> So if we need to do this at run time(in grub shell), can I use
>> "grub_cpu" variable instead?
>
> Yes, you can. Another possibility, probably better, is to check actual file
> type, see grub-file

Very good idea, will do in my v8 patchset, will send v8 in a day.

>>
>>
>> Thanks!
>>
>> >>
>> >> +   xen_loader="multiboot"
>> >> +   module_loader="module"
>> >> +else
>> >> +   xen_loader="xen_hypervisor"
>> >> +   module_loader="xen_module"
>> >> +fi
>> >>  while [ "x$list" != "x" ] ; do
>> >> linux=`version_find_latest $list`
>> >> gettext_printf "Found linux image: %s\n" "$linux" >&2
>> >> --
>> >> 2.9.3
>> >>
>> >
>>
>>
>>
>> --
>> Best regards,
>>
>> Fu Wei
>> Software Engineer
>> Red Hat



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

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


[Xen-devel] Hypercall Inquiries

2017-05-11 Thread Rapidash
Greetings,

My co-worker and I are looking into Xen Hypervisor. By any chance, do you or 
any of your colleagues have technical material/ papers/ presentations detailing 
how the hypercall interacts with the hypervisor?

Thank you in advance for any assistance,
- Rapidash

Some specific questions:
- If the memory of the VM is stored on non-congruent sections of the host's 
machine memory, how does the hypercall handler check whether a passed in 
pointer parameter falls within these VM claimed sections of memory?
- Since the hypercalls are limited in number compared to syscalls, is there 
ever an instance where the domain will require a syscall that the hypercall 
does not cover?

Also, we have been looking at the Xen hypercall source code to try and figure 
out the mechanics there. In file "hypercall-x86_64.h" there is the following 
code segment we are attempting to decipher (from it, we can figure out the 
preceding functions):

#define _hypercall5(type, name, a1, a2, a3, a4, a5) \
({ \
long __res, __ign1, __ign2, __ign3; \
asm volatile ( \
"movq %7,%%r10; movq %8,%%r8; " \
"call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\
: "=a" (__res), "=D" (__ign1), "=S" (__ign2), \
"=d" (__ign3) \
: "1" ((long)(a1)), "2" ((long)(a2)), \
"3" ((long)(a3)), "g" ((long)(a4)), \
"g" ((long)(a5)) \
: "memory", "r10", "r8" ); \
(type)__res; \
})

- The first line within the asm volatile section, are the contents of registers 
being saved to memory? If so, where?
- In the third and fourth lines within the same section, are values from __res 
being placed into the "a" register, or are the values within the "a" register 
being stored in the variable __res for use later?
- Does the "1", "2", "3", "g", "g" correspond to the ebx, ecx, edx, esi, and 
edi registers? Or are they a different set?

Sent with [ProtonMail](https://protonmail.com) Secure Email.___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v1 05/10] iommu/arm: Re-define iommu_use_hap_pt(d) as iommu_hap_pt_share

2017-05-11 Thread Oleksandr Tyshchenko
On Thu, May 11, 2017 at 2:28 PM, Julien Grall  wrote:
> Hi Oleksandr,
Hi Julien

>
> On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> Not every integrated into ARM SoCs IOMMU can share page tables
>> with the CPU and as result the iommu_use_hap_pt(d) is not always true.
>> Reuse x86's iommu_hap_pt_share flag to indicate whether the IOMMU
>> page table is shared or not.
>>
>> Now all IOMMU drivers on ARM are able to change this flag
>> according to their possibilities like x86-variants do.
>> Therefore set iommu_hap_pt_share flag for SMMU because it always shares
>> page table with the CPU.
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> ---
>>  xen/drivers/passthrough/arm/smmu.c | 3 +++
>>  xen/include/asm-arm/iommu.h| 7 +--
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/arm/smmu.c
>> b/xen/drivers/passthrough/arm/smmu.c
>> index 527a592..86ee12a 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -2870,6 +2870,9 @@ static __init int arm_smmu_dt_init(struct
>> dt_device_node *dev,
>>
>> platform_features &= smmu->features;
>>
>> +   /* Always share P2M table between the CPU and the SMMU */
>> +   iommu_hap_pt_share = true;
>> +
>
>
> I would prefer to bail-out if someone try to unshare the page-table rather
> than overriding. This would help us to know if someone are try to do that.
>
> So I would do:
>
> if ( !iommu_hap_pt_share )
> {
> printk()
> return -EINVAL;
> }
I got it for SMMU.

But, for IPMMU we will override since iommu_hap_pt_share is true by
default. Right?

/*
* The IPMMU can't reuse P2M table since it only supports
* stage-1 page tables.
*/
iommu_hap_pt_share = false;

>
>> return 0;
>>  }
>>
>> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
>> index 57d9b1e..10a6f23 100644
>> --- a/xen/include/asm-arm/iommu.h
>> +++ b/xen/include/asm-arm/iommu.h
>> @@ -20,8 +20,11 @@ struct arch_iommu
>>  void *priv;
>>  };
>>
>> -/* Always share P2M Table between the CPU and the IOMMU */
>> -#define iommu_use_hap_pt(d) (1)
>> +/*
>> + * The ARM domain always has a P2M table, but not every integrated into
>> + * ARM SoCs IOMMU can use it as page table.
>
>
> The first part: "ARM domain has a P2M table" is pretty obvious. I would
> instead say: "Not every ARM SoCs IOMMU use the same page-table format as the
> processor.".
Agree.

>
>> + */
>> +#define iommu_use_hap_pt(d) (iommu_hap_pt_share)
>>
>>  const struct iommu_ops *iommu_get_ops(void);
>>  void __init iommu_set_ops(const struct iommu_ops *ops);
>>
>
> Cheers,
>
> --
> Julien Grall


-- 
Regards,

Oleksandr Tyshchenko

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


Re: [Xen-devel] Proposal to drop Windows XP tests from Xen Project CI

2017-05-11 Thread Ian Jackson
Ian Jackson writes ("Proposal to drop Windows XP tests from Xen Project CI"):
> Recently, the tests of Windows XP SP3 that are run by osstest, as part
> of the Xen Project's test suite, have started failing a lot more.
> 
> It is not clear what has caused this.  The failures are causing
> blockages: several of our `staging' branches are not gettting pushed
> to the corresponding `stable' or `master' branches.  Older Xen
> releases, as well as current master (4.9-rc) are affected.  This is a
> problem for Xen development.
> 
> In my capacity as osstest administrator, I have tried to find someone
> to help debug these.  (I don't have the knowledge to do so myself.)
> 
> I haven't had a good response.  My colleagues at Citrix tell me that
> their internal XenRT system, used for XenServer, will be dropping its
> own tests of Windows XP.  It's been suggested to me to simply drop the
> Xen Project tests of Windows XP.
> 
> If you thinks that XP should continue to work well, and therefore to
> be tested, I'm afraid I need your help.  Please contact me at the
> address above, and I can provide more details, help, etc.

I have just pushed to osstest pretest the change to drop testing of
Windows XP.  We are replacing it with tests of Windows Server 2016 and
Windows 10 (but of course as those are new tests, they will not count
for blocking regressions until they have passed).

Jan Beulich writes ("Re: [OSSTEST PATCH 3/3] make-flight: Drop Windows XP 
tests"):
> On 04.05.17 at 14:20,  wrote:
> > We drop the tests only on the branches:
...
> >   xen-4.7-testing
> >   xen-4.8-testing
> >   xen-unstable
> > 
> > The other branches are mostly out-of-support Xen branches.  These are
> > either old ones we are still doing security support for (and would
> > like to know about regressions on, even for old guests), or very old
> > ones which we don't expect to change ever.
> 
> 4.6, while having passed the 1.5 year general support limit, had
> its most recent stable release go out before that time span was
> over, so I think we owe the community another stable release
> there. Hence I'm wondering whether it should be grouped with
> 4.7 and 4.8 above.

Done.  (Ie, I am dropping the tests on Xen 4.6 too.)

Ian.

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


[Xen-devel] [libvirt test] 109301: tolerable all pass - PUSHED

2017-05-11 Thread osstest service owner
flight 109301 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109301/

Failures :-/ but no regressions.

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

version targeted for testing:
 libvirt  bf123952303abd3b715e6c05c5d147bda11a4996
baseline version:
 libvirt  1d07a5bf3c03309642068d20698a1f55739dafa2

Last test of basis   109246  2017-05-10 04:20:43 Z1 days
Testing same since   109301  2017-05-11 04:21:19 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrange 
  Jiri Denemark 
  Peter Krempa 
  Wim ten Have 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



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

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

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

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


Pushing

Re: [Xen-devel] [block-xen-blkback] question about pontential null pointer dereference

2017-05-11 Thread Gustavo A. R. Silva

Hi Juergen,

Quoting Juergen Gross :


On 10/05/17 18:49, Gustavo A. R. Silva wrote:


Hello everybody,

While looking into Coverity ID 1350942 I ran into the following piece of
code at drivers/block/xen-blkback/xenbus.c:490:

490static int xen_blkbk_remove(struct xenbus_device *dev)
491{
492struct backend_info *be = dev_get_drvdata(&dev->dev);
493
494pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
495
496if (be->major || be->minor)
497xenvbd_sysfs_delif(dev);
498
499if (be->backend_watch.node) {
500unregister_xenbus_watch(&be->backend_watch);
501kfree(be->backend_watch.node);
502be->backend_watch.node = NULL;
503}
504
505dev_set_drvdata(&dev->dev, NULL);
506
507if (be->blkif)
508xen_blkif_disconnect(be->blkif);
509
510/* Put the reference we set in xen_blkif_alloc(). */
511xen_blkif_put(be->blkif);
512kfree(be->mode);
513kfree(be);
514return 0;
515}

The issue here is that line 507 implies that be->blkif might be NULL. If
this is the case, there is a NULL pointer dereference when executing
line 511 once macro xen_blkif_put() dereference be->blkif

Is there any chance for be->blkif to be NULL at line 511?


Yes. xen_blkbk_probe() will call xen_blkbk_remove() with be->blkif being
NULL in the failure path.

The call to xen_blkif_put() should be guarded by the "if (be->blkif)" of
line 507, too.



Thanks for clarifying. I'll send a patch to fix this shortly.

--
Gustavo A. R. Silva







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


Re: [Xen-devel] [ARM] Native application design and discussion (I hope)

2017-05-11 Thread Volodymyr Babchuk
Hi Stefano,

On 10 May 2017 at 21:24, Stefano Stabellini  wrote:
> I just want to point out that the comparision with tasklets is not
> helpful. Tasklets involve the idle vcpu, which we are trying to step away
> from because it increases irq latency. Tasklets don't provide any
> isolation. The context switch model for the idle vcpu and for EL0 apps
> is different, thus it has a different cost.
>
> I think we shouldn't mention tasklets in this thread any longer.
Yep, you are right. Let's forget about tasklets and focus on EL0 apps.

I want summarize political (opposed to technical) part of the discussion.

We, here at EPAM, viewed EL0 apps primarily as a way to extend
hypervisor. Because when it comes to embedded and automotive, there
arise some ugly things, that are needed at hypervisor level:
TEE mediators (OP-TEE is a good TEE, but for example there is TI's
MSHIELD with deeply proprietary license), device drivers for vcopros,
device drivers for cpufreq, and so on.
Some of this things can't be included in hypervisor due to legal
issues, some - because of code size or complexity. And we can't run
them in stubdoms, because stubdoms are slow for certain use-cases, in
some cases they are insecure, in some cases they just don't fit at
all.

On other hand you consider EL0 apps as ideal host for emulators only.
I can see your point, because XEN was always viewed as hypervisor for
servers.
But servers have different requirements in comparison to embedded
applications. Traditional servers does not use hardware accelerated
video decoders, they don't need to disable cpu's or scale frequencies
to preserve energy (okay, they need to, but it is not as pressing, as
on battery-powered device), there almost no proprietary code (or even
proprietary blobs, argh!).
Looks like virtualization on embedded is the next big thing. Linux
kernel was able to satisfy both parties. I hope that XEN can do the
same.

So, going back to EL0 apps. Honestly, I'd prefer not to use them as
extension mechanism. Yes, they provide isolation, but interfacing with
them will be painful. Probably we can leave them to emulators only
(but as I can see, PL011 emulator is going to be merged right into
hypervisor. Will be there need for other emulators?).
What I really want to ask: what do you thing about old good modules
like ones in linux kernel? There will be no isolation, this is bad.
But:
 - you can load proprietary modules if you want to
 - they are fast
 - you can interface with them in a nativest way possible: just call a function

Artem, could you please comment from your side?

-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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


[Xen-devel] [PATCH] block: xen-blkback: add null check to avoid null pointer dereference

2017-05-11 Thread Gustavo A. R. Silva
Add null check before calling xen_blkif_put() to avoid potential
null pointer dereference.

Addresses-Coverity-ID: 1350942
Cc: Juergen Gross 
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/block/xen-blkback/xenbus.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c 
b/drivers/block/xen-blkback/xenbus.c
index 8fe61b5..1f3dfaa 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -504,11 +504,13 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
 
dev_set_drvdata(&dev->dev, NULL);
 
-   if (be->blkif)
+   if (be->blkif) {
xen_blkif_disconnect(be->blkif);
 
-   /* Put the reference we set in xen_blkif_alloc(). */
-   xen_blkif_put(be->blkif);
+   /* Put the reference we set in xen_blkif_alloc(). */
+   xen_blkif_put(be->blkif);
+   }
+
kfree(be->mode);
kfree(be);
return 0;
-- 
2.5.0


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


[Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread Julien Grall
Renaming the subject + adding more people in the conversation as this is 
not related to only ARM anymore.


On 11/05/17 16:19, Volodymyr Babchuk wrote:

Hi Stefano,

On 10 May 2017 at 21:24, Stefano Stabellini  wrote:

I just want to point out that the comparision with tasklets is not
helpful. Tasklets involve the idle vcpu, which we are trying to step away
from because it increases irq latency. Tasklets don't provide any
isolation. The context switch model for the idle vcpu and for EL0 apps
is different, thus it has a different cost.

I think we shouldn't mention tasklets in this thread any longer.

Yep, you are right. Let's forget about tasklets and focus on EL0 apps.

I want summarize political (opposed to technical) part of the discussion.

We, here at EPAM, viewed EL0 apps primarily as a way to extend
hypervisor. Because when it comes to embedded and automotive, there
arise some ugly things, that are needed at hypervisor level:
TEE mediators (OP-TEE is a good TEE, but for example there is TI's
MSHIELD with deeply proprietary license), device drivers for vcopros,
device drivers for cpufreq, and so on.
Some of this things can't be included in hypervisor due to legal
issues, some - because of code size or complexity. And we can't run
them in stubdoms, because stubdoms are slow for certain use-cases, in
some cases they are insecure, in some cases they just don't fit at
all.

On other hand you consider EL0 apps as ideal host for emulators only.
I can see your point, because XEN was always viewed as hypervisor for
servers.
But servers have different requirements in comparison to embedded
applications. Traditional servers does not use hardware accelerated
video decoders, they don't need to disable cpu's or scale frequencies
to preserve energy (okay, they need to, but it is not as pressing, as
on battery-powered device), there almost no proprietary code (or even
proprietary blobs, argh!).
Looks like virtualization on embedded is the next big thing. Linux
kernel was able to satisfy both parties. I hope that XEN can do the
same.

So, going back to EL0 apps. Honestly, I'd prefer not to use them as
extension mechanism. Yes, they provide isolation, but interfacing with
them will be painful. Probably we can leave them to emulators only
(but as I can see, PL011 emulator is going to be merged right into
hypervisor. Will be there need for other emulators?).
What I really want to ask: what do you thing about old good modules
like ones in linux kernel? There will be no isolation, this is bad.
But:
 - you can load proprietary modules if you want to
 - they are fast
 - you can interface with them in a nativest way possible: just call a function

Artem, could you please comment from your side?



--
Julien Grall

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


[Xen-devel] [ovmf baseline-only test] 71289: tolerable FAIL

2017-05-11 Thread Platform Team regression test user
This run is configured for baseline tests only.

flight 71289 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71289/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-libvirt   5 libvirt-buildfail   like 71288
 build-i386-libvirt5 libvirt-buildfail   like 71288

version targeted for testing:
 ovmf 29dc8aa861fac78c6d62391dff312db934b755e3
baseline version:
 ovmf a6c31c6d6349a51041d8b77df375c340043e36bd

Last test of basis71288  2017-05-11 10:48:55 Z0 days
Testing same since71289  2017-05-11 13:46:38 Z0 days1 attempts


People who touched revisions under test:
  Chao Zhang 
  Jeff Fan 
  Zhang, Chao B 

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



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


commit 29dc8aa861fac78c6d62391dff312db934b755e3
Author: Jeff Fan 
Date:   Thu May 11 15:01:39 2017 +0800

UefiCpuPkg/PiSmmCpuDxeSmm: Fix logic check error

Cc: Jiewen Yao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
Reviewed-by: Jiewen Yao 

commit b7025df8f9102a1698879aa451bf5af592c37bc1
Author: Jeff Fan 
Date:   Wed May 10 16:32:25 2017 +0800

UefiCpuPkg/PiSmmCpuDxeSmm: Check ProcessorId == INVALID_APIC_ID

If PcdCpuHotPlugSupport is TRUE, gSmst->NumberOfCpus will be the
PcdCpuMaxLogicalProcessorNumber. If gSmst->SmmStartupThisAp() is invoked for
those un-existed processors, ASSERT() happened in 
ConfigSmmCodeAccessCheck().

This fix is to check if ProcessorId is valid before invoke
gSmst->SmmStartupThisAp() in ConfigSmmCodeAccessCheck() and to check if
ProcessorId is valid in InternalSmmStartupThisAp() to avoid unexpected DEBUG
error message displayed.

Cc: Jiewen Yao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
Reviewed-by: Eric Dong 

commit 5d0933f9bab2781bf5df078d12c22d50df165617
Author: Jeff Fan 
Date:   Wed May 10 14:47:03 2017 +0800

UefiCpuPkg/SmmCpuFeaturesLib: Correct print level

Cc: Jiewen Yao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
Reviewed-by: Eric Dong 

commit 6afc643ce0eded9ccbd015a5cd6d2ba0e264b01d
Author: Jeff Fan 
Date:   Wed May 10 14:42:41 2017 +0800

UefiCpuPkg/SmmCpuFeaturesLib: Fix Ia32/SmiEntry.asm build issue

Cc: Jiewen Yao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
Reviewed-by: Eric Dong 

commit 6d92ae11d14abe39f2587a360bc5d6c370325cad
Author: Zhang, Chao B 
Date:   Thu May 11 13:08:30 2017 +0800

SecurityPkg: Add TCG Spec info to TCG related modules

Add TCG Spec compliance info to TCG related module INFs.

Cc: Qin Long 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
Reviewed-by: Qin Long 
Reviewed-by: Yao Jiewen 

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


Re: [Xen-devel] [PATCH v3 4/9] mm: Scrub memory from idle loop

2017-05-11 Thread Dario Faggioli
On Thu, 2017-05-11 at 10:19 -0400, Boris Ostrovsky wrote:
> > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > > index 90e2b1f..a5f62b5 100644
> > > --- a/xen/arch/x86/domain.c
> > > +++ b/xen/arch/x86/domain.c
> > > @@ -118,7 +118,8 @@ static void idle_loop(void)
> > >  {
> > >  if ( cpu_is_offline(smp_processor_id()) )
> > >  play_dead();
> > > -(*pm_idle)();
> > > +if ( !scrub_free_pages() )
> > > +(*pm_idle)();
> > >  do_tasklet();
> > > 
> > 
> > This means that, if we got here to run a tasklet (as in, if the
> > idle
> > vCPU has been forced into execution, because there were a vCPU
> > context
> > tasklet wanting to run), we will (potentially) do some scrubbing
> > first.
> > 
> We can move do_tasklet() above scrub_free_pages(). And new tasklet
> after
> that would result in a softirq being set so we'd do an early exit
> from
> scrub_free_pages().
> 
How early?

In fact, right now, if there is one tasklet queued, this is what
happens:

 tasklet_schedule(t)
   tasklet_enqueue(t)
 test_and_set_bit(_TASKLET_enqueued, tasklet_work_to_do);
 raise_softirq(SCHEDULE_SOFTIRQ);
 schedule()
   set_bit(_TASKLET_scheduled, tasklet_work_to_do)
   tasklet_work_scheduled = 1;
   do_schedule(tasklet_work_scheduled)
 csched_schedule(tasklet_work_to_do)
   snext = CSCHED_VCPU(idle_vcpu[cpu]);
 idle_loop()
   (*pm_idle)()
 if ( !cpu_is_haltable() ) return;
   do_tasklet() /* runs tasklet t */
 clear_bit(_TASKLET_enqueued, work_to_do); /* list_empty(list) == true */
 raise_softirq(SCHEDULE_SOFTIRQ);
   do_softirq()
 schedule()
   clear_bit(_TASKLET_scheduled, tasklet_work);
   tasklet_work_scheduled = 0;
   do_schedule(tasklet_work_scheduled)
 csched_schedule(tasklet_work_to_do)
   snext = CSCHED_VCPU(idle_vcpu[cpu]);
 idle_loop()
   (*pm_idle)()
 if ( !cpu_is_haltable() )
 ...

If we move do_tasklet up, as you suggest, this is what happens:

 tasklet_schedule(t)
   tasklet_enqueue(t)
 test_and_set_bit(_TASKLET_enqueued, tasklet_work_to_do);
 raise_softirq(SCHEDULE_SOFTIRQ);
 schedule()
   set_bit(_TASKLET_scheduled, tasklet_work_to_do)
   tasklet_work_scheduled = 1;
   do_schedule(tasklet_work_scheduled)
 csched_schedule(tasklet_work_to_do)
   snext = CSCHED_VCPU(idle_vcpu[cpu]);
 idle_loop()
   do_tasklet() /* runs tasklet t */
 clear_bit(_TASKLET_enqueued, work_to_do); /* list_empty(list) == true */
 raise_softirq(SCHEDULE_SOFTIRQ);
   if ( !scrub_free_pages() )
 //do some scrubbing, but softirq_pending() is true, so return 1
   do_softirq()
 schedule()
   clear_bit(_TASKLET_scheduled, tasklet_work);
   tasklet_work_scheduled = 0;
   do_schedule(tasklet_work_scheduled)
 csched_schedule(tasklet_work_to_do)
   snext = CSCHED_VCPU(idle_vcpu[cpu]);
 idle_loop()
   if ( !scrub_free_pages() )
 //do the scrubbing, returns 0, so we enter the if
     (*pm_idle)()
       if ( !cpu_is_haltable() )
 ...

IOW (provided I'm understanding your code right, of course), I still
see it happening that we switched to idle *not* because the system was
idle, but for running a tasklet, and yet we end up doing at least some
scrubbing (like if the system were idle).

Which still feels wrong to me.

If there's more than one tasklet queued (or another one, or more,
is/are queued before the one t is processed), it's even worse, because
we go through the whole schedule()->idle_loop()->do_tasklet() again and
again, and at each step we do a bit of scrubbing, before going back to
schedule().

It probably would be at least a bit better, if scrub_free_pages() would
check for softirqs() _before_ starting any scrubbing (which I don't
think it does, right now, am I right?).

> OTOH since, as you say, we only get to idle loop() if no tasklet is
> pending (cpu_is_haltable() test) then would even that be needed?
> 
Err... sorry, not getting. It's the other way round. One of the reasons
why we end up executing idle_loop(), is that there is at least a
tasklet pending.

Where we only get to if there's nothing pending is to calling
(*pm_idle)().

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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


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

2017-05-11 Thread osstest service owner
flight 109310 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109310/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   5 xen-buildfail REGR. vs. 107636
 build-arm64   5 xen-buildfail REGR. vs. 107636
 build-i3865 xen-buildfail REGR. vs. 107636
 build-amd64-xsm   5 xen-buildfail REGR. vs. 107636
 build-amd64   5 xen-buildfail REGR. vs. 107636
 build-armhf   5 xen-buildfail REGR. vs. 107636
 build-armhf-xsm   5 xen-buildfail REGR. vs. 107636
 build-i386-xsm5 xen-buildfail REGR. vs. 107636

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-intel  1 build-check(1)  blocked n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-winxpsp3  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-pvh-intel  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-

Re: [Xen-devel] [PATCH for-4.9] xl: don't ignore return value from libxl_device_events_handler

2017-05-11 Thread Ian Jackson
Wei Liu writes ("[PATCH for-4.9] xl: don't ignore return value from 
libxl_device_events_handler"):
> That function can return a whole slew of error codes. Translate them
> to EXIT_FAILURE.

Acked-by: Ian Jackson 

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


[Xen-devel] [Patch] Fix broken package config file xenlight.pc.in

2017-05-11 Thread Charles Arnold
The Requires line in this config file uses the wrong names for two dependencies.

The package config file for xenctrl is called 'xencontrol' and for blktapctl is
called 'xenblktapctl'. Running a command like 'pkg-config --exists xenlight' 
will
fail without this fix.

Signed-off-by: Charles Arnold 

diff --git a/tools/libxl/xenlight.pc.in b/tools/libxl/xenlight.pc.in
index 71d093a0ae..86c38a5634 100644
--- a/tools/libxl/xenlight.pc.in
+++ b/tools/libxl/xenlight.pc.in
@@ -9,4 +9,4 @@ Description: The Xenlight library for Xen hypervisor
 Version: @@version@@
 Cflags: -I${includedir}
 Libs: @@libsflag@@${libdir} -lxenlight
-Requires.private: xentoollog,xenevtchn,xenctrl,xenguest,xenstore,blktapctl
+Requires.private: 
xentoollog,xenevtchn,xencontrol,xenguest,xenstore,xenblktapctl



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


Re: [Xen-devel] [Patch] Fix broken package config file xenlight.pc.in

2017-05-11 Thread Wei Liu
On Thu, May 11, 2017 at 10:29:42AM -0600, Charles Arnold wrote:
> The Requires line in this config file uses the wrong names for two 
> dependencies.
> 
> The package config file for xenctrl is called 'xencontrol' and for blktapctl 
> is
> called 'xenblktapctl'. Running a command like 'pkg-config --exists xenlight' 
> will
> fail without this fix.
> 
> Signed-off-by: Charles Arnold 

Acked-by: Wei Liu 

Julien this should be in 4.9.

> 
> diff --git a/tools/libxl/xenlight.pc.in b/tools/libxl/xenlight.pc.in
> index 71d093a0ae..86c38a5634 100644
> --- a/tools/libxl/xenlight.pc.in
> +++ b/tools/libxl/xenlight.pc.in
> @@ -9,4 +9,4 @@ Description: The Xenlight library for Xen hypervisor
>  Version: @@version@@
>  Cflags: -I${includedir}
>  Libs: @@libsflag@@${libdir} -lxenlight
> -Requires.private: xentoollog,xenevtchn,xenctrl,xenguest,xenstore,blktapctl
> +Requires.private: 
> xentoollog,xenevtchn,xencontrol,xenguest,xenstore,xenblktapctl
> 
> 

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


Re: [Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread George Dunlap
On 11/05/17 16:35, Julien Grall wrote:
> On 11/05/17 16:19, Volodymyr Babchuk wrote:
>> Hi Stefano,
>>
>> On 10 May 2017 at 21:24, Stefano Stabellini 
>> wrote:
>>> I just want to point out that the comparision with tasklets is not
>>> helpful. Tasklets involve the idle vcpu, which we are trying to step
>>> away
>>> from because it increases irq latency. Tasklets don't provide any
>>> isolation. The context switch model for the idle vcpu and for EL0 apps
>>> is different, thus it has a different cost.
>>>
>>> I think we shouldn't mention tasklets in this thread any longer.
>> Yep, you are right. Let's forget about tasklets and focus on EL0 apps.
>>
>> I want summarize political (opposed to technical) part of the discussion.
>>
>> We, here at EPAM, viewed EL0 apps primarily as a way to extend
>> hypervisor. Because when it comes to embedded and automotive, there
>> arise some ugly things, that are needed at hypervisor level:
>> TEE mediators (OP-TEE is a good TEE, but for example there is TI's
>> MSHIELD with deeply proprietary license), device drivers for vcopros,
>> device drivers for cpufreq, and so on.
>> Some of this things can't be included in hypervisor due to legal
>> issues, some - because of code size or complexity. And we can't run
>> them in stubdoms, because stubdoms are slow for certain use-cases, in
>> some cases they are insecure, in some cases they just don't fit at
>> all.
>>
>> On other hand you consider EL0 apps as ideal host for emulators only.
>> I can see your point, because XEN was always viewed as hypervisor for
>> servers.
>> But servers have different requirements in comparison to embedded
>> applications. Traditional servers does not use hardware accelerated
>> video decoders, they don't need to disable cpu's or scale frequencies
>> to preserve energy (okay, they need to, but it is not as pressing, as
>> on battery-powered device), there almost no proprietary code (or even
>> proprietary blobs, argh!).
>> Looks like virtualization on embedded is the next big thing. Linux
>> kernel was able to satisfy both parties. I hope that XEN can do the
>> same.
>>
>> So, going back to EL0 apps. Honestly, I'd prefer not to use them as
>> extension mechanism. Yes, they provide isolation, but interfacing with
>> them will be painful. Probably we can leave them to emulators only
>> (but as I can see, PL011 emulator is going to be merged right into
>> hypervisor. Will be there need for other emulators?).
>> What I really want to ask: what do you thing about old good modules
>> like ones in linux kernel? There will be no isolation, this is bad.
>> But:
>>  - you can load proprietary modules if you want to
>>  - they are fast
>>  - you can interface with them in a nativest way possible: just call a
>> function

Even better would be to skip the module-loading step entirely, and just
compile proprietary code directly into your Xen binary.

Both solutions, unfortunately, are illegal.*

 -George

* I am not a lawyer, and this is not legal advice; but see this
presentation for a bit more information:
http://www.kroah.com/log/linux/ols_2006_keynote.html


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


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

2017-05-11 Thread osstest service owner
flight 109316 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/109316/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf f4ac4354652b2bcf4f138b5ebd79b2f07710d4ef
baseline version:
 ovmf 29dc8aa861fac78c6d62391dff312db934b755e3

Last test of basis   109312  2017-05-11 10:45:18 Z0 days
Testing same since   109316  2017-05-11 13:46:34 Z0 days1 attempts


People who touched revisions under test:
  Jeff Westfahl 
  Ruiyu Ni 

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



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

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

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

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


Pushing revision :

+ branch=ovmf
+ revision=f4ac4354652b2bcf4f138b5ebd79b2f07710d4ef
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
f4ac4354652b2bcf4f138b5ebd79b2f07710d4ef
+ branch=ovmf
+ revision=f4ac4354652b2bcf4f138b5ebd79b2f07710d4ef
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' xf4ac4354652b2bcf4f138b5ebd79b2f07710d4ef = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/gi

Re: [Xen-devel] [PATCH v3 4/9] mm: Scrub memory from idle loop

2017-05-11 Thread Boris Ostrovsky
On 05/11/2017 11:48 AM, Dario Faggioli wrote:
> On Thu, 2017-05-11 at 10:19 -0400, Boris Ostrovsky wrote:
 diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
 index 90e2b1f..a5f62b5 100644
 --- a/xen/arch/x86/domain.c
 +++ b/xen/arch/x86/domain.c
 @@ -118,7 +118,8 @@ static void idle_loop(void)
  {
  if ( cpu_is_offline(smp_processor_id()) )
  play_dead();
 -(*pm_idle)();
 +if ( !scrub_free_pages() )
 +(*pm_idle)();
  do_tasklet();

>>> This means that, if we got here to run a tasklet (as in, if the
>>> idle
>>> vCPU has been forced into execution, because there were a vCPU
>>> context
>>> tasklet wanting to run), we will (potentially) do some scrubbing
>>> first.
>>>
>> We can move do_tasklet() above scrub_free_pages(). And new tasklet
>> after
>> that would result in a softirq being set so we'd do an early exit
>> from
>> scrub_free_pages().
>>
> How early?
>
> In fact, right now, if there is one tasklet queued, this is what
> happens:
>
>  tasklet_schedule(t)
>tasklet_enqueue(t)
>  test_and_set_bit(_TASKLET_enqueued, tasklet_work_to_do);
>  raise_softirq(SCHEDULE_SOFTIRQ);
>  schedule()
>set_bit(_TASKLET_scheduled, tasklet_work_to_do)
>tasklet_work_scheduled = 1;
>do_schedule(tasklet_work_scheduled)
>  csched_schedule(tasklet_work_to_do)
>snext = CSCHED_VCPU(idle_vcpu[cpu]);
>  idle_loop()
>(*pm_idle)()
>  if ( !cpu_is_haltable() ) return;
>do_tasklet() /* runs tasklet t */
>  clear_bit(_TASKLET_enqueued, work_to_do); /* list_empty(list) == true */
>  raise_softirq(SCHEDULE_SOFTIRQ);
>do_softirq()
>  schedule()
>clear_bit(_TASKLET_scheduled, tasklet_work);
>tasklet_work_scheduled = 0;
>do_schedule(tasklet_work_scheduled)
>  csched_schedule(tasklet_work_to_do)
>snext = CSCHED_VCPU(idle_vcpu[cpu]);
>  idle_loop()
>(*pm_idle)()
>  if ( !cpu_is_haltable() )
>  ...
>
> If we move do_tasklet up, as you suggest, this is what happens:
>
>  tasklet_schedule(t)
>tasklet_enqueue(t)
>  test_and_set_bit(_TASKLET_enqueued, tasklet_work_to_do);
>  raise_softirq(SCHEDULE_SOFTIRQ);
>  schedule()
>set_bit(_TASKLET_scheduled, tasklet_work_to_do)
>tasklet_work_scheduled = 1;
>do_schedule(tasklet_work_scheduled)
>  csched_schedule(tasklet_work_to_do)
>snext = CSCHED_VCPU(idle_vcpu[cpu]);
>  idle_loop()
>do_tasklet() /* runs tasklet t */
>  clear_bit(_TASKLET_enqueued, work_to_do); /* list_empty(list) == true */
>  raise_softirq(SCHEDULE_SOFTIRQ);
>if ( !scrub_free_pages() )
>  //do some scrubbing, but softirq_pending() is true, so return 1
>do_softirq()
>  schedule()
>clear_bit(_TASKLET_scheduled, tasklet_work);
>tasklet_work_scheduled = 0;
>do_schedule(tasklet_work_scheduled)
>  csched_schedule(tasklet_work_to_do)
>snext = CSCHED_VCPU(idle_vcpu[cpu]);
>  idle_loop()
>if ( !scrub_free_pages() )
>  //do the scrubbing, returns 0, so we enter the if
>  (*pm_idle)()
>if ( !cpu_is_haltable() )
>  ...
>
> IOW (provided I'm understanding your code right, of course), I still
> see it happening that we switched to idle *not* because the system was
> idle, but for running a tasklet, and yet we end up doing at least some
> scrubbing (like if the system were idle).
>
> Which still feels wrong to me.
>
> If there's more than one tasklet queued (or another one, or more,
> is/are queued before the one t is processed), it's even worse, because
> we go through the whole schedule()->idle_loop()->do_tasklet() again and
> again, and at each step we do a bit of scrubbing, before going back to
> schedule().
>
> It probably would be at least a bit better, if scrub_free_pages() would
> check for softirqs() _before_ starting any scrubbing (which I don't
> think it does, right now, am I right?).

Right.

I didn't realize that do_tasklet() also schedules softirq. So you are
suggesting something along the lines of

do_tasklet();

if ( !softirq_pending(smp_processor_id() && !scrub_free_pages() )
(*pm_idle)();

do_softirq();


>
>> OTOH since, as you say, we only get to idle loop() if no tasklet is
>> pending (cpu_is_haltable() test) then would even that be needed?
>>
> Err... sorry, not getting. It's the other way round. One of the reasons
> why we end up executing idle_loop(), is that there is at least a
> tasklet pending.

Nevermind that. I was thinking we enter idle_loop() based on
cpu_is_haltable().

-boris

>
> Where we only get to if there's nothing pending is to calling
> (*pm_idle)().
>
> Regards,
> Dario




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


[Xen-devel] [PATCH for-4.9] x86/mm: Mark pages as dirty after (rather than before) writing to them

2017-05-11 Thread Andrew Cooper
Otherwise a logdirty client can race with observing the page becoming dirty,
and copy the frame before the write is complete and end up with a stale
version.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Julien Grall 
---
 xen/arch/x86/mm.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 77b0af1..97c3cb8 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3556,11 +3556,10 @@ long do_mmuext_op(
 break;
 }
 
-/* A page is dirtied when it's being cleared. */
-paging_mark_dirty(pg_owner, _mfn(page_to_mfn(page)));
-
 clear_domain_page(_mfn(page_to_mfn(page)));
 
+paging_mark_dirty(pg_owner, _mfn(page_to_mfn(page)));
+
 put_page_and_type(page);
 break;
 }
@@ -3594,12 +3593,11 @@ long do_mmuext_op(
 break;
 }
 
-/* A page is dirtied when it's being copied to. */
-paging_mark_dirty(pg_owner, _mfn(page_to_mfn(dst_page)));
-
 copy_domain_page(_mfn(page_to_mfn(dst_page)),
  _mfn(page_to_mfn(src_page)));
 
+paging_mark_dirty(pg_owner, _mfn(page_to_mfn(dst_page)));
+
 put_page_and_type(dst_page);
 put_page(src_page);
 break;
-- 
2.1.4


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


Re: [Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread George Dunlap
On 11/05/17 16:35, Julien Grall wrote:
> Renaming the subject + adding more people in the conversation as this is
> not related to only ARM anymore.
> 
> On 11/05/17 16:19, Volodymyr Babchuk wrote:
>> Hi Stefano,
>>
>> On 10 May 2017 at 21:24, Stefano Stabellini 
>> wrote:
>>> I just want to point out that the comparision with tasklets is not
>>> helpful. Tasklets involve the idle vcpu, which we are trying to step
>>> away
>>> from because it increases irq latency. Tasklets don't provide any
>>> isolation. The context switch model for the idle vcpu and for EL0 apps
>>> is different, thus it has a different cost.
>>>
>>> I think we shouldn't mention tasklets in this thread any longer.
>> Yep, you are right. Let's forget about tasklets and focus on EL0 apps.
>>
>> I want summarize political (opposed to technical) part of the discussion.
>>
>> We, here at EPAM, viewed EL0 apps primarily as a way to extend
>> hypervisor. Because when it comes to embedded and automotive, there
>> arise some ugly things, that are needed at hypervisor level:
>> TEE mediators (OP-TEE is a good TEE, but for example there is TI's
>> MSHIELD with deeply proprietary license),

If you're going to use a deeply proprietary TEE mediator, then you need
to find yourself a deeply proprietary hypervisor to go along with it --
either one you pay a license fee for or one you develop yourself.  It
would almost certainly be cheaper to improve the open-source one than to
do either of those.

Or you can try mixing the two and see what happens; but that doesn't
seem like a very sound legal strategy to me.

>> ...some [things can't be included in hypervisor] because of code
>> size or complexity.

Sorry, just to be clear: below you mentioned modules as a solution, and
given the context this would be included.  So can you expand on what you
mean that there are things that 1) can't be included in the hypervisor
because of code size or complexity, but for which 2) loadable modules
would be a suitable solution?

>> And we can't run
>> them in stubdoms, because stubdoms are slow for certain use-cases, in
>> some cases they are insecure, in some cases they just don't fit at
>> all.
>> On other hand you consider EL0 apps as ideal host for emulators only.
>> I can see your point, because XEN was always viewed as hypervisor for
>> servers.
>> But servers have different requirements in comparison to embedded
>> applications. Traditional servers does not use hardware accelerated
>> video decoders, they don't need to disable cpu's or scale frequencies
>> to preserve energy (okay, they need to, but it is not as pressing, as
>> on battery-powered device), there almost no proprietary code (or even
>> proprietary blobs, argh!).
>> Looks like virtualization on embedded is the next big thing. Linux
>> kernel was able to satisfy both parties. I hope that XEN can do the
>> same.

For many of these, there are probably technical solutions that we could
come up with that would allow proprietary content (such as video
decoders &c) that would have suitable performance without needing access
to the Xen address space.

Maybe I'm just not familiar with things, but it's hard for me to imagine
why you'd need proprietary blobs to disable cpus or scale frequency.
Are these really such complex activities that it's worth investing
thousands of hours of developer work into developing proprietary
solutions that you license?

Loading proprietary modules into Linux is as illegal as it would be in
Xen.  Many people obviously do it anyway, but you are really putting
yourself at a risk of meeting a guy like Patrick McHardy[1], a private
individual with copyright on the Linux kernel who by some estimates has
made almost EUR 2m in the last few years suing companies for GPL violations.

 -George

[1] https://lwn.net/Articles/721458/

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


Re: [Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread Volodymyr Babchuk
Hi George,

On 11 May 2017 at 19:35, George Dunlap  wrote:
> Even better would be to skip the module-loading step entirely, and just
> compile proprietary code directly into your Xen binary.
>
> Both solutions, unfortunately, are illegal.*
Look, I don't saying we want to produce closed-source modules or apps.
We want to write open source code. Just imagine, that certain header
files have some proprietary license (e.g. some device interface
definition and this interface is IP of company which developed it).
AFAIK, it can't be included into Xen distribution. I thought, that it
can be included in some module with different (but still open source)
license.  But if you say that it can't... Then I don't know. It is out
of my competence. I'm not lawyer also.

-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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


Re: [Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread George Dunlap
On Thu, May 11, 2017 at 6:14 PM, George Dunlap  wrote:
> yourself at a risk of meeting a guy like Patrick McHardy[1], a private
> individual with copyright on the Linux kernel

This should be "copyright on *code in the* Linux Kernel".  Obviously
he doesn't own a copyright on the whole thing, just a decent chunk of
it.

 -George

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


[Xen-devel] Added 5 additional Design sessions to Developer Summit Schedule

2017-05-11 Thread Lars Kurth
Hi everyone,

I added the following sessions to the Design Part of the Summit: you can see 
them via 
https://xendeveloperanddesignsummit2017.sched.com/overview/type/Interactive+Design+%26+Problem+Solving+Session
 (just showing Design sessions)

Please also make use of the "Add to my Sched(ule)" feature in 
https://xendeveloperanddesignsummit2017.sched.com: this will help me identify 
scheduling conflicts *before* the event and will make the event smoother: in 
other words we can make scheduling decisions using real data, before the event.

Added Sessions:

July 12
===
Design Discussion: Intel New QoS (RDT) Features - Yi Sun, Intel 

Design Discussion: SGX virtualization - Kai Huang, Intel

Design Discussion: Support for 5-level paging (including support for PV-guests) 
- Yi Liu, Intel & Jürgen Groß, SUSE 
Note: We merged a proposal from Yi and Jürgen. If we need more time, we can add 
another session on the 13th

July 13
===
Design Discussion: Shared Virtual Memory Virtualization Implementation on Xen - 
Yi Liu, Intel

Best Regards
Lars
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] Modules support in Xen (WAS: Re: [ARM] Native application design and discussion (I hope))

2017-05-11 Thread George Dunlap
On Thu, May 11, 2017 at 6:14 PM, Volodymyr Babchuk
 wrote:
> Hi George,
>
> On 11 May 2017 at 19:35, George Dunlap  wrote:
>> Even better would be to skip the module-loading step entirely, and just
>> compile proprietary code directly into your Xen binary.
>>
>> Both solutions, unfortunately, are illegal.*
> Look, I don't saying we want to produce closed-source modules or apps.
> We want to write open source code. Just imagine, that certain header
> files have some proprietary license (e.g. some device interface
> definition and this interface is IP of company which developed it).
> AFAIK, it can't be included into Xen distribution. I thought, that it
> can be included in some module with different (but still open source)
> license.  But if you say that it can't... Then I don't know. It is out
> of my competence. I'm not lawyer also.

I see.  That's good to know, but it doesn't change the legal aspect of
things. :-0

It used to be held that the information contained in headers --
constants, interface definitions, and so on -- weren't copyrightable;
in which case you could just include the header (or a modified version
of it) without any problems.  Unfortunately Oracle v Google may have
changed that.  But you'd have to ask a lawyer about that...

 -George

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


[Xen-devel] [resend PATCH] xen: common: rbtree: ported updates from linux tree

2017-05-11 Thread Praveen Kumar
The patch contains the updated version of rbtree implementation from linux
kernel tree containing the fixes so far handled.

Signed-off-by: Praveen Kumar 
---
 xen/common/rbtree.c| 748 +
 xen/include/xen/compiler.h |  60 +++
 xen/include/xen/rbtree.h   | 120 --
 xen/include/xen/rbtree_augmented.h | 283 ++
 4 files changed, 931 insertions(+), 280 deletions(-)
 create mode 100644 xen/include/xen/rbtree_augmented.h

diff --git a/xen/common/rbtree.c b/xen/common/rbtree.c
index 3328960d56..ae152c5bf2 100644
--- a/xen/common/rbtree.c
+++ b/xen/common/rbtree.c
@@ -2,7 +2,8 @@
   Red Black Trees
   (C) 1999  Andrea Arcangeli 
   (C) 2002  David Woodhouse 
-  
+  (C) 2012  Michel Lespinasse 
+
   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
@@ -14,286 +15,479 @@
   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 .
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
   linux/lib/rbtree.c
 */
 
+#include 
 #include 
-#include 
-
-static void __rb_rotate_left(struct rb_node *node, struct rb_root *root)
-{
-struct rb_node *right = node->rb_right;
-struct rb_node *parent = rb_parent(node);
 
-if ((node->rb_right = right->rb_left))
-rb_set_parent(right->rb_left, node);
-right->rb_left = node;
+/*
+ * red-black trees properties:  http://en.wikipedia.org/wiki/Rbtree
+ *
+ *  1) A node is either red or black
+ *  2) The root is black
+ *  3) All leaves (NULL) are black
+ *  4) Both children of every red node are black
+ *  5) Every simple path from root to leaves contains the same number
+ * of black nodes.
+ *
+ *  4 and 5 give the O(log n) guarantee, since 4 implies you cannot have two
+ *  consecutive red nodes in a path and every red node is therefore followed by
+ *  a black. So if B is the number of black nodes on every simple path (as per
+ *  5), then the longest possible path due to 4 is 2B.
+ *
+ *  We shall indicate color with case, where black nodes are uppercase and red
+ *  nodes will be lowercase. Unknown color nodes shall be drawn as red within
+ *  parentheses and have some accompanying text comment.
+ */
 
-rb_set_parent(right, parent);
+/*
+ * Notes on lockless lookups:
+ *
+ * All stores to the tree structure (rb_left and rb_right) must be done using
+ * WRITE_ONCE(). And we must not inadvertently cause (temporary) loops in the
+ * tree structure as seen in program order.
+ *
+ * These two requirements will allow lockless iteration of the tree -- not
+ * correct iteration mind you, tree rotations are not atomic so a lookup might
+ * miss entire subtrees.
+ *
+ * But they do guarantee that any such traversal will only see valid elements
+ * and that it will indeed complete -- does not get stuck in a loop.
+ *
+ * It also guarantees that if the lookup returns an element it is the 'correct'
+ * one. But not returning an element does _NOT_ mean it's not present.
+ *
+ * NOTE:
+ *
+ * Stores to __rb_parent_color are not important for simple lookups so those
+ * are left undone as of now. Nor did I check for loops involving parent
+ * pointers.
+ */
 
-if (parent)
-{
-if (node == parent->rb_left)
-parent->rb_left = right;
-else
-parent->rb_right = right;
-}
-else
-root->rb_node = right;
-rb_set_parent(node, right);
+static inline void rb_set_black(struct rb_node *rb)
+{
+rb->__rb_parent_color |= RB_BLACK;
 }
 
-static void __rb_rotate_right(struct rb_node *node, struct rb_root *root)
+static inline struct rb_node *rb_red_parent(struct rb_node *red)
 {
-struct rb_node *left = node->rb_left;
-struct rb_node *parent = rb_parent(node);
-
-if ((node->rb_left = left->rb_right))
-rb_set_parent(left->rb_right, node);
-left->rb_right = node;
-
-rb_set_parent(left, parent);
+return (struct rb_node *)red->__rb_parent_color;
+}
 
-if (parent)
-{
-if (node == parent->rb_right)
-parent->rb_right = left;
-else
-parent->rb_left = left;
-}
-else
-root->rb_node = left;
-rb_set_parent(node, left);
+/*
+ * Helper function for rotations:
+ * - old's parent and color get assigned to new
+ * - old gets assigned new as a parent and 'color' as a color.
+ */
+static inline void
+__rb_rotate_set_parents(struct rb_node *old, struct rb_node *new,
+struct rb_root *root, int color)
+{
+struct rb_node *parent = rb_parent(old);
+new->__rb_parent_color = old->__rb_parent_color;
+rb_set_parent_color(old, new, color);
+__rb_change_child(old, new, par

  1   2   >