Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests

2017-02-13 Thread Andrew Cooper
On 14/02/2017 02:52, Tian, Kevin wrote:
>> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
>> Sent: Monday, February 13, 2017 10:32 PM
>>
>> hvm_hw_cpu->msr_flags is in fact the VMX dirty bitmap of MSRs needing to be
>> restored when switching into guest context.  It should never have been part 
>> of
>> the migration state to start with, and Xen must not make any decisions based
>> on the value seen during restore.
>>
>> Identify it as obsolete in the header files, consistently save it as zero and
>> ignore it on restore.
>>
>> The MSRs must be considered dirty during VMCS creation to cause the proper
>> defaults of 0 to be visible to the guest.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Kevin Tian , with one small comment.
>
> the effect of this patch should be more than not leaking syscall MSR.
> what about making the subject clearer when check-in?

What other effects do you think are going on here?  Yes, we now context
switch the MSRs right from the start of the domain, but that is
necessary to avoid leaking them.

~Andrew

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


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

2017-02-13 Thread osstest service owner
flight 105778 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105778/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 
guest-localmigrate/x10 fail REGR. vs. 59254
 test-armhf-armhf-libvirt  6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-credit2   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-arndale   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-libvirt-xsm  6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail REGR. vs. 59254

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail in 105767 pass in 105778
 test-amd64-amd64-xl-qemut-debianhvm-amd64 13 guest-localmigrate fail pass in 
105767

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-rtds  9 debian-installfail REGR. vs. 59254
 test-armhf-armhf-libvirt-raw  6 xen-bootfail baseline untested
 test-armhf-armhf-xl-vhd   6 xen-bootfail baseline untested
 test-amd64-amd64-rumprun-amd64 16 rumprun-demo-xenstorels/xenstorels.repeat 
fail in 105767 baseline untested
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   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-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 build-arm64   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail 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-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 linux7089db84e356562f8ba737c29e472cc42d530dbc
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z  586 days
Failing since 59348  2015-07-10 04:24:05 Z  585 days  272 attempts
Testing same since   105757  2017-02-13 03:57:45 Z1 days3 attempts


7580 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  fail
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  blocked 
 build-armhf-libvirt 

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

2017-02-13 Thread osstest service owner
flight 105782 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105782/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 5b97eb4c35316cbe8235ae526209263da818e1a4
baseline version:
 ovmf 296153c5bf9976a3b5f00566819f109d1c23c135

Last test of basis   105760  2017-02-13 07:45:53 Z0 days
Testing same since   105782  2017-02-14 01:16:33 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=5b97eb4c35316cbe8235ae526209263da818e1a4
+ . ./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 
5b97eb4c35316cbe8235ae526209263da818e1a4
+ branch=ovmf
+ revision=5b97eb4c35316cbe8235ae526209263da818e1a4
+ . ./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
+ '[' x5b97eb4c35316cbe8235ae526209263da818e1a4 = 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
++ : 

[Xen-devel] HostedXen: Build Instructions

2017-02-13 Thread rohan kumbhar
Hi,

I am interested to restart Hosted Xen Project.

I have gone through the presentation of Hosted Xen and Ian Pratt's video in
Xen Summit 9.
I have secured the source code for HostedXen from
https://downloads.xen.org/HostedXen/release/

Tried to build it on windows but its failing.
It would be of utmost help if detailed build instructions be available.

I have searched the archives with no success.

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


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

2017-02-13 Thread osstest service owner
flight 105777 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105777/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2 3 host-install(3)broken REGR. vs. 105756
 build-amd64-xsm   5 xen-buildfail REGR. vs. 105756
 build-armhf-libvirt   4 host-build-prep  fail REGR. vs. 105756

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 105756
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 105756
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 105756
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 105756
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 105756

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-xl-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
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-arm64-arm64-xl   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-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 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-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 build-arm64   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-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-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-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-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass

version targeted for testing:
 xen  28722e98f7ff1ef0da1bef885f98e232f89a8ca7
baseline version:
 xen  6f6d3b10ec8168e2a78cf385d89803397f116397

Last test of 

Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling

2017-02-13 Thread Tian, Kevin
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Monday, February 13, 2017 9:04 PM
> 
> Sending an invalidation to the device model is an internal detail of
> completing the hypercall; callers should not need to be responsible for it.
> Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
> appropriate.
> 
> This makes the function boolean in nature, although the existing
> HVM_HCALL_{completed,preempted} to aid code clarity.  While updating the
> return type, drop _do from the name, as it is redundant.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Kevin Tian 

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


Re: [Xen-devel] [PATCH 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Tian, Kevin
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Monday, February 13, 2017 10:33 PM
> 
> To avoid leaking host MSR state into guests, guest LSTAR, STAR and
> SYSCALL_MASK state is unconditionally loaded when switching into guest
> context.
> 
> Attempting to dirty-track the state is pointless; host state is always
> restoring upon exit from guest context, meaning that guest state is always
> considered dirty.
> 
> Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The guests
> MSR values are stored plainly in arch_vmx_struct, in the same way as shadow_gs
> and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure
> that the hardware MSR values are always up-to-date.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Kevin Tian 

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


Re: [Xen-devel] [PATCH 3/4] x86/vmx: Remove vmx_save_host_msrs() and host_msr_state

2017-02-13 Thread Tian, Kevin
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Monday, February 13, 2017 10:32 PM
> 
> A pcpu's LSTAR, STAR and SYSCALL_MASK MSRs are unconditionally switched when
> moving in and out of HVM vcpu context.  Two of these values are compile time
> constants, and the third is directly available in an existing per-cpu
> variable.
> 
> There is no need to save host state in vmx_cpu_up() into a different per-cpu
> structure, so drop all the infrastructure.  vmx_restore_host_msrs() is
> simplified to 3 plain WRMSR instructions.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Kevin Tian 

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


Re: [Xen-devel] [PATCH 1/4] x86/vmx: Don't leak host syscall MSR state into HVM guests

2017-02-13 Thread Tian, Kevin
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: Monday, February 13, 2017 10:32 PM
> 
> hvm_hw_cpu->msr_flags is in fact the VMX dirty bitmap of MSRs needing to be
> restored when switching into guest context.  It should never have been part of
> the migration state to start with, and Xen must not make any decisions based
> on the value seen during restore.
> 
> Identify it as obsolete in the header files, consistently save it as zero and
> ignore it on restore.
> 
> The MSRs must be considered dirty during VMCS creation to cause the proper
> defaults of 0 to be visible to the guest.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Kevin Tian , with one small comment.

the effect of this patch should be more than not leaking syscall MSR.
what about making the subject clearer when check-in?

Thanks
Kevin

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


Re: [Xen-devel] [PATCH v4 3/3] x86/vvmx: correctly emulate VMREAD

2017-02-13 Thread Tian, Kevin
> From: Sergey Dyasli [mailto:sergey.dya...@citrix.com]
> Sent: Monday, February 13, 2017 10:21 PM
> 
> There is an issue with the original __vmread() in nested vmx mode:
> emulation of a guest's VMREAD with invalid arguments leads to BUG().
> 
> Fix this by using vmread_safe() and reporting any kind of VMfail back
> to the guest.
> 
> A new safe versions of get_vvmcs() macro and related functions are
> introduced because of new function signatures and lots of existing
> users.
> 
> Signed-off-by: Sergey Dyasli 

Acked-by: Kevin Tian 

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


Re: [Xen-devel] [PATCH v4 2/3] x86/vvmx: correctly emulate VMWRITE

2017-02-13 Thread Tian, Kevin
> From: Sergey Dyasli [mailto:sergey.dya...@citrix.com]
> Sent: Monday, February 13, 2017 10:21 PM
> 
> There is an issue with the original __vmwrite() in nested vmx mode:
> emulation of a guest's VMWRITE with invalid arguments leads to BUG().
> 
> Fix this by using vmwrite_safe() and reporting any kind of VMfail back
> to the guest.
> 
> A new safe versions of set_vvmcs() macro and related functions are
> introduced because of new function signatures and lots of existing
> users.
> 
> Signed-off-by: Sergey Dyasli 


Acked-by: Kevin Tian 

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


Re: [Xen-devel] [PATCH v4 1/3] x86/vmx: introduce VMX_INSN_SUCCEED

2017-02-13 Thread Tian, Kevin
> From: Sergey Dyasli [mailto:sergey.dya...@citrix.com]
> Sent: Monday, February 13, 2017 10:21 PM
> 
> The new value corresponds to VMsucceed status of VMX instructions.
> This will replace usage of literal zeroes in related functions.
> 
> Update vmfail(), vmread_safe() and vmwrite_safe().
> 
> Signed-off-by: Sergey Dyasli 

Acked-by: Kevin Tian 

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


[Xen-devel] [PATCH] x86/vpmu: fix vpmu can't enabled in guest

2017-02-13 Thread Luwei Kang
vpmu_enable() can not use for check if vpmu is enabled in guest during
booting up. Because linux kernel get the status of if supported pmu
is earler than xen vpmu initialise. vpmu_enable() will return false
even if vpmu has been enabled in guest.

Signed-off-by: Luwei Kang 
---
 xen/arch/x86/cpu/vpmu.c|  2 +-
 xen/arch/x86/cpuid.c   | 11 +--
 xen/include/asm-x86/vpmu.h |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 35a9403..8c8ffc9 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -50,7 +50,7 @@ CHECK_pmu_params;
  * "vpmu=arch" : vpmu enabled for predef arch counters only (restrictive)
  * flag combinations are allowed, eg, "vpmu=ipc,bts".
  */
-static unsigned int __read_mostly opt_vpmu_enabled;
+unsigned int __read_mostly opt_vpmu_enabled;
 unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF;
 unsigned int __read_mostly vpmu_features = 0;
 static void parse_vpmu_params(char *s);
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index e0a387e..b63c5d8 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -713,8 +713,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
 }
 }
 
-if ( vpmu_enabled(curr) &&
- vpmu_is_set(vcpu_vpmu(curr), VPMU_CPU_HAS_DS) )
+if ( opt_vpmu_enabled && boot_cpu_has(X86_FEATURE_DS) )
 {
 res->d |= cpufeat_mask(X86_FEATURE_DS);
 if ( cpu_has(_cpu_data, X86_FEATURE_DTES64) )
@@ -726,7 +725,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
 
 case 0x000a: /* Architectural Performance Monitor Features (Intel) */
 if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- !vpmu_enabled(curr) )
+ !opt_vpmu_enabled )
 goto unsupported;
 
 /* Report at most version 3 since that's all we currently emulate. */
@@ -793,8 +792,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
 if ( !hap_enabled(d) && !hvm_pae_enabled(v) )
 res->d &= ~cpufeat_mask(X86_FEATURE_PSE36);
 
-if ( vpmu_enabled(v) &&
- vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_DS) )
+if ( opt_vpmu_enabled && boot_cpu_has(X86_FEATURE_DS) )
 {
 res->d |= cpufeat_mask(X86_FEATURE_DS);
 if ( cpu_has(_cpu_data, X86_FEATURE_DTES64) )
@@ -811,7 +809,8 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
 break;
 
 case 0x000a: /* Architectural Performance Monitor Features (Intel) */
-if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || !vpmu_enabled(v) )
+if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+   !opt_vpmu_enabled )
 {
 *res = EMPTY_LEAF;
 break;
diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h
index e50618f..1148d66 100644
--- a/xen/include/asm-x86/vpmu.h
+++ b/xen/include/asm-x86/vpmu.h
@@ -25,7 +25,6 @@
 
 #define vcpu_vpmu(vcpu)   (&(vcpu)->arch.vpmu)
 #define vpmu_vcpu(vpmu)   container_of((vpmu), struct vcpu, arch.vpmu)
-#define vpmu_enabled(vcpu) vpmu_is_set(vcpu_vpmu(vcpu), VPMU_CONTEXT_ALLOCATED)
 
 #define MSR_TYPE_COUNTER0
 #define MSR_TYPE_CTRL   1
@@ -121,6 +120,7 @@ static inline int vpmu_do_rdmsr(unsigned int msr, uint64_t 
*msr_content)
 return vpmu_do_msr(msr, msr_content, 0, 0);
 }
 
+extern unsigned int opt_vpmu_enabled;
 extern unsigned int vpmu_mode;
 extern unsigned int vpmu_features;
 
-- 
1.8.3.1


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


Re: [Xen-devel] [xen-unstable test] 104131: regressions - FAIL

2017-02-13 Thread Xuquan (Quan Xu)
On February 13, 2017 4:24 PM, Tian, Kevin wrote:
>> From: Tian, Kevin
>> Sent: Monday, February 13, 2017 4:21 PM
>>
>> > From: Jan Beulich [mailto:jbeul...@suse.com]
>> > Sent: Wednesday, February 08, 2017 4:52 PM
>> >
>> > >>> On 08.02.17 at 09:27,  wrote:
>> > > Assumed vCPU is in guest_mode..
>> > > When apicv is enabled, hypervisor calls vmx_deliver_posted_intr(),
>> > > then
>> > > __vmx_deliver_posted_interrupt() to deliver interrupt, but no
>> > > vmexit (also no
>> > > vcpu_kick() )..
>> > > In __vmx_deliver_posted_interrupt(), it is __conditional__ to
>> > > deliver posted interrupt. if posted interrupt is not delivered,
>> > > the posted interrupt is pending until next VM entry -- by PIR to vIRR..
>> > >
>> > > one condition is :
>> > > In __vmx_deliver_posted_interrupt(),  ' if (
>> > > !test_and_set_bit(VCPU_KICK_SOFTIRQ, _pending(cpu))' ..
>> > >
>> > > Specifically, we did verify it by RES interrupt, which is used for
>> > > smp_reschedule_interrupt..
>> > > We even cost more time to deliver RES interrupt than no-apicv in
>average..
>> > >
>> > > If RES interrupt (no. 1) is delivered by posted way (the vcpu is
>> > > still guest_mode).. when tries to deliver next-coming RES
>> > > interrupt (no. 2) by posted way, The next-coming RES interrupt
>> > > (no. 2) is not delivered, as we set the VCPU_KICK_SOFTIRQ bit when
>> > > we deliver RES interrupt (no. 1)..
>> > >
>> > > Then the next-coming RES interrupt (no. 2) is pending until next
>> > > VM entry -- by PIR to vIRR..
>> > >
>> > >
>> > > We can fix it as below(I don't think this is a best one, it is
>> > > better to set the VCPU_KICK_SOFTIRQ bit, but not test it):
>> > >
>> > > --- a/xen/arch/x86/hvm/vmx/vmx.c
>> > > +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> > > @@ -1846,7 +1846,7 @@ static void
>__vmx_deliver_posted_interrupt(struct vcpu *v)
>> > >  {
>> > >  unsigned int cpu = v->processor;
>> > >
>> > > -if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ,
>_pending(cpu))
>> > > +if ( !test_bit(VCPU_KICK_SOFTIRQ, _pending(cpu))
>> > >   && (cpu != smp_processor_id()) )
>> > >  send_IPI_mask(cpumask_of(cpu), posted_intr_vector);
>> > >  }
>> >
>> > While I don't think I fully understand your description, the line
>> > you change here has always been puzzling me: If we were to raise a
>> > softirq here, we ought to call cpu_raise_softirq() instead of partly
>> > open coding what it does.
>>
>> We require posted_intr_vector for target CPU to ack/deliver virtual
>> interrupt in non-root mode. cpu_raise_softirq uses a different vector,
>> which cannot trigger such effect.
>>
>> > So I think not marking that softirq
>> > pending (but doing this incompletely) is a valid change in any case.
>> > But I'll have to defer to Kevin in the hopes that he fully
>> > understands what you explain above as well as him knowing why this
>> > was a test-and-set here in the first place.
>> >
>>
>> I agree we have a misuse of softirq mechanism here. If guest is
>> already in non-root mode, the 1st posted interrupt will be directly
>> delivered to guest (leaving softirq being set w/o actually incurring a
>> VM-exit - breaking desired softirq behavior). Then further posted
>> interrupts will skip the IPI, stay in PIR and not noted until another
>> VM-exit happens. Looks Quan observes such delay of delivery in his
>> experiments.
>>
>> I'm OK to remove the set here. Actually since it's an optimization for
>> less IPIs, we'd better check softirq_pending(cpu) directly instead of
>> sticking to one bit only.
>>
>
>sent too fast... Quan, can you work out a patch following this suggestion
>and see whether your slow-delivery issue is solved?
>(hope I understand your issue correctly here).
>

Cool, Very Correct!! 
Sure, I will send out a patch in coming days..

Quan










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


Re: [Xen-devel] [PATCH 05/28] ARM: GICv3 ITS: map ITS command buffer

2017-02-13 Thread Stefano Stabellini
On Mon, 30 Jan 2017, Andre Przywara wrote:
> Instead of directly manipulating the tables in memory, an ITS driver
> sends commands via a ring buffer to the ITS h/w to create or alter the
> LPI mappings.
> Allocate memory for that buffer and tell the ITS about it to be able
> to send ITS commands.
> 
> Signed-off-by: Andre Przywara 
> ---
>  xen/arch/arm/gic-v3-its.c| 46 
> 
>  xen/include/asm-arm/gic_v3_its.h |  6 ++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index c31fef6..ad7cd2a 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -27,6 +27,8 @@
>  #include 
>  #include 
>  
> +#define ITS_CMD_QUEUE_SZSZ_64K
> +
>  #define BASER_ATTR_MASK   \
>  ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)   | \
>   (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \
> @@ -44,6 +46,45 @@ static uint64_t encode_phys_addr(paddr_t addr, int 
> page_bits)
>  return ret | (addr & GENMASK(51, 48)) >> (48 - 12);
>  }
>  
> +static void *its_map_cbaser(struct host_its *its)
> +{
> +void __iomem *cbasereg = its->its_base + GITS_CBASER;
> +uint64_t reg, regc;
> +void *buffer;
> +paddr_t paddr;
> +
> +reg  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
> +reg |= GIC_BASER_CACHE_SameAsInner << 
> GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> +reg |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> +
> +buffer = _xzalloc(ITS_CMD_QUEUE_SZ, PAGE_SIZE);
> +if ( !buffer )
> +return NULL;
> +paddr = virt_to_maddr(buffer);
> +ASSERT(!(paddr & ~GENMASK(51, 12)));
> +
> +reg |= GITS_VALID_BIT | paddr;
> +reg |= ((ITS_CMD_QUEUE_SZ / PAGE_SIZE) - 1) & GITS_CBASER_SIZE_MASK;
> +writeq_relaxed(reg, cbasereg);
> +regc = readq_relaxed(cbasereg);
> +
> +/* If the ITS dropped shareability, drop cacheability as well. */
> +if ( (regc & GITS_BASER_SHAREABILITY_MASK) == 0 )
> +{
> +regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK;
> +writeq_relaxed(regc, cbasereg);
> +}
> +
> +/*
> + * If the command queue memory is mapped as uncached, we need to flush
> + * it on every access.
> + */
> +if ( !(regc & GITS_BASER_INNER_CACHEABILITY_MASK) )
> +its->flags |= HOST_ITS_FLUSH_CMD_QUEUE;
> +
> +return buffer;
> +}
> +
>  #define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT)
>  
>  static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items)
> @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its)
>  }
>  }
>  
> +hw_its->cmd_buf = its_map_cbaser(hw_its);
> +if ( !hw_its->cmd_buf )
> +return -ENOMEM;
> +writeq_relaxed(0, hw_its->its_base + GITS_CWRITER);
 
Why this new write?

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


Re: [Xen-devel] [PATCH 04/28] ARM: GICv3 ITS: allocate device and collection table

2017-02-13 Thread Stefano Stabellini
On Mon, 6 Feb 2017, Julien Grall wrote:
> > +static uint64_t encode_phys_addr(paddr_t addr, int page_bits)
> > +{
> > +uint64_t ret;
> > +
> > +if ( page_bits < 16 )
> > +return (uint64_t)addr & GENMASK(47, page_bits);
> > +
> > +ret = addr & GENMASK(47, 16);
> > +return ret | (addr & GENMASK(51, 48)) >> (48 - 12);
> > +}
> > +
> > +#define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT)
> 
> I know that PAGE_SHIFT has been suggested by Stefano on the previous version.
> However, I think  this is wrong. The PAGE_BITS is not based on the page
> granularity of Xen, so I would much prefer to keep an 12 hardcoded with a
> comment.

OK

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


Re: [Xen-devel] [PATCH 04/28] ARM: GICv3 ITS: allocate device and collection table

2017-02-13 Thread Stefano Stabellini
On Mon, 30 Jan 2017, Andre Przywara wrote:
> Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and
> an EventID (the MSI payload or interrupt ID) to a pair of LPI number
> and collection ID, which points to the target CPU.
> This mapping is stored in the device and collection tables, which software
> has to provide for the ITS to use.
> Allocate the required memory and hand it the ITS.
> The maximum number of devices is limited to a compile-time constant
> exposed in Kconfig.
> 
> Signed-off-by: Andre Przywara 
> ---
>  xen/arch/arm/Kconfig |  14 +
>  xen/arch/arm/gic-v3-its.c| 129 
> +++
>  xen/arch/arm/gic-v3.c|   5 ++
>  xen/include/asm-arm/gic_v3_its.h |  55 -
>  4 files changed, 202 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 71734a1..81bc233 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -64,6 +64,20 @@ config MAX_PHYS_LPI_BITS
>This can be overriden on the command line with the max_lpi_bits
>parameter.
>  
> +config MAX_PHYS_ITS_DEVICE_BITS
> +depends on HAS_ITS
> +int "Number of device bits the ITS supports"
> +range 1 32
> +default "10"
> +help
> +  Specifies the maximum number of devices which want to use the ITS.
> +  Xen needs to allocates memory for the whole range very early.
> +  The allocation scheme may be sparse, so a much larger number must
> +  be supported to cover devices with a high bus number or those on
> +  separate bus segments.
> +  This can be overriden on the command line with the 
> max_its_device_bits
> +  parameter.
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index ff0f571..c31fef6 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -20,9 +20,138 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#define BASER_ATTR_MASK   \
> +((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)   | \
> + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \
> + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT))
> +#define BASER_RO_MASK   (GENMASK(58, 56) | GENMASK(52, 48))
> +
> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits)
> +{
> +uint64_t ret;
> +
> +if ( page_bits < 16 )
> +return (uint64_t)addr & GENMASK(47, page_bits);
> +
> +ret = addr & GENMASK(47, 16);
> +return ret | (addr & GENMASK(51, 48)) >> (48 - 12);
> +}
> +
> +#define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT)
> +
> +static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items)
> +{
> +uint64_t attr, reg;
> +int entry_size = ((regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1;
> +int pagesz = 0, order, table_size;
> +void *buffer = NULL;
> +
> +attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
> +attr |= GIC_BASER_CACHE_SameAsInner << 
> GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> +attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> +
> +/*
> + * Setup the BASE register with the attributes that we like. Then read
> + * it back and see what sticks (page size, cacheability and shareability
> + * attributes), retrying if necessary.
> + */
> +while ( 1 )
> +{
> +table_size = ROUNDUP(nr_items * entry_size, BIT(PAGE_BITS(pagesz)));
> +order = get_order_from_bytes(table_size);
> +
> +if ( !buffer )
> +buffer = alloc_xenheap_pages(order, 0);
> +if ( !buffer )
> +return -ENOMEM;
> +
> +reg  = attr;
> +reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT);
> +reg |= table_size >> PAGE_BITS(pagesz);
> +reg |= regc & BASER_RO_MASK;
> +reg |= GITS_VALID_BIT;
> +reg |= encode_phys_addr(virt_to_maddr(buffer), PAGE_BITS(pagesz));
> +
> +writeq_relaxed(reg, basereg);
> +regc = readl_relaxed(basereg);
> +
> +/* The host didn't like our attributes, just use what it returned. */
> +if ( (regc & BASER_ATTR_MASK) != attr )
> +{
> +/* If we can't map it shareable, drop cacheability as well. */
> +if ( (regc & GITS_BASER_SHAREABILITY_MASK) == 
> GIC_BASER_NonShareable )
> +{
> +regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK;
> +attr = regc & BASER_ATTR_MASK;
> +continue;

Why do we continue at this point? Shouldn't we go ahead to check if the
page size was accepted?


> +}
> +attr = regc & BASER_ATTR_MASK;
> +}
> +
> +/* If the host accepted our page size, we are 

Re: [Xen-devel] [PATCH 03/28] ARM: GICv3: allocate LPI pending and property table

2017-02-13 Thread Stefano Stabellini
On Mon, 30 Jan 2017, Andre Przywara wrote:
> The ARM GICv3 provides a new kind of interrupt called LPIs.
> The pending bits and the configuration data (priority, enable bits) for
> those LPIs are stored in tables in normal memory, which software has to
> provide to the hardware.
> Allocate the required memory, initialize it and hand it over to each
> redistributor. The maximum number of LPIs to be used can be adjusted with
> the command line option "max_lpi_bits", which defaults to a compile time
> constant exposed in Kconfig.
> 
> Signed-off-by: Andre Przywara 
> ---
>  xen/arch/arm/Kconfig  |  15 +
>  xen/arch/arm/Makefile |   1 +
>  xen/arch/arm/gic-v3-lpi.c | 129 
> ++
>  xen/arch/arm/gic-v3.c |  44 +
>  xen/include/asm-arm/bitops.h  |   1 +
>  xen/include/asm-arm/gic.h |   2 +
>  xen/include/asm-arm/gic_v3_defs.h |  52 ++-
>  xen/include/asm-arm/gic_v3_its.h  |  22 ++-
>  8 files changed, 264 insertions(+), 2 deletions(-)
>  create mode 100644 xen/arch/arm/gic-v3-lpi.c
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index bf64c61..71734a1 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -49,6 +49,21 @@ config HAS_ITS
>  bool "GICv3 ITS MSI controller support"
>  depends on HAS_GICV3
>  
> +config MAX_PHYS_LPI_BITS
> +depends on HAS_ITS
> +int "Maximum bits for GICv3 host LPIs (14-32)"
> +range 14 32
> +default "20"
> +help
> +  Specifies the maximum number of LPIs (in bits) Xen should take
> +  care of. The host ITS may provide support for a very large number
> +  of supported LPIs, for all of which we may not want to allocate
> +  memory, so this number here allows to limit this.
> +  Xen itself does not know how many LPIs domains will ever need
> +  beforehand.
> +  This can be overriden on the command line with the max_lpi_bits
> +  parameter.
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 5f4ff23..4ccf2eb 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -19,6 +19,7 @@ obj-y += gic.o
>  obj-y += gic-v2.o
>  obj-$(CONFIG_HAS_GICV3) += gic-v3.o
>  obj-$(CONFIG_HAS_ITS) += gic-v3-its.o
> +obj-$(CONFIG_HAS_ITS) += gic-v3-lpi.o
>  obj-y += guestcopy.o
>  obj-y += hvm.o
>  obj-y += io.o
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> new file mode 100644
> index 000..e2fc901
> --- /dev/null
> +++ b/xen/arch/arm/gic-v3-lpi.c
> @@ -0,0 +1,129 @@
> +/*
> + * xen/arch/arm/gic-v3-lpi.c
> + *
> + * ARM GICv3 Locality-specific Peripheral Interrupts (LPI) support
> + *
> + * Copyright (C) 2016,2017 - ARM Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Global state */
> +static struct {
> +uint8_t *lpi_property;
> +unsigned int host_lpi_bits;

It's still missing a comment


> +} lpi_data;
> +
> +/* Pending table for each redistributor */
> +static DEFINE_PER_CPU(void *, pending_table);
> +
> +#define MAX_PHYS_LPIS   (BIT_ULL(lpi_data.host_lpi_bits) - LPI_OFFSET)
> +
> +uint64_t gicv3_lpi_allocate_pendtable(void)
> +{
> +uint64_t reg;
> +void *pendtable;
> +
> +reg  = GIC_BASER_CACHE_RaWaWb << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
> +reg |= GIC_BASER_CACHE_SameAsInner << 
> GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
> +reg |= GIC_BASER_InnerShareable << GICR_PENDBASER_SHAREABILITY_SHIFT;
> +
> +if ( !this_cpu(pending_table) )
> +{
> +/*
> + * The pending table holds one bit per LPI and even covers bits for
> + * interrupt IDs below 8192, so we allocate the full range.
> + * The GICv3 imposes a 64KB alignment requirement.
> + */
> +pendtable = _xmalloc(BIT_ULL(lpi_data.host_lpi_bits) / 8, SZ_64K);
> +if ( !pendtable )
> +return 0;
> +
> +memset(pendtable, 0, BIT_ULL(lpi_data.host_lpi_bits) / 8);
> +__flush_dcache_area(pendtable, BIT_ULL(lpi_data.host_lpi_bits) / 8);
> +
> +this_cpu(pending_table) = pendtable;
> +}
> +else
> +{
> +pendtable = this_cpu(pending_table);

it's still missing a BUG_ON


> +}
> +
> +reg |= 

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

2017-02-13 Thread osstest service owner
flight 105775 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105775/

Regressions :-(

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

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 105279
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 105279
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 105279
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 105279
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 105279
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 105279

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   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-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  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
 build-arm64-xsm   5 xen-buildfail   never pass
 build-arm64   5 xen-buildfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  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-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-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-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu305e6c8a2ff7a6e3f4942b50e853230f18eeb5a9
baseline version:
 qemuu6fe791b5e3aca8a6de8a322e85e76d2f13338a7e

Last test of basis   105279  2017-02-02 10:18:27 Z   11 days
Failing since105294  2017-02-02 16:42:47 Z   11 days   78 attempts
Testing same since   105775  2017-02-13 19:18:39 Z0 days1 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alex Williamson 
  Alexander Graf 
  Alexander Indenbaum 
  Andrea Bolognani 
  Anthony PERARD 
  Avinesh Kumar 
  Bharata B Rao 
  Cao jin 
  Christian Borntraeger 

Re: [Xen-devel] [PATCH v4 2/2] arm: proper ordering for correct execution of gic_update_one_lr and vgic_store_itargetsr

2017-02-13 Thread Stefano Stabellini
On Fri, 10 Feb 2017, Stefano Stabellini wrote:
> Concurrent execution of gic_update_one_lr and vgic_store_itargetsr can
> result in the wrong pcpu being set as irq target, see
> http://marc.info/?l=xen-devel=148218667104072.
> 
> To solve the issue, add barriers, remove an irq from the inflight
> queue, only after the affinity has been set. On the other end, write the
> new vcpu target, before checking GIC_IRQ_GUEST_MIGRATING and inflight.
> 
> Signed-off-by: Stefano Stabellini 

For reference, with this patch, gic_update_one_lr and vgic_store_itargetsr
can still run simultaneously on two cpus without any lock protections. This
scenario can happen:


  CPU0: gic_update_one_lr   CPU1: vgic_store_itargetsr
  --
  clear GIC_IRQ_GUEST_MIGRATING
set rank->vcpu (final)
vgic_migrate_irq
  if (inflight) set 
GIC_IRQ_GUEST_MIGRATING
  read rank->vcpu (final)
  irq_set_affinity (final)
  remove from inflight

At this point, the affinity is correctly set to the right pcpu and
everything else is OK too, except that GIC_IRQ_GUEST_MIGRATING is still
set, while inflight has been cleared.

What does that mean? Next time gic_update_one_lr is called again
cleaning up this interrupt, the GIC_IRQ_GUEST_MIGRATING path is taken
again, even though it's not necessary. irq_set_affinity is called one
more time, but should be harmless. However, GIC_IRQ_GUEST_QUEUED is
ignored, but I think that's acceptable: it is only set at this stage if
a second interrupt came through while the first one was active (not
possible for level interrupts) and the vcpu wasn't current. In that
case, the second interrupt will be lost, but next time we get an
interrupt it will be injected as usual.

If that's not acceptable, we can avoid this scenario by using this patch

http://marc.info/?l=xen-devel=148237295920492


> ---
>  xen/arch/arm/gic.c | 3 ++-
>  xen/arch/arm/vgic-v2.c | 4 ++--
>  xen/arch/arm/vgic-v3.c | 4 +++-
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index a5348f2..bb52959 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -503,12 +503,13 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>   !test_bit(GIC_IRQ_GUEST_MIGRATING, >status) )
>  gic_raise_guest_irq(v, irq, p->priority);
>  else {
> -list_del_init(>inflight);
>  if ( test_and_clear_bit(GIC_IRQ_GUEST_MIGRATING, >status) )
>  {
>  struct vcpu *v_target = vgic_get_target_vcpu(v, irq);
>  irq_set_affinity(p->desc, cpumask_of(v_target->processor));
>  }
> +smp_mb();
> +list_del_init(>inflight);
>  }
>  }
>  }
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index b30379e..f47286e 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -153,6 +153,8 @@ static void vgic_store_itargetsr(struct domain *d, struct 
> vgic_irq_rank *rank,
>  new_target--;
>  
>  old_target = read_atomic(>vcpu[offset]);
> +write_atomic(>vcpu[offset], new_target);
> +smp_mb();
>  
>  /* Only migrate the vIRQ if the target vCPU has changed */
>  if ( new_target != old_target )
> @@ -161,8 +163,6 @@ static void vgic_store_itargetsr(struct domain *d, struct 
> vgic_irq_rank *rank,
>   d->vcpu[new_target],
>   virq);
>  }
> -
> -write_atomic(>vcpu[offset], new_target);
>  }
>  }
>  
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 7dc9b6f..e82 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -150,11 +150,13 @@ static void vgic_store_irouter(struct domain *d, struct 
> vgic_irq_rank *rank,
>  if ( !new_vcpu )
>  return;
>  
> +write_atomic(>vcpu[offset], new_vcpu->vcpu_id);
> +smp_mb();
> +
>  /* Only migrate the IRQ if the target vCPU has changed */
>  if ( new_vcpu != old_vcpu )
>  vgic_migrate_irq(old_vcpu, new_vcpu, virq);
>  
> -write_atomic(>vcpu[offset], new_vcpu->vcpu_id);
>  }
>  
>  static inline bool vgic_reg64_check_access(struct hsr_dabt dabt)
> -- 
> 1.9.1
> 

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


Re: [Xen-devel] [RFC v2 6/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node

2017-02-13 Thread Stefano Stabellini
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Stop blacklisting ZynqMP's power management node.
> This is now possible since we allow the hardware domain to
> issue HVC/SMC calls to firmware.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


>  xen/arch/arm/platforms/xilinx-zynqmp.c | 8 
>  1 file changed, 8 deletions(-)
> 
> diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c 
> b/xen/arch/arm/platforms/xilinx-zynqmp.c
> index d826282..a2128e2 100644
> --- a/xen/arch/arm/platforms/xilinx-zynqmp.c
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
> @@ -26,13 +26,6 @@ static const char * const zynqmp_dt_compat[] __initconst =
>  NULL
>  };
>  
> -static const struct dt_device_match zynqmp_blacklist_dev[] __initconst =
> -{
> -/* Power management is not yet supported.  */
> -DT_MATCH_COMPATIBLE("xlnx,zynqmp-pm"),
> -{ /* sentinel */ },
> -};
> -
>  bool zynqmp_hvc(struct cpu_user_regs *regs)
>  {
>  register_t ret[4] = { 0 };
> @@ -52,7 +45,6 @@ bool zynqmp_hvc(struct cpu_user_regs *regs)
>  PLATFORM_START(xgene_storm, "Xilinx ZynqMP")
>  .compatible = zynqmp_dt_compat,
>  .hvc = zynqmp_hvc,
> -.blacklist_dev = zynqmp_blacklist_dev,
>  PLATFORM_END
>  
>  /*
> -- 
> 2.7.4
> 

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


Re: [Xen-devel] [RFC v2 5/6] xen/arm: zynqmp: Forward plaform specific firmware calls

2017-02-13 Thread Stefano Stabellini
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Implement an EEMI mediator and forward platform specific
> firmware calls from guests to firmware.
> 
> The EEMI mediator is responsible for implementing access
> controls modifying or blocking calls that try to operate
> on setup for devices that are not under the calling guest's
> control.
> 
> EEMI:
> https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
> 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  xen/arch/arm/platforms/Makefile|   1 +
>  xen/arch/arm/platforms/xilinx-zynqmp-eemi.c| 794 
> +
>  xen/arch/arm/platforms/xilinx-zynqmp.c |  18 +
>  xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 337 +
>  xen/include/asm-arm/platforms/xilinx-zynqmp-mm.h   | 284 
>  5 files changed, 1434 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
>  create mode 100644 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
>  create mode 100644 xen/include/asm-arm/platforms/xilinx-zynqmp-mm.h
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 49fa683..b58b71f 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_ARM_64) += seattle.o
>  obj-$(CONFIG_ARM_32) += sunxi.o
>  obj-$(CONFIG_ARM_64) += xgene-storm.o
>  obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> +obj-$(CONFIG_ARM_64) += xilinx-zynqmp-eemi.o
> diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c 
> b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> new file mode 100644
> index 000..c2c184d
> --- /dev/null
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> @@ -0,0 +1,794 @@
> +/*
> + * xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> + *
> + * Xilinx ZynqMP EEMI API mediator.
> + *
> + * Copyright (c) 2017 Xilinx Inc.
> + * Written by Edgar E. Iglesias 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/*
> + * Mediator for the EEMI Power Mangement API.
> + *
> + * Refs:
> + * 
> https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
> + *
> + * Background:
> + * The ZynqMP has a subsystem named the PMU with a CPU and special devices
> + * dedicated to running Power Management Firmware. Other masters in the
> + * system need to send requests to the PMU in order to for example:
> + * * Manage power state
> + * * Configure clocks
> + * * Program bitstreams for the programmable logic
> + * * etc
> + *
> + * Allthough the details of the setup are configurable, in the common case
> + * the PMU lives in the Secure world. NS World cannot directly communicate
> + * with it and must use proxy services from ARM Trusted Firmware to reach
> + * the PMU.
> + *
> + * Power Management on the ZynqMP is implemented in a layered manner.
> + * The PMU knows about various masters and will enforce access controls
> + * based on a pre-configured partitioning. This configuration dictates
> + * which devices are owned by the various masters and the PMU FW makes sure
> + * that a given master cannot turn off a device that it does not own or that
> + * is in use by other masters.
> + *
> + * The PMU is not aware of multiple execution states in masters.
> + * For example, it treats the ARMv8 cores as single units and does not
> + * distinguish between Secure vs NS OS:s nor does it know about Hypervisors
> + * and multiple guests. It is up to software on the ARMv8 cores to present
> + * a unified view of its power requirements.
> + *
> + * To implement this unified view, ARM Trusted Firmware at EL3 provides
> + * access to the PM API via SMC calls. ARM Trusted Firmware is responsible
> + * for mediating between the Secure and the NS world, rejecting SMC calls
> + * that request changes that are not allowed.
> + *
> + * Xen running above ATF owns the NS world and is responsible for presenting
> + * unified PM requests taking all guests and the hypervisor into account.
> + *
> + * Implementation:
> + * The PM API contains different classes of calls.
> + * Certain calls are harmless to expose to any guest.
> + * These include calls to get the PM API Version, or to read out the version
> + * of the chip we're running on.
> + *
> + * Other calls are more problematic. Here're some:
> + * * Power requests for nodes (i.e turn on or off a given device)
> + * * Reset line control (i.e reset a 

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

2017-02-13 Thread osstest service owner
flight 105767 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105767/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt  6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-credit2   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-arndale   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-libvirt-xsm  6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 
guest-localmigrate/x10 fail in 105757 REGR. vs. 59254

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt  5 xen-install  fail in 105757 pass in 105767
 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-localmigrate/x10 fail in 
105757 pass in 105767
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail pass in 105757

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-rtds  9 debian-installfail REGR. vs. 59254
 test-armhf-armhf-libvirt-raw  6 xen-bootfail baseline untested
 test-armhf-armhf-xl-vhd   6 xen-bootfail baseline untested
 test-amd64-amd64-rumprun-amd64 16 rumprun-demo-xenstorels/xenstorels.repeat 
fail baseline untested
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stopfail in 105757 like 59254
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   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-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   5 xen-buildfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail 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-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 linux7089db84e356562f8ba737c29e472cc42d530dbc
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z  585 days
Failing since 59348  2015-07-10 04:24:05 Z  584 days  271 attempts
Testing same since   105757  2017-02-13 03:57:45 Z0 days2 attempts


7580 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  fail
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt

Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread hpa
On February 13, 2017 2:34:01 PM PST, Waiman Long  wrote:
>On 02/13/2017 04:52 PM, Peter Zijlstra wrote:
>> On Mon, Feb 13, 2017 at 03:12:45PM -0500, Waiman Long wrote:
>>> On 02/13/2017 02:42 PM, Waiman Long wrote:
 On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
> On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>> That way we'd end up with something like:
>>
>> asm("
>> push %rdi;
>> movslq %edi, %rdi;
>> movq __per_cpu_offset(,%rdi,8), %rax;
>> cmpb $0, %[offset](%rax);
>> setne %al;
>> pop %rdi;
>> " : : [offset] "i" (((unsigned long)_time) +
>offsetof(struct steal_time, preempted)));
>>
>> And if we could get rid of the sign extend on edi we could avoid
>all the
>> push-pop nonsense, but I'm not sure I see how to do that (then
>again,
>> this asm foo isn't my strongest point).
> Maybe:
>
> movsql %edi, %rax;
> movq __per_cpu_offset(,%rax,8), %rax;
> cmpb $0, %[offset](%rax);
> setne %al;
>
> ?
 Yes, that looks good to me.

 Cheers,
 Longman

>>> Sorry, I am going to take it back. The displacement or offset can
>only
>>> be up to 32-bit. So we will still need to use at least one more
>>> register, I think.
>> I don't think that would be a problem, I very much doubt we declare
>more
>> than 4G worth of per-cpu variables in the kernel.
>>
>> In any case, use "e" or "Z" as constraint (I never quite know when to
>> use which). That are s32 and u32 displacement immediates resp. and
>> should fail compile with a semi-sensible failure if the displacement
>is
>> too big.
>>
>It is the address of _time that will exceed the 32-bit limit.
>
>Cheers,
>Longman

That seems odd in the extreme?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Waiman Long
On 02/13/2017 04:52 PM, Peter Zijlstra wrote:
> On Mon, Feb 13, 2017 at 03:12:45PM -0500, Waiman Long wrote:
>> On 02/13/2017 02:42 PM, Waiman Long wrote:
>>> On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
 On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
> That way we'd end up with something like:
>
> asm("
> push %rdi;
> movslq %edi, %rdi;
> movq __per_cpu_offset(,%rdi,8), %rax;
> cmpb $0, %[offset](%rax);
> setne %al;
> pop %rdi;
> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct 
> steal_time, preempted)));
>
> And if we could get rid of the sign extend on edi we could avoid all the
> push-pop nonsense, but I'm not sure I see how to do that (then again,
> this asm foo isn't my strongest point).
 Maybe:

 movsql %edi, %rax;
 movq __per_cpu_offset(,%rax,8), %rax;
 cmpb $0, %[offset](%rax);
 setne %al;

 ?
>>> Yes, that looks good to me.
>>>
>>> Cheers,
>>> Longman
>>>
>> Sorry, I am going to take it back. The displacement or offset can only
>> be up to 32-bit. So we will still need to use at least one more
>> register, I think.
> I don't think that would be a problem, I very much doubt we declare more
> than 4G worth of per-cpu variables in the kernel.
>
> In any case, use "e" or "Z" as constraint (I never quite know when to
> use which). That are s32 and u32 displacement immediates resp. and
> should fail compile with a semi-sensible failure if the displacement is
> too big.
>
It is the address of _time that will exceed the 32-bit limit.

Cheers,
Longman




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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Peter Zijlstra
On Mon, Feb 13, 2017 at 05:24:36PM -0500, Waiman Long wrote:

> >> movsql %edi, %rax;
> >> movq __per_cpu_offset(,%rax,8), %rax;
> >> cmpb $0, %[offset](%rax);
> >> setne %al;

> I have thought of that too. However, the goal is to eliminate memory
> read/write from/to stack. Eliminating a register sign-extend instruction
> won't help much in term of performance.

Problem here is that all instructions have dependencies, so if you can
get rid of the sign extend mov you kill a bunch of stall cycles (I would
expect).

But yes, peanuts vs the stack load/stores.

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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Waiman Long
On 02/13/2017 03:06 PM, h...@zytor.com wrote:
> On February 13, 2017 2:53:43 AM PST, Peter Zijlstra  
> wrote:
>> On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>>> That way we'd end up with something like:
>>>
>>> asm("
>>> push %rdi;
>>> movslq %edi, %rdi;
>>> movq __per_cpu_offset(,%rdi,8), %rax;
>>> cmpb $0, %[offset](%rax);
>>> setne %al;
>>> pop %rdi;
>>> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct
>> steal_time, preempted)));
>>> And if we could get rid of the sign extend on edi we could avoid all
>> the
>>> push-pop nonsense, but I'm not sure I see how to do that (then again,
>>> this asm foo isn't my strongest point).
>> Maybe:
>>
>> movsql %edi, %rax;
>> movq __per_cpu_offset(,%rax,8), %rax;
>> cmpb $0, %[offset](%rax);
>> setne %al;
>>
>> ?
> We could kill the zero or sign extend by changing the calling interface to 
> pass an unsigned long instead of an int.  It is much more likely that a zero 
> extend is free for the caller than a sign extend.

I have thought of that too. However, the goal is to eliminate memory
read/write from/to stack. Eliminating a register sign-extend instruction
won't help much in term of performance.

Cheers,
Longman


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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Stefano Stabellini
On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> On Mon, Feb 13, 2017 at 2:54 PM, Stefano Stabellini
>  wrote:
> > On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> >> On Mon, Feb 13, 2017 at 2:13 PM, Stefano Stabellini
> >>  wrote:
> >> > On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> >> >> On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  
> >> >> wrote:
> >> >> >
> >> >> >
> >> >> > On 13/02/17 16:59, Tamas K Lengyel wrote:
> >> >> >>
> >> >> >> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
> >> >> >> wrote:
> >> >> >>>
> >> >> >>> Hi Tamas,
> >> >> >>>
> >> >> >>>
> >> >> >>> On 13/02/17 16:20, Tamas K Lengyel wrote:
> >> >> 
> >> >> 
> >> >>  On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
> >> >>   wrote:
> >> >> >
> >> >> >
> >> >> > Hello,
> >> >> >
> >> >> > This e-mail is sort of follow-up to the two threads: [1] (my 
> >> >> > thread
> >> >> > about TEE interaction) and [2] (Edgar's thread regarding handling 
> >> >> > SMC
> >> >> > calls in platform_hvc). I want to discuss more broad topic there.
> >> >> >
> >> >> > Obviously, there are growing number of SMC users and current 
> >> >> > state of
> >> >> > SMC handling in Xen satisfies nobody. My team wants to handle 
> >> >> > SMCs in
> >> >> > secure way, Xilinx wants to forward some calls directly to Secure
> >> >> > Monitor, while allowing to handle other in userspace, etc.
> >> >> >
> >> >> > My proposition is to gather all requirements to SMC (and HVC) 
> >> >> > handling
> >> >> > in one place (e.g. in this mail thread). After we' will have clear
> >> >> > picture of what we want, we will be able to develop some solution,
> >> >> > that will satisfy us all. At least, I hope so :)
> >> >> >
> >> >> > Also I want to remind, that there are ARM document called "SMC 
> >> >> > Calling
> >> >> > Convention" [3]. According to it, any aarch64 hypervisor "must
> >> >> > implement the Standard Secure and Hypervisor Service calls". At 
> >> >> > this
> >> >> > moment XEN does not conform to this.
> >> >> >
> >> >> > So, lets get started with the requirements:
> >> >> > 0. There are no much difference between SMC and HVC handling (at 
> >> >> > least
> >> >> > according to SMCCC).
> >> >> > 1. Hypervisor should at least provide own UUID and version while
> >> >> > called by SMC/HVC
> >> >> > 2. Hypervisor should forward some calls from dom0 directly to 
> >> >> > Secure
> >> >> > Monitor (Xilinx use case)
> >> >> > 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
> >> >> > architecture service calls, etc.
> >> >> > 4. Hypervisor should handle TEE calls in a secure way (e.g. no
> >> >> > untrusted handlers in Dom0 userspace).
> >> >> > 5. Hypervisor should support multiple TEEs (at least at 
> >> >> > compilation
> >> >> > time).
> >> >> > 6. Hypervisor should do this as fast as possible (DRM playback use
> >> >> > case).
> >> >> > 7. All domains (including dom0) should be handled in the same way.
> >> >> > 8. Not all domains will have right to issue certain SMCs.
> >> >> > 9. Hypervisor will issue own SMCs in some cases.
> >> >> 
> >> >> 
> >> >> 
> >> >>  10. Domains on which the monitor privileged call feature is enabled
> >> >>  (which is by default disabled for all domains) should not be able 
> >> >>  to
> >> >>  issue SMCs such that it reaches the firmware directly. Xen should 
> >> >>  not
> >> >>  bounce such calls to the firmware on behalf of the domain. Xen 
> >> >>  should
> >> >>  not alter the state of the domain automatically (ie. incrementing 
> >> >>  PC).
> >> >>  These calls should be exclusively transfered to the monitor 
> >> >>  subscriber
> >> >>  for further processing. HVC calls need not be included in the 
> >> >>  monitor
> >> >>  forwarding as long as the HVC call can be governed by XSM.
> >> >> >>>
> >> >> >>>
> >> >> >>>
> >> >> >>> This should not be a strong requirement. Whilst in your use case 
> >> >> >>> you want
> >> >> >>> to
> >> >> >>> forward all the SMCs to the monitor app, there are use case where 
> >> >> >>> Xen
> >> >> >>> would
> >> >> >>> need to emulate SMCs on the behalf of the guest. For instance see 
> >> >> >>> PSCI
> >> >> >>> (arch/arm/vpsci.c).
> >> >> >>
> >> >> >>
> >> >> >> In my usecases it is a strong requirement. What happens when the
> >> >> >> monitor system is disabled is beyond my concerns - Xen can emulate or
> >> >> >> forward the call as it wishes. But when the monitor application is in
> >> >> >> use - in my usecase - it needs to be in exclusive control. If that
> >> >> >> breaks an in-guest application, that is acceptable in my usecase. As
> >> >> >> soon as there is 

Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Mon, Feb 13, 2017 at 2:54 PM, Stefano Stabellini
 wrote:
> On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
>> On Mon, Feb 13, 2017 at 2:13 PM, Stefano Stabellini
>>  wrote:
>> > On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
>> >> On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  
>> >> wrote:
>> >> >
>> >> >
>> >> > On 13/02/17 16:59, Tamas K Lengyel wrote:
>> >> >>
>> >> >> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi Tamas,
>> >> >>>
>> >> >>>
>> >> >>> On 13/02/17 16:20, Tamas K Lengyel wrote:
>> >> 
>> >> 
>> >>  On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>> >>   wrote:
>> >> >
>> >> >
>> >> > Hello,
>> >> >
>> >> > This e-mail is sort of follow-up to the two threads: [1] (my thread
>> >> > about TEE interaction) and [2] (Edgar's thread regarding handling 
>> >> > SMC
>> >> > calls in platform_hvc). I want to discuss more broad topic there.
>> >> >
>> >> > Obviously, there are growing number of SMC users and current state 
>> >> > of
>> >> > SMC handling in Xen satisfies nobody. My team wants to handle SMCs 
>> >> > in
>> >> > secure way, Xilinx wants to forward some calls directly to Secure
>> >> > Monitor, while allowing to handle other in userspace, etc.
>> >> >
>> >> > My proposition is to gather all requirements to SMC (and HVC) 
>> >> > handling
>> >> > in one place (e.g. in this mail thread). After we' will have clear
>> >> > picture of what we want, we will be able to develop some solution,
>> >> > that will satisfy us all. At least, I hope so :)
>> >> >
>> >> > Also I want to remind, that there are ARM document called "SMC 
>> >> > Calling
>> >> > Convention" [3]. According to it, any aarch64 hypervisor "must
>> >> > implement the Standard Secure and Hypervisor Service calls". At this
>> >> > moment XEN does not conform to this.
>> >> >
>> >> > So, lets get started with the requirements:
>> >> > 0. There are no much difference between SMC and HVC handling (at 
>> >> > least
>> >> > according to SMCCC).
>> >> > 1. Hypervisor should at least provide own UUID and version while
>> >> > called by SMC/HVC
>> >> > 2. Hypervisor should forward some calls from dom0 directly to Secure
>> >> > Monitor (Xilinx use case)
>> >> > 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
>> >> > architecture service calls, etc.
>> >> > 4. Hypervisor should handle TEE calls in a secure way (e.g. no
>> >> > untrusted handlers in Dom0 userspace).
>> >> > 5. Hypervisor should support multiple TEEs (at least at compilation
>> >> > time).
>> >> > 6. Hypervisor should do this as fast as possible (DRM playback use
>> >> > case).
>> >> > 7. All domains (including dom0) should be handled in the same way.
>> >> > 8. Not all domains will have right to issue certain SMCs.
>> >> > 9. Hypervisor will issue own SMCs in some cases.
>> >> 
>> >> 
>> >> 
>> >>  10. Domains on which the monitor privileged call feature is enabled
>> >>  (which is by default disabled for all domains) should not be able to
>> >>  issue SMCs such that it reaches the firmware directly. Xen should not
>> >>  bounce such calls to the firmware on behalf of the domain. Xen should
>> >>  not alter the state of the domain automatically (ie. incrementing 
>> >>  PC).
>> >>  These calls should be exclusively transfered to the monitor 
>> >>  subscriber
>> >>  for further processing. HVC calls need not be included in the monitor
>> >>  forwarding as long as the HVC call can be governed by XSM.
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> This should not be a strong requirement. Whilst in your use case you 
>> >> >>> want
>> >> >>> to
>> >> >>> forward all the SMCs to the monitor app, there are use case where Xen
>> >> >>> would
>> >> >>> need to emulate SMCs on the behalf of the guest. For instance see PSCI
>> >> >>> (arch/arm/vpsci.c).
>> >> >>
>> >> >>
>> >> >> In my usecases it is a strong requirement. What happens when the
>> >> >> monitor system is disabled is beyond my concerns - Xen can emulate or
>> >> >> forward the call as it wishes. But when the monitor application is in
>> >> >> use - in my usecase - it needs to be in exclusive control. If that
>> >> >> breaks an in-guest application, that is acceptable in my usecase. As
>> >> >> soon as there is another usecase that would need to support such an
>> >> >> application while the monitor system is enabled, the monitor system
>> >> >> can be fine-tuned for those needs to allow Xen to emulate. I've said
>> >> >> it many times, I have nothing against doing that, but as I don't need
>> >> >> it I won't be able to spend time implementing it.
>> >
>> > Right, as I wrote in the other thread, I 

Re: [Xen-devel] [RFC v2 4/6] xen/arm: Introduce call_smcc64

2017-02-13 Thread Stefano Stabellini
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Introduce call_smcc64, a helper function to issue 64-bit SMC
> calls that follow the SMC Calling Convention. This includes
> returning up to 4 64-bit values.
> 
> Signed-off-by: Edgar E. Iglesias 

Please write it in assembly following the example of the exiting
call_smc.



>  xen/include/asm-arm/processor.h | 40 
>  1 file changed, 40 insertions(+)
> 
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index afc0e9a..a604f8c 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -706,6 +706,46 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>  int call_smc(register_t function_id, register_t arg0, register_t arg1,
>   register_t arg2);
>  
> +/*
> + * Helper to issue a 64-bit SMC according to the SMC Calling Convention.
> + *
> + * @fid:  Function Identifier
> + * @a0 - a5:  6 arguments
> + * @ret:  Pointer to 4 register_t carrying return values
> + */
> +static inline register_t call_smcc64(register_t fid,
> + register_t a0,
> + register_t a1,
> + register_t a2,
> + register_t a3,
> + register_t a4,
> + register_t a5,
> + register_t *ret)
> +{
> +register register_t x0 asm("x0") = fid;
> +register register_t x1 asm("x1") = a0;
> +register register_t x2 asm("x2") = a1;
> +register register_t x3 asm("x3") = a2;
> +register register_t x4 asm("x4") = a3;
> +register register_t x5 asm("x5") = a4;
> +register register_t x6 asm("x6") = a5;
> +
> +asm volatile ("smc #0\n"
> +  : "+r" (x0), "+r" (x1), "+r" (x2), "+r" (x3),
> +"+r" (x4), "+r" (x5), "+r" (x6)
> +  :
> +  : "x7", "x8", "x9", "x10", "x11", "x12",
> +"x13", "x14", "x15", "x16", "x17" );
> +
> +if (ret) {
> +ret[0] = x0;
> +ret[1] = x1;
> +ret[2] = x2;
> +ret[3] = x3;
> +}
> +return x0;
> +}
> +
>  void do_trap_guest_error(struct cpu_user_regs *regs);
>  
>  #endif /* __ASSEMBLY__ */
> -- 
> 2.7.4
> 

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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread hpa
On February 13, 2017 1:52:20 PM PST, Peter Zijlstra  
wrote:
>On Mon, Feb 13, 2017 at 03:12:45PM -0500, Waiman Long wrote:
>> On 02/13/2017 02:42 PM, Waiman Long wrote:
>> > On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
>> >> On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>> >>> That way we'd end up with something like:
>> >>>
>> >>> asm("
>> >>> push %rdi;
>> >>> movslq %edi, %rdi;
>> >>> movq __per_cpu_offset(,%rdi,8), %rax;
>> >>> cmpb $0, %[offset](%rax);
>> >>> setne %al;
>> >>> pop %rdi;
>> >>> " : : [offset] "i" (((unsigned long)_time) +
>offsetof(struct steal_time, preempted)));
>> >>>
>> >>> And if we could get rid of the sign extend on edi we could avoid
>all the
>> >>> push-pop nonsense, but I'm not sure I see how to do that (then
>again,
>> >>> this asm foo isn't my strongest point).
>> >> Maybe:
>> >>
>> >> movsql %edi, %rax;
>> >> movq __per_cpu_offset(,%rax,8), %rax;
>> >> cmpb $0, %[offset](%rax);
>> >> setne %al;
>> >>
>> >> ?
>> > Yes, that looks good to me.
>> >
>> > Cheers,
>> > Longman
>> >
>> Sorry, I am going to take it back. The displacement or offset can
>only
>> be up to 32-bit. So we will still need to use at least one more
>> register, I think.
>
>I don't think that would be a problem, I very much doubt we declare
>more
>than 4G worth of per-cpu variables in the kernel.
>
>In any case, use "e" or "Z" as constraint (I never quite know when to
>use which). That are s32 and u32 displacement immediates resp. and
>should fail compile with a semi-sensible failure if the displacement is
>too big.

Oh, and unless you are explicitly forcing 32-bit addressing mode, displacements 
are always "e" (or "m" if you let gcc pick the addressing mode.)
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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


Re: [Xen-devel] [RFC v2 2/6] xen/arm: Introduce platform_hvc

2017-02-13 Thread Stefano Stabellini
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Introduce platform_hvc as a way to handle hypercalls that
> Xen does not know about in a platform specific way. This
> is particularly useful for implementing the SiP (SoC
> implementation specific) service calls.
> 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  xen/arch/arm/platform.c| 8 
>  xen/arch/arm/traps.c   | 3 +++
>  xen/include/asm-arm/platform.h | 5 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index 0af6d57..90ea6b8 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -127,6 +127,14 @@ void platform_poweroff(void)
>  platform->poweroff();
>  }
>  
> +bool platform_hvc(struct cpu_user_regs *regs)

This is fine, but we need a different name for it, as it can be used to
handle both HVC and SMC calls. Maybe "firmware_call"?


> +{
> +if ( platform && platform->hvc )
> +return platform->hvc(regs);
> +
> +return false;
> +}
> +
>  bool_t platform_has_quirk(uint32_t quirk)
>  {
>  uint32_t quirks = 0;
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index c5a4d41..33950d9 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -44,6 +44,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "decode.h"
>  #include "vtimer.h"
> @@ -1430,6 +1431,8 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  }
>  break;
>  default:
> +if ( platform_hvc(regs) )
> +return;
>  domain_crash_synchronous();
>  return;
>  }
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index 08010ba..4d51f0a 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -26,6 +26,10 @@ struct platform_desc {
>  void (*reset)(void);
>  /* Platform power-off */
>  void (*poweroff)(void);
> +/* Platform specific HVC handler.
> + * Returns true if the call was handled and false if not.
> + */
> +bool (*hvc)(struct cpu_user_regs *regs);
>  /*
>   * Platform quirks
>   * Defined has a function because a platform can support multiple
> @@ -55,6 +59,7 @@ int platform_cpu_up(int cpu);
>  #endif
>  void platform_reset(void);
>  void platform_poweroff(void);
> +bool platform_hvc(struct cpu_user_regs *regs);
>  bool_t platform_has_quirk(uint32_t quirk);
>  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);

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


Re: [Xen-devel] [RFC v2 1/6] xen/arm: traps: Reorder early overwrite of FID

2017-02-13 Thread Stefano Stabellini
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Move the early setting of PSCI_RESULT_REG to a later stage
> avoiding the early override of the FID that's stored in
> the same register.
> 
> No functional change.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


>  xen/arch/arm/traps.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 92b1d80..c5a4d41 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1335,8 +1335,6 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  {
>  register_t fid = PSCI_ARG(regs,0);
>  
> -/* preloading in case psci_mode_check fails */
> -PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  switch( fid )
>  {
>  case PSCI_cpu_off:
> @@ -1369,6 +1367,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>  case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>  perfc_incr(vpsci_migrate_info_up_cpu);
> +PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  if ( psci_mode_check(current->domain, fid) )
>  PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_up_cpu();
>  break;
> @@ -1385,6 +1384,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  case PSCI_0_2_FN_CPU_ON:
>  case PSCI_0_2_FN64_CPU_ON:
>  perfc_incr(vpsci_cpu_on);
> +PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  if ( psci_mode_check(current->domain, fid) )
>  {
>  register_t vcpuid = PSCI_ARG(regs,1);
> @@ -1397,6 +1397,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  case PSCI_0_2_FN_CPU_SUSPEND:
>  case PSCI_0_2_FN64_CPU_SUSPEND:
>  perfc_incr(vpsci_cpu_suspend);
> +PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  if ( psci_mode_check(current->domain, fid) )
>  {
>  uint32_t pstate = PSCI_ARG32(regs,1);
> @@ -1409,6 +1410,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  case PSCI_0_2_FN_AFFINITY_INFO:
>  case PSCI_0_2_FN64_AFFINITY_INFO:
>  perfc_incr(vpsci_cpu_affinity_info);
> +PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  if ( psci_mode_check(current->domain, fid) )
>  {
>  register_t taff = PSCI_ARG(regs,1);
> @@ -1420,6 +1422,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>  case PSCI_0_2_FN_MIGRATE:
>  case PSCI_0_2_FN64_MIGRATE:
>  perfc_incr(vpsci_cpu_migrate);
> +PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
>  if ( psci_mode_check(current->domain, fid) )
>  {
>  uint32_t tcpu = PSCI_ARG32(regs,1);
> -- 
> 2.7.4
> 

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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Peter Zijlstra
On Mon, Feb 13, 2017 at 12:06:44PM -0800, h...@zytor.com wrote:

> >Maybe:
> >
> >movsql %edi, %rax;
> >movq __per_cpu_offset(,%rax,8), %rax;
> >cmpb $0, %[offset](%rax);
> >setne %al;
> >
> >?
> 
> We could kill the zero or sign extend by changing the calling
> interface to pass an unsigned long instead of an int.  It is much more
> likely that a zero extend is free for the caller than a sign extend.

Right, Boris and me talked about that on IRC. I was wondering if the
argument was u32 if we could assume the top 32 bits are 0 and then use
rdi without prior movzx.

That would allow reducing the thing one more instruction.

Also, PVOP_CALL_ARG#() have an (unsigned long) cast in them that doesn't
make sense. That cast ends up resulting in the calling code doing
explicit sign or zero extends into the full 64bit register for no good
reason.

If one removes that cast things still compile, but I worry something
somehow relies on this weird behaviour and will come apart.

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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Stefano Stabellini
On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> On Mon, Feb 13, 2017 at 2:13 PM, Stefano Stabellini
>  wrote:
> > On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> >> On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  
> >> wrote:
> >> >
> >> >
> >> > On 13/02/17 16:59, Tamas K Lengyel wrote:
> >> >>
> >> >> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
> >> >> wrote:
> >> >>>
> >> >>> Hi Tamas,
> >> >>>
> >> >>>
> >> >>> On 13/02/17 16:20, Tamas K Lengyel wrote:
> >> 
> >> 
> >>  On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
> >>   wrote:
> >> >
> >> >
> >> > Hello,
> >> >
> >> > This e-mail is sort of follow-up to the two threads: [1] (my thread
> >> > about TEE interaction) and [2] (Edgar's thread regarding handling SMC
> >> > calls in platform_hvc). I want to discuss more broad topic there.
> >> >
> >> > Obviously, there are growing number of SMC users and current state of
> >> > SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
> >> > secure way, Xilinx wants to forward some calls directly to Secure
> >> > Monitor, while allowing to handle other in userspace, etc.
> >> >
> >> > My proposition is to gather all requirements to SMC (and HVC) 
> >> > handling
> >> > in one place (e.g. in this mail thread). After we' will have clear
> >> > picture of what we want, we will be able to develop some solution,
> >> > that will satisfy us all. At least, I hope so :)
> >> >
> >> > Also I want to remind, that there are ARM document called "SMC 
> >> > Calling
> >> > Convention" [3]. According to it, any aarch64 hypervisor "must
> >> > implement the Standard Secure and Hypervisor Service calls". At this
> >> > moment XEN does not conform to this.
> >> >
> >> > So, lets get started with the requirements:
> >> > 0. There are no much difference between SMC and HVC handling (at 
> >> > least
> >> > according to SMCCC).
> >> > 1. Hypervisor should at least provide own UUID and version while
> >> > called by SMC/HVC
> >> > 2. Hypervisor should forward some calls from dom0 directly to Secure
> >> > Monitor (Xilinx use case)
> >> > 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
> >> > architecture service calls, etc.
> >> > 4. Hypervisor should handle TEE calls in a secure way (e.g. no
> >> > untrusted handlers in Dom0 userspace).
> >> > 5. Hypervisor should support multiple TEEs (at least at compilation
> >> > time).
> >> > 6. Hypervisor should do this as fast as possible (DRM playback use
> >> > case).
> >> > 7. All domains (including dom0) should be handled in the same way.
> >> > 8. Not all domains will have right to issue certain SMCs.
> >> > 9. Hypervisor will issue own SMCs in some cases.
> >> 
> >> 
> >> 
> >>  10. Domains on which the monitor privileged call feature is enabled
> >>  (which is by default disabled for all domains) should not be able to
> >>  issue SMCs such that it reaches the firmware directly. Xen should not
> >>  bounce such calls to the firmware on behalf of the domain. Xen should
> >>  not alter the state of the domain automatically (ie. incrementing PC).
> >>  These calls should be exclusively transfered to the monitor subscriber
> >>  for further processing. HVC calls need not be included in the monitor
> >>  forwarding as long as the HVC call can be governed by XSM.
> >> >>>
> >> >>>
> >> >>>
> >> >>> This should not be a strong requirement. Whilst in your use case you 
> >> >>> want
> >> >>> to
> >> >>> forward all the SMCs to the monitor app, there are use case where Xen
> >> >>> would
> >> >>> need to emulate SMCs on the behalf of the guest. For instance see PSCI
> >> >>> (arch/arm/vpsci.c).
> >> >>
> >> >>
> >> >> In my usecases it is a strong requirement. What happens when the
> >> >> monitor system is disabled is beyond my concerns - Xen can emulate or
> >> >> forward the call as it wishes. But when the monitor application is in
> >> >> use - in my usecase - it needs to be in exclusive control. If that
> >> >> breaks an in-guest application, that is acceptable in my usecase. As
> >> >> soon as there is another usecase that would need to support such an
> >> >> application while the monitor system is enabled, the monitor system
> >> >> can be fine-tuned for those needs to allow Xen to emulate. I've said
> >> >> it many times, I have nothing against doing that, but as I don't need
> >> >> it I won't be able to spend time implementing it.
> >
> > Right, as I wrote in the other thread, I think we should be able to find
> > a way to satisfy Tamas' requirement as well as all the others. Of
> > course, once you enable the "forward all SMCs to monitor" mode, some of
> > the other requirements might not be met anymore, but that's 

Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

2017-02-13 Thread Stefano Stabellini
On Mon, 13 Feb 2017, Bhupinder Thakur wrote:
> Hi Stefano,
> 
> On 9 February 2017 at 05:40, Stefano Stabellini  
> wrote:
> > On Wed, 8 Feb 2017, Bhupinder Thakur wrote:
> >> Hi Julien,
> >>
> >> On 3 February 2017 at 19:38, Julien Grall  wrote:
> >> > So if I understand correctly, you don't receive anymore output. Correct?
> >> > Have you tried to see whether the pl011 driver is receiving interrupt or
> >> > even Linux calling it.
> >>
> >> The pl011 driver is not receiving the interrupts (I put a
> >> HYPERVISOR_console_io() call inside the interrupt handler and I do not
> >> see the print on the xen console) . When the console is in this state,
> >> if I keep typing the characters on the console then Xen keeps raising
> >> the interrupts (for each character typed) but all those interrupts are
> >> marked as inflight. So it seems that interrupt is not getting cleared
> >> in GIC. Which GIC function can I look at to further debug what is the
> >> issue?
> >>
> >> Earlier, when I used to manually write characters to /dev/ttyAMA0 then
> >> I used to receive the interrupts all fine.
> >
> > DomU doesn't get emulated PL011 interrupts from Xen? Make sure you don't
> > mark the interrupt as a real hardware interrupt by mistake
> > (GICH_V2_LR_HW). At the same time, check that the virtual PL011
> > interrupt has been added correctly to the LRs
> > (xen/arch/arm/gic-v2.c:gicv2_update_lr).
> 
> I verified that the PL011 interrupt is getting marked pending by Xen
> but guest does not receive it. I am not sure if the local interrupt
> delivery is disabled on the guest because of which it may not be
> receiving the interrupts. Is there a way to verify that interrupts are
> indeed disabled at that point. Since the interrupt delivery works fine
> when I manually read/write characters to /dev/ttyAMA0, I think there
> is no issue from the Xen side in terms of raising an interrupt.
> 
> One observation is that pl011_request_port () calls request_mem_region
> () for the MMIO space allocated to pl011. The function returns -EBUSY
> indicating that the memory region is already in use. But when I do cat
> /proc/iomem, I do not see any overlaps. I am using 0x2200 as the
> start of the pl011 MMIO space. I see the same error while booting the
> dom0 kernel also.
> 
> I have attached the guest boot logs when console=ttyAMA0. I have added
> debug prints marked with [DEBUG]. I added debug prints in amba-pl011.c
> and serial_core.c. The output from the console seems to suddenly stop
> once the pl011 port is started. I have not added getty for ttyAMA0 in
> the /etc/inittab file. I believe auto-serial-console should be able to
> attach a getty session to ttyAMA0 automatically (i did try with adding
> a getty command in the inittab file but that did not solve the issue).
> 
> Towards the end of the log, we can see that every time a character is
> typed, Xen raises an interrupt and the characters keep getting
> collected in the IN RING buffer (in_prod keeps incrementing). Later
> interrupts remain in inflight as the earlier interrupt is not cleared.

Let me get this straight: who is printing all those messages we see on
the guest console (xl console guest) up until "Freeing unused kernel
memory"?  Is it the early pl011 driver? Or is it the Xen PV console
driver?

Are you using a DEBUG build of Xen? Only DEBUG builds allow DomUs to
issue HYPERVISOR_console_io hypercalls. You might want to check that
do_console_io in Xen is letting DomU calls through.

Otherwise pl011_int might actually be called but the printk might not
show up on the Xen console.

The interrupt is enabled from Xen point of view, otherwise it would not
be added to the LR. However Linux might still refuse to call the
interrupt handler for some reason. You might want to add a printk in the
Linux generic interrupt handler, probably
kernel/irq/chip.c:handle_fasteoi_irq in your case.

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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Mon, Feb 13, 2017 at 2:13 PM, Stefano Stabellini
 wrote:
> On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
>> On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  wrote:
>> >
>> >
>> > On 13/02/17 16:59, Tamas K Lengyel wrote:
>> >>
>> >> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
>> >> wrote:
>> >>>
>> >>> Hi Tamas,
>> >>>
>> >>>
>> >>> On 13/02/17 16:20, Tamas K Lengyel wrote:
>> 
>> 
>>  On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>>   wrote:
>> >
>> >
>> > Hello,
>> >
>> > This e-mail is sort of follow-up to the two threads: [1] (my thread
>> > about TEE interaction) and [2] (Edgar's thread regarding handling SMC
>> > calls in platform_hvc). I want to discuss more broad topic there.
>> >
>> > Obviously, there are growing number of SMC users and current state of
>> > SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
>> > secure way, Xilinx wants to forward some calls directly to Secure
>> > Monitor, while allowing to handle other in userspace, etc.
>> >
>> > My proposition is to gather all requirements to SMC (and HVC) handling
>> > in one place (e.g. in this mail thread). After we' will have clear
>> > picture of what we want, we will be able to develop some solution,
>> > that will satisfy us all. At least, I hope so :)
>> >
>> > Also I want to remind, that there are ARM document called "SMC Calling
>> > Convention" [3]. According to it, any aarch64 hypervisor "must
>> > implement the Standard Secure and Hypervisor Service calls". At this
>> > moment XEN does not conform to this.
>> >
>> > So, lets get started with the requirements:
>> > 0. There are no much difference between SMC and HVC handling (at least
>> > according to SMCCC).
>> > 1. Hypervisor should at least provide own UUID and version while
>> > called by SMC/HVC
>> > 2. Hypervisor should forward some calls from dom0 directly to Secure
>> > Monitor (Xilinx use case)
>> > 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
>> > architecture service calls, etc.
>> > 4. Hypervisor should handle TEE calls in a secure way (e.g. no
>> > untrusted handlers in Dom0 userspace).
>> > 5. Hypervisor should support multiple TEEs (at least at compilation
>> > time).
>> > 6. Hypervisor should do this as fast as possible (DRM playback use
>> > case).
>> > 7. All domains (including dom0) should be handled in the same way.
>> > 8. Not all domains will have right to issue certain SMCs.
>> > 9. Hypervisor will issue own SMCs in some cases.
>> 
>> 
>> 
>>  10. Domains on which the monitor privileged call feature is enabled
>>  (which is by default disabled for all domains) should not be able to
>>  issue SMCs such that it reaches the firmware directly. Xen should not
>>  bounce such calls to the firmware on behalf of the domain. Xen should
>>  not alter the state of the domain automatically (ie. incrementing PC).
>>  These calls should be exclusively transfered to the monitor subscriber
>>  for further processing. HVC calls need not be included in the monitor
>>  forwarding as long as the HVC call can be governed by XSM.
>> >>>
>> >>>
>> >>>
>> >>> This should not be a strong requirement. Whilst in your use case you want
>> >>> to
>> >>> forward all the SMCs to the monitor app, there are use case where Xen
>> >>> would
>> >>> need to emulate SMCs on the behalf of the guest. For instance see PSCI
>> >>> (arch/arm/vpsci.c).
>> >>
>> >>
>> >> In my usecases it is a strong requirement. What happens when the
>> >> monitor system is disabled is beyond my concerns - Xen can emulate or
>> >> forward the call as it wishes. But when the monitor application is in
>> >> use - in my usecase - it needs to be in exclusive control. If that
>> >> breaks an in-guest application, that is acceptable in my usecase. As
>> >> soon as there is another usecase that would need to support such an
>> >> application while the monitor system is enabled, the monitor system
>> >> can be fine-tuned for those needs to allow Xen to emulate. I've said
>> >> it many times, I have nothing against doing that, but as I don't need
>> >> it I won't be able to spend time implementing it.
>
> Right, as I wrote in the other thread, I think we should be able to find
> a way to satisfy Tamas' requirement as well as all the others. Of
> course, once you enable the "forward all SMCs to monitor" mode, some of
> the other requirements might not be met anymore, but that's acceptable
> because they are for different use-cases.
>
>
>> > Let me remind you that this discussion is not about what you implemented 
>> > but
>> > what is a sensible design to fit everyone. I also never ask you to 
>> > implement
>> > anything.
>> >>
>> >>>
>> >>> Another valid use case is 

Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Stefano Stabellini
On Mon, 13 Feb 2017, Tamas K Lengyel wrote:
> On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  wrote:
> >
> >
> > On 13/02/17 16:59, Tamas K Lengyel wrote:
> >>
> >> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
> >> wrote:
> >>>
> >>> Hi Tamas,
> >>>
> >>>
> >>> On 13/02/17 16:20, Tamas K Lengyel wrote:
> 
> 
>  On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>   wrote:
> >
> >
> > Hello,
> >
> > This e-mail is sort of follow-up to the two threads: [1] (my thread
> > about TEE interaction) and [2] (Edgar's thread regarding handling SMC
> > calls in platform_hvc). I want to discuss more broad topic there.
> >
> > Obviously, there are growing number of SMC users and current state of
> > SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
> > secure way, Xilinx wants to forward some calls directly to Secure
> > Monitor, while allowing to handle other in userspace, etc.
> >
> > My proposition is to gather all requirements to SMC (and HVC) handling
> > in one place (e.g. in this mail thread). After we' will have clear
> > picture of what we want, we will be able to develop some solution,
> > that will satisfy us all. At least, I hope so :)
> >
> > Also I want to remind, that there are ARM document called "SMC Calling
> > Convention" [3]. According to it, any aarch64 hypervisor "must
> > implement the Standard Secure and Hypervisor Service calls". At this
> > moment XEN does not conform to this.
> >
> > So, lets get started with the requirements:
> > 0. There are no much difference between SMC and HVC handling (at least
> > according to SMCCC).
> > 1. Hypervisor should at least provide own UUID and version while
> > called by SMC/HVC
> > 2. Hypervisor should forward some calls from dom0 directly to Secure
> > Monitor (Xilinx use case)
> > 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
> > architecture service calls, etc.
> > 4. Hypervisor should handle TEE calls in a secure way (e.g. no
> > untrusted handlers in Dom0 userspace).
> > 5. Hypervisor should support multiple TEEs (at least at compilation
> > time).
> > 6. Hypervisor should do this as fast as possible (DRM playback use
> > case).
> > 7. All domains (including dom0) should be handled in the same way.
> > 8. Not all domains will have right to issue certain SMCs.
> > 9. Hypervisor will issue own SMCs in some cases.
> 
> 
> 
>  10. Domains on which the monitor privileged call feature is enabled
>  (which is by default disabled for all domains) should not be able to
>  issue SMCs such that it reaches the firmware directly. Xen should not
>  bounce such calls to the firmware on behalf of the domain. Xen should
>  not alter the state of the domain automatically (ie. incrementing PC).
>  These calls should be exclusively transfered to the monitor subscriber
>  for further processing. HVC calls need not be included in the monitor
>  forwarding as long as the HVC call can be governed by XSM.
> >>>
> >>>
> >>>
> >>> This should not be a strong requirement. Whilst in your use case you want
> >>> to
> >>> forward all the SMCs to the monitor app, there are use case where Xen
> >>> would
> >>> need to emulate SMCs on the behalf of the guest. For instance see PSCI
> >>> (arch/arm/vpsci.c).
> >>
> >>
> >> In my usecases it is a strong requirement. What happens when the
> >> monitor system is disabled is beyond my concerns - Xen can emulate or
> >> forward the call as it wishes. But when the monitor application is in
> >> use - in my usecase - it needs to be in exclusive control. If that
> >> breaks an in-guest application, that is acceptable in my usecase. As
> >> soon as there is another usecase that would need to support such an
> >> application while the monitor system is enabled, the monitor system
> >> can be fine-tuned for those needs to allow Xen to emulate. I've said
> >> it many times, I have nothing against doing that, but as I don't need
> >> it I won't be able to spend time implementing it.

Right, as I wrote in the other thread, I think we should be able to find
a way to satisfy Tamas' requirement as well as all the others. Of
course, once you enable the "forward all SMCs to monitor" mode, some of
the other requirements might not be met anymore, but that's acceptable
because they are for different use-cases.


> > Let me remind you that this discussion is not about what you implemented but
> > what is a sensible design to fit everyone. I also never ask you to implement
> > anything.
> >>
> >>>
> >>> Another valid use case is Xen handling power management for device
> >>> assigned
> >>> to the guest and having the monitor app acting as a "Trusted App".
> >>>
> >>> Regarding the HVC call governed by XSM. I think this is the 

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

2017-02-13 Thread osstest service owner
flight 105766 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105766/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail REGR. vs. 105756

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 105756
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 105756
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 105756
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 105756
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 105756
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 105756
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 105756

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   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-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-rtds  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 build-arm64-xsm   5 xen-buildfail   never pass
 build-arm64   5 xen-buildfail   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-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 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-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-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-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass

version targeted for testing:
 xen  ad01a8d7242f56765f10e9e3dbebce786c470345
baseline version:
 xen  6f6d3b10ec8168e2a78cf385d89803397f116397

Last test of basis   105756  2017-02-13 01:58:50 Z0 days
Testing same since   105766  2017-02-13 11:44:46 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  fail
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  

Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Waiman Long
On 02/13/2017 02:42 PM, Waiman Long wrote:
> On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
>> On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>>> That way we'd end up with something like:
>>>
>>> asm("
>>> push %rdi;
>>> movslq %edi, %rdi;
>>> movq __per_cpu_offset(,%rdi,8), %rax;
>>> cmpb $0, %[offset](%rax);
>>> setne %al;
>>> pop %rdi;
>>> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct 
>>> steal_time, preempted)));
>>>
>>> And if we could get rid of the sign extend on edi we could avoid all the
>>> push-pop nonsense, but I'm not sure I see how to do that (then again,
>>> this asm foo isn't my strongest point).
>> Maybe:
>>
>> movsql %edi, %rax;
>> movq __per_cpu_offset(,%rax,8), %rax;
>> cmpb $0, %[offset](%rax);
>> setne %al;
>>
>> ?
> Yes, that looks good to me.
>
> Cheers,
> Longman
>
Sorry, I am going to take it back. The displacement or offset can only
be up to 32-bit. So we will still need to use at least one more
register, I think.

Cheers,
Longman


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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread hpa
On February 13, 2017 2:53:43 AM PST, Peter Zijlstra  
wrote:
>On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>> That way we'd end up with something like:
>> 
>> asm("
>> push %rdi;
>> movslq %edi, %rdi;
>> movq __per_cpu_offset(,%rdi,8), %rax;
>> cmpb $0, %[offset](%rax);
>> setne %al;
>> pop %rdi;
>> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct
>steal_time, preempted)));
>> 
>> And if we could get rid of the sign extend on edi we could avoid all
>the
>> push-pop nonsense, but I'm not sure I see how to do that (then again,
>> this asm foo isn't my strongest point).
>
>Maybe:
>
>movsql %edi, %rax;
>movq __per_cpu_offset(,%rax,8), %rax;
>cmpb $0, %[offset](%rax);
>setne %al;
>
>?

We could kill the zero or sign extend by changing the calling interface to pass 
an unsigned long instead of an int.  It is much more likely that a zero extend 
is free for the caller than a sign extend.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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


Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document

2017-02-13 Thread Stefano Stabellini
On Mon, 13 Feb 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 10/02/17 01:01, Stefano Stabellini wrote:
> > On Fri, 3 Feb 2017, Edgar E. Iglesias wrote:
> > > A possible hack could be to allocate a chunk of DDR dedicated for PCI DMA.
> > > PCI DMA devs could be locked in to only be able to access this mem + MSI
> > > doorbell.
> > > Guests can still screw each other up but at least it becomes harder to
> > > read/write directly from each others OS memory.
> > > It may not be worth the effort though
> > 
> > Actually, we do have the swiotlb in Dom0, which can be used to bounce
> > DMA requests over a buffer that has been previously setup to be DMA safe
> > using an hypercall. That is how the swiotlb is used on x86. On ARM it is
> > used to issue cache flushes via hypercall, but it could be adapted to do
> > both. It would degrade performance, due to the additional memcpy, but it
> > would work, I believe.
> 
> A while ago, Globallogic suggested to use direct memory mapping for the guest
> to allow guest using DMA on platform not supporting SMMU.
> 
> I believe we can use the same trick on platform where SMMUs can not
> distinguish PCI devices.

Yes, that would work, but only on platforms with a very limited number
of guests. However, it might still be a very common use-case on a
platform such as the Zynq MPSoC.

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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Mon, Feb 13, 2017 at 11:06 AM, Julien Grall  wrote:
>
>
> On 13/02/17 16:59, Tamas K Lengyel wrote:
>>
>> On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall 
>> wrote:
>>>
>>> Hi Tamas,
>>>
>>>
>>> On 13/02/17 16:20, Tamas K Lengyel wrote:


 On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
  wrote:
>
>
> Hello,
>
> This e-mail is sort of follow-up to the two threads: [1] (my thread
> about TEE interaction) and [2] (Edgar's thread regarding handling SMC
> calls in platform_hvc). I want to discuss more broad topic there.
>
> Obviously, there are growing number of SMC users and current state of
> SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
> secure way, Xilinx wants to forward some calls directly to Secure
> Monitor, while allowing to handle other in userspace, etc.
>
> My proposition is to gather all requirements to SMC (and HVC) handling
> in one place (e.g. in this mail thread). After we' will have clear
> picture of what we want, we will be able to develop some solution,
> that will satisfy us all. At least, I hope so :)
>
> Also I want to remind, that there are ARM document called "SMC Calling
> Convention" [3]. According to it, any aarch64 hypervisor "must
> implement the Standard Secure and Hypervisor Service calls". At this
> moment XEN does not conform to this.
>
> So, lets get started with the requirements:
> 0. There are no much difference between SMC and HVC handling (at least
> according to SMCCC).
> 1. Hypervisor should at least provide own UUID and version while
> called by SMC/HVC
> 2. Hypervisor should forward some calls from dom0 directly to Secure
> Monitor (Xilinx use case)
> 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
> architecture service calls, etc.
> 4. Hypervisor should handle TEE calls in a secure way (e.g. no
> untrusted handlers in Dom0 userspace).
> 5. Hypervisor should support multiple TEEs (at least at compilation
> time).
> 6. Hypervisor should do this as fast as possible (DRM playback use
> case).
> 7. All domains (including dom0) should be handled in the same way.
> 8. Not all domains will have right to issue certain SMCs.
> 9. Hypervisor will issue own SMCs in some cases.



 10. Domains on which the monitor privileged call feature is enabled
 (which is by default disabled for all domains) should not be able to
 issue SMCs such that it reaches the firmware directly. Xen should not
 bounce such calls to the firmware on behalf of the domain. Xen should
 not alter the state of the domain automatically (ie. incrementing PC).
 These calls should be exclusively transfered to the monitor subscriber
 for further processing. HVC calls need not be included in the monitor
 forwarding as long as the HVC call can be governed by XSM.
>>>
>>>
>>>
>>> This should not be a strong requirement. Whilst in your use case you want
>>> to
>>> forward all the SMCs to the monitor app, there are use case where Xen
>>> would
>>> need to emulate SMCs on the behalf of the guest. For instance see PSCI
>>> (arch/arm/vpsci.c).
>>
>>
>> In my usecases it is a strong requirement. What happens when the
>> monitor system is disabled is beyond my concerns - Xen can emulate or
>> forward the call as it wishes. But when the monitor application is in
>> use - in my usecase - it needs to be in exclusive control. If that
>> breaks an in-guest application, that is acceptable in my usecase. As
>> soon as there is another usecase that would need to support such an
>> application while the monitor system is enabled, the monitor system
>> can be fine-tuned for those needs to allow Xen to emulate. I've said
>> it many times, I have nothing against doing that, but as I don't need
>> it I won't be able to spend time implementing it.
>
>
> Let me remind you that this discussion is not about what you implemented but
> what is a sensible design to fit everyone. I also never ask you to implement
> anything.
>>
>>>
>>> Another valid use case is Xen handling power management for device
>>> assigned
>>> to the guest and having the monitor app acting as a "Trusted App".
>>>
>>> Regarding the HVC call governed by XSM. I think this is the wrong way to
>>> g.
>>> As it was mentioned a couple of time HVC is a valid conduit for Secure
>>> monitor call. You should not deny them on the basis that your monitor app
>>> is
>>> not able to handle it properly. A better way would be to have a list of
>>> Secure Monitor Call (HVC/SMC) that should be forwarded to the monitor
>>> app.
>>
>>
>> I disagree. XSM needs to be in complete control of all hypercalls.
>> Whether denying some of them will break an application or not is not
>> Xen's concern. That is up to me as a user of Xen and XSM. If Xen
>> overrides a 

[Xen-devel] [DOC v5] Xen transport for 9pfs

2017-02-13 Thread Stefano Stabellini
Changes in v5:
- add padding after out_prod
- add "Why ring.h is not needed"
- specify max-ring-page-order >= 1

Changes in v4:
- Backend XenBus Nodes first
- add version negotiation
- remove complex optimization to avoid unnecessary event notifications
- move out indexes to separate cacheline
- many clarifications

Changes in v3:
- clarify a few statements
- rename port- to event-channel-
- use grant_ref_t ref[1] instead of ref[]

Changes in v2:
- fix copy/paste error
- rename ring-ref- to ring-ref
- fix memory barriers
- add "verify prod/cons against local copy"
- add a paragraph on high level design
- add a note on the maximum possible max-ring-page-order value
- add mechanisms to avoid unnecessary evtchn notifications

---

# Xen transport for 9pfs version 1 

## Background

9pfs is a network filesystem protocol developed for Plan 9. 9pfs is very
simple and describes a series of commands and responses. It is
completely independent from the communication channels, in fact many
clients and servers support multiple channels, usually called
"transports". For example the Linux client supports tcp and unix
sockets, fds, virtio and rdma.


### 9pfs protocol

This document won't cover the full 9pfs specification. Please refer to
this [paper] and this [website] for a detailed description of it.
However it is useful to know that each 9pfs request and response has the
following header:

struct header {
uint32_t size;
uint8_t id;
uint16_t tag;
} __attribute__((packed));

0 4  57
+-+--++
|  size   |id|tag |
+-+--++

- *size*
The size of the request or response.

- *id*
The 9pfs request or response operation.

- *tag*
Unique id that identifies a specific request/response pair. It is used
to multiplex operations on a single channel.

It is possible to have multiple requests in-flight at any given time.


## Rationale

This document describes a Xen based transport for 9pfs, in the
traditional PV frontend and backend format. The PV frontend is used by
the client to send commands to the server. The PV backend is used by the
9pfs server to receive commands from clients and send back responses.

The transport protocol supports multiple rings up to the maximum
supported by the backend. The size of every ring is also configurable
and can span multiple pages, up to the maximum supported by the backend
(although it cannot be more than 2MB). The design is to exploit
parallelism at the vCPU level and support multiple outstanding requests
simultaneously.

This document does not cover the 9pfs client/server design or
implementation, only the transport for it.


## Xenstore

The frontend and the backend connect via xenstore to exchange
information. The toolstack creates front and back nodes with state
[XenbusStateInitialising]. The protocol node name is **9pfs**.

Multiple rings are supported for each frontend and backend connection.

### Backend XenBus Nodes

Backend specific properties, written by the backend, read by the
frontend:

versions
 Values: 

 List of comma separated protocol versions supported by the backend.
 For example "1,2,3". Currently the value is just "1", as there is
 only one version. N.B.: this is the version of the Xen trasport
 protocol, not the version of 9pfs supported by the server.

max-rings
 Values: 

 The maximum supported number of rings per frontend.

max-ring-page-order
 Values: 

 The maximum supported size of a memory allocation in units of
 log2n(machine pages), e.g. 1 = 2 pages, 2 == 4 pages, etc. It
 must be at least 1.

Backend configuration nodes, written by the toolstack, read by the
backend:

path
 Values: 

 Host filesystem path to share.

tag
 Values: 

 Alphanumeric tag that identifies the 9pfs share. The client needs
 to know the tag to be able to mount it.

security-model
 Values: "none"

 *none*: files are stored using the same credentials as they are
 created on the guest (no user ownership squash or remap)
 Only "none" is supported in this version of the protocol.

### Frontend XenBus Nodes

version
 Values: 

 Protocol version, chosen among the ones supported by the backend
 (see **versions** under [Backend XenBus Nodes]). Currently the
 value must be "1".

num-rings
 Values: 

 Number of rings. It needs to be lower or equal to max-rings.

event-channel- (event-channel-0, event-channel-1, etc)
 Values: 

 The identifier of the Xen event channel used to signal activity
 in the ring buffer. One for each ring.

ring-ref (ring-ref0, ring-ref1, etc)
 Values: 

 The Xen 

[Xen-devel] [DOC v9] PV Calls protocol design

2017-02-13 Thread Stefano Stabellini
Changes in v9:
- specify max-page-order must be >= 1
- clarifications
- add "Expanding the protocol"
- add padding after out_error
- add "Why ring.h is not needed"

Changes in v8:
- introduce the concept of indexes page
- many clarifications
- add a diagram
- introduce support for multiple versions of the protocol

Changes in v7:
- add a glossary of Xen terms 
- add a paragraph on why Xen was chosen 
- wording improvements
- add links to xenstore documents and headers
- specify that the current version is "1"
- rename max-dataring-page-order to max-page-order
- rename networking-calls to function-calls
- add links to [Data ring] throughout the document
- explain the difference between req_id and id
- mention that future commands larger than 56 bytes will require a
  version increase
- mention that the list of commands is in calling order
- clarify that reuse data rings are found by ref
- rename ENOTSUPP to ENOTSUP
- add padding in struct pvcalls_data_intf for cachelining
- rename pvcalls_ring_queued to pvcalls_ring_unconsumed


Changes in v6:
- add reuse field in release command
- add "networking-calls" backend node on xenstore
- fixed tab/whitespace indentation

Changes in v5:
- clarify text
- rename id to req_id
- rename sockid to id
- move id to request and response specific fields
- add version node to xenstore

Changes in v4:
- rename xensock to pvcalls

Changes in v3:
- add a dummy element to struct xen_xensock_request to make sure the
  size of the struct is the same on both x86_32 and x86_64

Changes in v2:
- add max-dataring-page-order
- add "Publish backend features and transport parameters" to backend
  xenbus workflow
- update new cmd values
- update xen_xensock_request
- add backlog parameter to listen and binary layout
- add description of new data ring format (interface+data)
- modify connect and accept to reflect new data ring format
- add link to POSIX docs
- add error numbers
- add address format section and relevant numeric definitions
- add explicit mention of unimplemented commands
- add protocol node name
- add xenbus shutdown diagram
- add socket operation

---

# PV Calls Protocol version 1

## Glossary

The following is a list of terms and definitions used in the Xen
community. If you are a Xen contributor you can skip this section.

* PV

  Short for paravirtualized.

* Dom0

  First virtual machine that boots. In most configurations Dom0 is
  privileged and has control over hardware devices, such as network
  cards, graphic cards, etc.

* DomU

  Regular unprivileged Xen virtual machine.

* Domain

  A Xen virtual machine. Dom0 and all DomUs are all separate Xen
  domains.

* Guest

  Same as domain: a Xen virtual machine.

* Frontend

  Each DomU has one or more paravirtualized frontend drivers to access
  disks, network, console, graphics, etc. The presence of PV devices is
  advertized on XenStore, a cross domain key-value database. Frontends
  are similar in intent to the virtio drivers in Linux.

* Backend

  A Xen paravirtualized backend typically runs in Dom0 and it is used to
  export disks, network, console, graphics, etcs, to DomUs. Backends can
  live both in kernel space and in userspace. For example xen-blkback
  lives under drivers/block in the Linux kernel and xen_disk lives under
  hw/block in QEMU. Paravirtualized backends are similar in intent to
  virtio device emulators.

* VMX and SVM
  
  On Intel processors, VMX is the CPU flag for VT-x, hardware
  virtualization support. It corresponds to SVM on AMD processors.



## Rationale

PV Calls is a paravirtualized protocol that allows the implementation of
a set of POSIX functions in a different domain. The PV Calls frontend
sends POSIX function calls to the backend, which implements them and
returns a value to the frontend and acts on the function call.

This version of the document covers networking function calls, such as
connect, accept, bind, release, listen, poll, recvmsg and sendmsg; but
the protocol is meant to be easily extended to cover different sets of
calls. Unimplemented commands return ENOTSUP.

PV Calls provide the following benefits:
* full visibility of the guest behavior on the backend domain, allowing
  for inexpensive filtering and manipulation of any guest calls
* excellent performance

Specifically, PV Calls for networking offer these advantages:
* guest networking works out of the box with VPNs, wireless networks and
  any other complex configurations on the host
* guest services listen on ports bound directly to the backend domain IP
  addresses
* localhost becomes a secure host wide network for inter-VMs
  communications


## Design

### Why Xen?

PV Calls are part of an effort to create a secure runtime environment
for containers (Open Containers Initiative images to be precise). PV
Calls are based on Xen, although porting them to other hypervisors is
possible. Xen was chosen because of its security and isolation
properties and because it supports PV guests, a type of virtual machines

Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Waiman Long
On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
> On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
>> That way we'd end up with something like:
>>
>> asm("
>> push %rdi;
>> movslq %edi, %rdi;
>> movq __per_cpu_offset(,%rdi,8), %rax;
>> cmpb $0, %[offset](%rax);
>> setne %al;
>> pop %rdi;
>> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct 
>> steal_time, preempted)));
>>
>> And if we could get rid of the sign extend on edi we could avoid all the
>> push-pop nonsense, but I'm not sure I see how to do that (then again,
>> this asm foo isn't my strongest point).
> Maybe:
>
> movsql %edi, %rax;
> movq __per_cpu_offset(,%rax,8), %rax;
> cmpb $0, %[offset](%rax);
> setne %al;
>
> ?

Yes, that looks good to me.

Cheers,
Longman


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


Re: [Xen-devel] [PATCH v2] x86/paravirt: Don't make vcpu_is_preempted() a callee-save function

2017-02-13 Thread Waiman Long
On 02/13/2017 05:47 AM, Peter Zijlstra wrote:
> On Fri, Feb 10, 2017 at 12:00:43PM -0500, Waiman Long wrote:
>
 +asm(
 +".pushsection .text;"
 +".global __raw_callee_save___kvm_vcpu_is_preempted;"
 +".type __raw_callee_save___kvm_vcpu_is_preempted, @function;"
 +"__raw_callee_save___kvm_vcpu_is_preempted:"
 +FRAME_BEGIN
 +"push %rdi;"
 +"push %rdx;"
 +"movslq  %edi, %rdi;"
 +"movq$steal_time+16, %rax;"
 +"movq__per_cpu_offset(,%rdi,8), %rdx;"
 +"cmpb$0, (%rdx,%rax);"
> Could we not put the $steal_time+16 displacement as an immediate in the
> cmpb and save a whole register here?
>
> That way we'd end up with something like:
>
> asm("
> push %rdi;
> movslq %edi, %rdi;
> movq __per_cpu_offset(,%rdi,8), %rax;
> cmpb $0, %[offset](%rax);
> setne %al;
> pop %rdi;
> " : : [offset] "i" (((unsigned long)_time) + offsetof(struct 
> steal_time, preempted)));
>
> And if we could get rid of the sign extend on edi we could avoid all the
> push-pop nonsense, but I'm not sure I see how to do that (then again,
> this asm foo isn't my strongest point).

Yes, I think that can work. I will try to ran this patch to see how
thing goes.

 +"setne   %al;"
 +"pop %rdx;"
 +"pop %rdi;"
 +FRAME_END
 +"ret;"
 +".popsection");
 +
 +#endif
 +
  /*
   * Setup pv_lock_ops to exploit KVM_FEATURE_PV_UNHALT if present.
   */
>>> That should work for now. I have done something similar for
>>> __pv_queued_spin_unlock. However, this has the problem of creating a
>>> dependency on the exact layout of the steal_time structure. Maybe the
>>> constant 16 can be passed in as a parameter offsetof(struct
>>> kvm_steal_time, preempted) to the asm call.
> Yeah it should be well possible to pass that in. But ideally we'd have
> GCC grow something like __attribute__((callee_saved)) or somesuch and it
> would do all this for us.

That will be really nice too. I am not too fond of working in assembly.

>> One more thing, that will improve KVM performance, but it won't help Xen.
> People still use Xen? ;-) In any case, their implementation looks very
> similar and could easily crib this.

In Red Hat, my focus will be on KVM performance. I do believe that there
are still Xen users out there. So we still need to keep their interest
into consideration. Given that, I am OK to make it work better in KVM
first and then think about Xen later.

>> I looked into the assembly code for rwsem_spin_on_owner, It need to save
>> and restore 2 additional registers with my patch. Doing it your way,
>> will transfer the save and restore overhead to the assembly code.
>> However, __kvm_vcpu_is_preempted() is called multiple times per
>> invocation of rwsem_spin_on_owner. That function is simple enough that
>> making __kvm_vcpu_is_preempted() callee-save won't produce much compiler
>> optimization opportunity.
> This is because of that noinline, right? Otherwise it would've been
> folded and register pressure would be much higher.

Yes, I guess so. The noinline is there so that we know what the CPU time
is for spinning rather than other activities within the slowpath.

>
>> The outer function rwsem_down_write_failed()
>> does appear to be a bit bigger (from 866 bytes to 884 bytes) though.
> I suspect GCC is being clever and since all this is static it plays
> games with the calling convention and pushes these clobbers out.
>
>

Cheers,
Longman


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


Re: [Xen-devel] [PATCH v3 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP

2017-02-13 Thread Paul Durrant

On 13 February 2017, at 19:11, Boris Ostrovsky  
wrote:

>
>
>> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
>> index 5e5c7ae..a33f17e 100644
>> --- a/drivers/xen/privcmd.c
>> +++ b/drivers/xen/privcmd.c
>> @@ -22,6 +22,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include 
>>  #include 
>> @@ -32,6 +33,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -43,6 +45,17 @@ MODULE_LICENSE("GPL");
>>
>>  #define PRIV_VMA_LOCKED ((void *)1)
>>
>> +unsigned int privcmd_dm_op_max_num = 16;
>> +module_param_named(dm_op_max_nr_bufs, privcmd_dm_op_max_num, uint, 0644);
>> +MODULE_PARM_DESC(dm_op_max_nr_bufs,
>> +  "Maximum number of buffers per dm_op hypercall");
>> +
>> +unsigned int privcmd_dm_op_buf_max_size = XEN_PAGE_SIZE;
>These need to be static. (I can fix it when committing.)

OK. Thanks.

>And I am still not sure about using XEN_PAGE_SIZE. There is no
>dependency in the hypervisor on buffers being page-sized, is there? If
>not, XEN_PAGE_SIZE is here just because it happens to be 4K, which is a
>reasonable value.
>How about just setting it to 4096?

I chose XEN_PAGE_SIZE because the hypercall will eventually copy in the buffer 
so it seemed like a reasonable value to use. If you want to just use 4096 then 
I am ok with that.

  Paul

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


Re: [Xen-devel] [PATCH v3 3/3] xen/privcmd: add IOCTL_PRIVCMD_RESTRICT

2017-02-13 Thread Boris Ostrovsky



On 02/13/2017 12:03 PM, Paul Durrant wrote:

The purpose if this ioctl is to allow a user of privcmd to restrict its
operation such that it will no longer service arbitrary hypercalls via
IOCTL_PRIVCMD_HYPERCALL, and will check for a matching domid when
servicing IOCTL_PRIVCMD_DM_OP.


and IOCTL_PRIVCMD_MMAP*.


The aim of this is to limit the attack
surface for a compromised device model.



-boris

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


Re: [Xen-devel] [PATCH v3 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP

2017-02-13 Thread Boris Ostrovsky




diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e5c7ae..a33f17e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -32,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +45,17 @@ MODULE_LICENSE("GPL");

 #define PRIV_VMA_LOCKED ((void *)1)

+unsigned int privcmd_dm_op_max_num = 16;
+module_param_named(dm_op_max_nr_bufs, privcmd_dm_op_max_num, uint, 0644);
+MODULE_PARM_DESC(dm_op_max_nr_bufs,
+"Maximum number of buffers per dm_op hypercall");
+
+unsigned int privcmd_dm_op_buf_max_size = XEN_PAGE_SIZE;


These need to be static. (I can fix it when committing.)

And I am still not sure about using XEN_PAGE_SIZE. There is no 
dependency in the hypervisor on buffers being page-sized, is there? If 
not, XEN_PAGE_SIZE is here just because it happens to be 4K, which is a 
reasonable value.


How about just setting it to 4096?


-boris

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


Re: [Xen-devel] [PATCH v3 1/3] xen/privcmd: return -ENOTTY for unimplemented IOCTLs

2017-02-13 Thread Boris Ostrovsky



On 02/13/2017 12:03 PM, Paul Durrant wrote:

The code sets the default return code to -ENOSYS but then overrides this
to -EINVAL in the switch() statement's default case, which is clearly
silly.

This patch removes the override and sets the default return code to
-ENOTTY, which is the conventional return for an unimplemented ioctl.

Signed-off-by: Paul Durrant 


Applied to for-linus-4.11.

-boris

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


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

2017-02-13 Thread osstest service owner
flight 105770 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105770/

Regressions :-(

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

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
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 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-check(1) blocked n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 

Re: [Xen-devel] [PATCH] x86/VMX: sanitize VM86 TSS handling

2017-02-13 Thread Andrew Cooper
On 13/02/17 13:19, Jan Beulich wrote:
> The present way of setting this up is flawed: Leaving the I/O bitmap
> pointer at zero means that the interrupt redirection bitmap lives
> outside (ahead of) the allocated space of the TSS. Similarly setting a
> TSS limit of 255 when only 128 bytes get allocated means that 128 extra
> bytes may be accessed by the CPU during I/O port access processing.
>
> Introduce a new HVM param to set the allocated size of the TSS, and
> have the hypervisor actually take care of setting namely the I/O bitmap
> pointer. Both this and the segment limit now take the allocated size
> into account.
>
> Signed-off-by: Jan Beulich 
> ---
> TBD: Do we really want to re-init the TSS every time we are about to
>  use it? This can happen quite often during boot, especially while
>  grub is running.

The only case we need worry about is if the guest manually writes to the
area covered by the vm86 tss.  I think, looking at the logic, that we
properly restore the old %tr when re-entering protected mode, so we
aren't at risk of having the vm86 tss on the leaving-side of a hardware
task switch.

We have lasted thus far without, but given the issues recently
identified, the only safe conclusion to be drawn is that this
functionality hasn't been used in anger.

For sensible performance, we need to keep the IO bitmap clear to avoid
taking the #GP emulation path.

For correct behaviour, we need the entire software bitmap to be 0.

As this functionality exists only for first-gen VT-x lacking
unrestricted_guest, I can't claim to care too much about the performance
impact.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2850,6 +2850,43 @@ static int hvm_load_segment_selector(
>  return 1;
>  }
>  
> +struct tss32 {
> +uint16_t back_link, :16;
> +uint32_t esp0;
> +uint16_t ss0, :16;
> +uint32_t esp1;
> +uint16_t ss1, :16;
> +uint32_t esp2;
> +uint16_t ss2, :16;
> +uint32_t cr3, eip, eflags, eax, ecx, edx, ebx, esp, ebp, esi, edi;
> +uint16_t es, :16, cs, :16, ss, :16, ds, :16, fs, :16, gs, :16, ldt, :16;
> +uint16_t trace /* :1 */, iomap;
> +};
> +
> +void hvm_prepare_vm86_tss(struct vcpu *v, uint32_t base, uint32_t limit)
> +{
> +/*
> + * If the provided area is large enough to cover at least the ISA port
> + * range, keep the bitmaps outside the base structure. For rather small
> + * areas (namely relevant for guests having been migrated from older
> + * Xen versions), maximize interrupt vector and port coverage by pointing
> + * the I/O bitmap at 0x20 (which puts the interrupt redirection bitmap
> + * right at zero), accepting accesses to port 0x235 (represented by bit 5
> + * of byte 0x46) to trigger #GP (which will simply result in the access
> + * being handled by the emulator via a slightly different path than it
> + * would be anyway). Be sure to include one extra byte at the end of the
> + * I/O bitmap (hence the missing "- 1" in the comparison is not an
> + * off-by-one mistake), which we deliberately don't fill with all ones.
> + */
> +uint16_t iomap = (limit >= sizeof(struct tss32) + (0x100 / 8) + (0x400 / 
> 8)
> +  ? sizeof(struct tss32) : 0) + (0x100 / 8);
> +
> +ASSERT(limit >= sizeof(struct tss32) - 1);
> +hvm_copy_to_guest_phys(base, NULL, limit + 1, v);
> +hvm_copy_to_guest_phys(base + offsetof(struct tss32, iomap),
> +   , sizeof(iomap), v);

This should be linear rather than phys.

In practice it will only make a difference if the guest manages to
corrupt its IDENT_PT (which is why I suggested that we move the IDENT_PT
entirely outside of the guest control), but in such a case, we will
re-enter the guest with the vm86 tss pointing to something other than
what we have just initialised.

> +}
> +
>  void hvm_task_switch(
>  uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
>  int32_t errcode)
> @@ -2862,18 +2899,7 @@ void hvm_task_switch(
>  unsigned int eflags;
>  pagefault_info_t pfinfo;
>  int exn_raised, rc;
> -struct {
> -u16 back_link,__blh;
> -u32 esp0;
> -u16 ss0, _0;
> -u32 esp1;
> -u16 ss1, _1;
> -u32 esp2;
> -u16 ss2, _2;
> -u32 cr3, eip, eflags, eax, ecx, edx, ebx, esp, ebp, esi, edi;
> -u16 es, _3, cs, _4, ss, _5, ds, _6, fs, _7, gs, _8, ldt, _9;
> -u16 trace, iomap;
> -} tss;
> +struct tss32 tss;
>  
>  hvm_get_segment_register(v, x86_seg_gdtr, );
>  hvm_get_segment_register(v, x86_seg_tr, _tr);
> @@ -4276,6 +4302,7 @@ static int hvm_allow_set_param(struct do
>  /* The following parameters can be set by the guest. */
>  case HVM_PARAM_CALLBACK_IRQ:
>  case HVM_PARAM_VM86_TSS:
> +case HVM_PARAM_VM86_TSS_SIZE:
>  case HVM_PARAM_ACPI_IOPORTS_LOCATION:
>  case HVM_PARAM_VM_GENERATION_ID_ADDR:
>  case 

[Xen-devel] Xen Security Advisory 208 (CVE-2017-2615) - oob access in cirrus bitblt copy

2017-02-13 Thread Xen . org security team
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Xen Security Advisory CVE-2017-2615 / XSA-208
  version 2

   oob access in cirrus bitblt copy

UPDATES IN VERSION 2


Included backport for qemu-xen versions 4.7 (and earlier); fixed
qemu-xen-traditional patch.  Also included proper (non-obscured)
e-mail addresses from upstream patch.

Removed "possibly" from Impact.

3 patches updated

ISSUE DESCRIPTION
=

When doing bitblt copy backwards, qemu should negate the blit width.
This avoids an oob access before the start of video memory.

IMPACT
==

A malicious guest administrator can cause an out of bounds memory
access, leading to information disclosure or privilege escalation.

VULNERABLE SYSTEMS
==

Versions of qemu shipped with all Xen versions are vulnerable.

Xen systems running on x86 with HVM guests, with the qemu process
running in dom0 are vulnerable.

Only guests provided with the "cirrus" emulated video card can exploit
the vulnerability.  The non-default "stdvga" emulated video card is
not vulnerable.  (With xl the emulated video card is controlled by the
"stdvga=" and "vga=" domain configuration options.)

ARM systems are not vulnerable.  Systems using only PV guests are not
vulnerable.

For VMs whose qemu process is running in a stub domain, a successful
attacker will only gain the privileges of that stubdom, which should
be only over the guest itself.

Both upstream-based versions of qemu (device_model_version="qemu-xen")
and `traditional' qemu (device_model_version="qemu-xen-traditional")
are vulnerable.

MITIGATION
==

Running only PV guests will avoid the issue.

Running HVM guests with the device model in a stubdomain will mitigate
the issue.

Changing the video card emulation to stdvga (stdvga=1, vga="stdvga",
in the xl domain configuration) will avoid the vulnerability.

RESOLUTION
==

Applying the appropriate attached patch resolves this issue.

xsa208-qemuu.patch   mainline qemu, qemu-xen master,4.8
xsa208-qemuu-4.7.patch   qemu-xen 4.4, 4.5, 4.6, 4.7
xsa208-qemut.patch   qemu-xen-traditional

$ sha256sum xsa208*
afde3e9d4bf5225f92c36dec9ff673b0b1b0bad4452d406f0c12edc85e2fec72  
xsa208-qemut.patch
e492d528141be5899d46c2ac0bcd0c40ca9d9bfc40906a8e7a565361f17ce38d  
xsa208-qemuu.patch
09471b66c9d9fc5616e7b96ab67bbb51987e7d9520d1b81cb27cbbb168659ad5  
xsa208-qemuu-4.7.patch
$


NOTE REGARDING LACK OF EMBARGO
==

This issue has already been publicly disclosed.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQEcBAEBAgAGBQJYofdiAAoJEIP+FMlX6CvZ3UEIAMJUV177OqZ0O7436zYpM9S+
fEku8b/G7npRcm0L9PtD8PG39IVtqrtIDHIpzMxHA0qbMx3PqWp1G3iBVwFnj21e
ALtKjdNaoDA8nqFEQ3/AbyZ7jn91oYWwmJ7+pKGds+Q+juFof6FVOXCjhNp0XSA6
EDvsz8vOI4fWTtEuVGbg1GnvgEAjKLE9/bE/4zdkWo2WSiWRRCj/yEAr5n0v0R5n
0EEvk21H0XESk2zBk0/UxompNuqbHwOZhBkQ65DxNSkWMIA9hUgqyinR674luHKC
mDkAq8bXar6n1TBQCbWq5f/+50FOApEs0EvJuzWAG7MEkFPaeDSilFb6obhxHjo=
=294C
-END PGP SIGNATURE-


xsa208-qemut.patch
Description: Binary data


xsa208-qemuu.patch
Description: Binary data


xsa208-qemuu-4.7.patch
Description: Binary data
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


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

2017-02-13 Thread osstest service owner
flight 105763 linux-next real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105763/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   5 xen-buildfail REGR. vs. 105753

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-arndale  18 leak-check/checkfail blocked in 105753
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-check fail blocked in 
105753
 test-armhf-armhf-libvirt   13 saverestore-support-check fail blocked in 105753
 test-armhf-armhf-libvirt-raw 12 saverestore-support-check fail blocked in 
105753
 test-armhf-armhf-xl-rtds   15 guest-start/debian.repeat fail blocked in 105753

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
 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-arm64-arm64-libvirt  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-i386-xl-qemut-winxpsp3  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemut-rhel6hvm-intel  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-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-amd64-amd64-xl-qemut-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 build-amd64-rumprun   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 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
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumprun-amd64  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-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qemut-winxpsp3  1 build-check(1)   blocked n/a
 test-arm64-arm64-xl   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-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-amd  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 

Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 18:01,  wrote:
> On 13/02/17 16:49, Jan Beulich wrote:
> On 13.02.17 at 14:03,  wrote:
>>> Sending an invalidation to the device model is an internal detail of
>>> completing the hypercall; callers should not need to be responsible for it.
>>> Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
>>> appropriate.
>>>
>>> This makes the function boolean in nature, although the existing
>>> HVM_HCALL_{completed,preempted} to aid code clarity.
>> "are being retained" missing somewhere here?
> 
> Yes.  I already noticed and fixed that up.  It now reads
> 
> "This makes the function boolean in nature, although the existing
> HVM_HCALL_{completed,preempted} constants are kept to aid code clarity."
> 
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3874,7 +3874,7 @@ static const hypercall_table_t hvm_hypercall_table[] 
>>> = 
>>> {
>>>  #undef HYPERCALL
>>>  #undef COMPAT_CALL
>>>  
>>> -int hvm_do_hypercall(struct cpu_user_regs *regs)
>>> +bool hvm_hypercall(struct cpu_user_regs *regs)
>> I don't think bool is a particularly good choice when the callers can't
>> sensibly use the result without comparing with HVM_HCALL_*
> 
> Ok.  I will keep it as int.

With that
Reviewed-by: Jan Beulich 



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


Re: [Xen-devel] PML causing race condition during guest bootstorm and host crash on Broadwell cpu.

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 17:56,  wrote:
 On 13.02.17 at 17:32,  wrote:
>> On 09/02/17 16:22, Jan Beulich wrote:
>>> Mind giving the attached patch a try (which admittedly was only
>>> lightly tested so far - in particular I haven't seen the second of
>>> the debug messages being logged, yet)?
>> Patch looks promising. I couldn't do much thorough testing, but initial 
>> reboot cycles (around 20 reboots of 32 VMS) went fine.
> 
> Thanks. The most interesting part though is whether the 2nd of the
> two log messages ever showed up. I any event I'll submit the cleaned
> up patch, for more formal discussion of the approach to happen there.

Actually, no, while putting together the commit message I thought
of a second situation which likely would also need addressing: If
we bypass __context_switch() also on the context-switch-in path
(because of scheduling the same vCPU again) the CPU may also
have lost control of the VMCS. There's no context switch hook
though to place the reload invocation in, so I'll first have to think
about what the best model here would be (of course I'd be more
than happy about suggestions).

Jan


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


[Xen-devel] [PATCH v3 3/3] xen/privcmd: add IOCTL_PRIVCMD_RESTRICT

2017-02-13 Thread Paul Durrant
The purpose if this ioctl is to allow a user of privcmd to restrict its
operation such that it will no longer service arbitrary hypercalls via
IOCTL_PRIVCMD_HYPERCALL, and will check for a matching domid when
servicing IOCTL_PRIVCMD_DM_OP. The aim of this is to limit the attack
surface for a compromised device model.

Signed-off-by: Paul Durrant 
---
Cc: Boris Ostrovsky 
Cc: Juergen Gross 

v3:
- Extend restriction to mapping ioctls

v2:
- Make sure that a restriction cannot be cleared
---
 drivers/xen/privcmd.c  | 88 +-
 include/uapi/xen/privcmd.h |  2 ++
 2 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index a33f17e..f50d984 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -56,16 +56,25 @@ module_param_named(dm_op_buf_max_size, 
privcmd_dm_op_buf_max_size, uint,
 MODULE_PARM_DESC(dm_op_buf_max_size,
 "Maximum size of a dm_op hypercall buffer");
 
+struct privcmd_data {
+   domid_t domid;
+};
+
 static int privcmd_vma_range_is_mapped(
struct vm_area_struct *vma,
unsigned long addr,
unsigned long nr_pages);
 
-static long privcmd_ioctl_hypercall(void __user *udata)
+static long privcmd_ioctl_hypercall(struct file *file, void __user *udata)
 {
+   struct privcmd_data *data = file->private_data;
struct privcmd_hypercall hypercall;
long ret;
 
+   /* Disallow arbitrary hypercalls if restricted */
+   if (data->domid != DOMID_INVALID)
+   return -EPERM;
+
if (copy_from_user(, udata, sizeof(hypercall)))
return -EFAULT;
 
@@ -242,8 +251,9 @@ static int mmap_gfn_range(void *data, void *state)
return 0;
 }
 
-static long privcmd_ioctl_mmap(void __user *udata)
+static long privcmd_ioctl_mmap(struct file *file, void __user *udata)
 {
+   struct privcmd_data *data = file->private_data;
struct privcmd_mmap mmapcmd;
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -258,6 +268,10 @@ static long privcmd_ioctl_mmap(void __user *udata)
if (copy_from_user(, udata, sizeof(mmapcmd)))
return -EFAULT;
 
+   /* If restriction is in place, check the domid matches */
+   if (data->domid != DOMID_INVALID && data->domid != mmapcmd.dom)
+   return -EPERM;
+
rc = gather_array(,
  mmapcmd.num, sizeof(struct privcmd_mmap_entry),
  mmapcmd.entry);
@@ -429,8 +443,10 @@ static int alloc_empty_pages(struct vm_area_struct *vma, 
int numpgs)
 
 static const struct vm_operations_struct privcmd_vm_ops;
 
-static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
+static long privcmd_ioctl_mmap_batch(
+   struct file *file, void __user *udata, int version)
 {
+   struct privcmd_data *data = file->private_data;
int ret;
struct privcmd_mmapbatch_v2 m;
struct mm_struct *mm = current->mm;
@@ -459,6 +475,10 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, 
int version)
return -EINVAL;
}
 
+   /* If restriction is in place, check the domid matches */
+   if (data->domid != DOMID_INVALID && data->domid != m.dom)
+   return -EPERM;
+
nr_pages = DIV_ROUND_UP(m.num, XEN_PFN_PER_PAGE);
if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
return -EINVAL;
@@ -603,8 +623,9 @@ static void unlock_pages(struct page *pages[], unsigned int 
nr_pages)
}
 }
 
-static long privcmd_ioctl_dm_op(void __user *udata)
+static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
 {
+   struct privcmd_data *data = file->private_data;
struct privcmd_dm_op kdata;
struct privcmd_dm_op_buf *kbufs;
unsigned int nr_pages = 0;
@@ -616,6 +637,10 @@ static long privcmd_ioctl_dm_op(void __user *udata)
if (copy_from_user(, udata, sizeof(kdata)))
return -EFAULT;
 
+   /* If restriction is in place, check the domid matches */
+   if (data->domid != DOMID_INVALID && data->domid != kdata.dom)
+   return -EPERM;
+
if (kdata.num == 0)
return 0;
 
@@ -683,6 +708,23 @@ static long privcmd_ioctl_dm_op(void __user *udata)
return rc;
 }
 
+static long privcmd_ioctl_restrict(struct file *file, void __user *udata)
+{
+   struct privcmd_data *data = file->private_data;
+   domid_t dom;
+
+   if (copy_from_user(, udata, sizeof(dom)))
+   return -EFAULT;
+
+   /* Set restriction to the specified domain, or check it matches */
+   if (data->domid == DOMID_INVALID)
+   data->domid = dom;
+   else if (data->domid != dom)
+   return -EINVAL;
+
+   return 0;
+}
+
 static long privcmd_ioctl(struct file *file,
  

[Xen-devel] [PATCH v3 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP

2017-02-13 Thread Paul Durrant
Recently a new dm_op[1] hypercall was added to Xen to provide a mechanism
for restricting device emulators (such as QEMU) to a limited set of
hypervisor operations, and being able to audit those operations in the
kernel of the domain in which they run.

This patch adds IOCTL_PRIVCMD_DM_OP as gateway for __HYPERVISOR_dm_op.

NOTE: There is no requirement for user-space code to bounce data through
  locked memory buffers (as with IOCTL_PRIVCMD_HYPERCALL) since
  privcmd has enough information to lock the original buffers
  directly.

[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=524a98c2

Signed-off-by: Paul Durrant 
---
Cc: Boris Ostrovsky 
Cc: Juergen Gross 

v3:
- Add module parameters for max number of dm_op buffers and max buffer
  size
- Fix arm build
- Fix commit comment to reflect re-worked patch

v2:
- Lock the user pages rather than bouncing through kernel memory
---
 arch/arm/xen/enlighten.c |   1 +
 arch/arm/xen/hypercall.S |   1 +
 arch/arm64/xen/hypercall.S   |   1 +
 arch/x86/include/asm/xen/hypercall.h |   7 ++
 drivers/xen/privcmd.c| 139 +++
 include/uapi/xen/privcmd.h   |  13 
 include/xen/arm/hypercall.h  |   1 +
 include/xen/interface/hvm/dm_op.h|  32 
 include/xen/interface/xen.h  |   1 +
 9 files changed, 196 insertions(+)
 create mode 100644 include/xen/interface/hvm/dm_op.h

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 11d9f28..81e3217 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -457,4 +457,5 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_platform_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vm_assist);
+EXPORT_SYMBOL_GPL(HYPERVISOR_dm_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
index a648dfc..b0b80c0 100644
--- a/arch/arm/xen/hypercall.S
+++ b/arch/arm/xen/hypercall.S
@@ -92,6 +92,7 @@ HYPERCALL1(tmem_op);
 HYPERCALL1(platform_op_raw);
 HYPERCALL2(multicall);
 HYPERCALL2(vm_assist);
+HYPERCALL3(dm_op);
 
 ENTRY(privcmd_call)
stmdb sp!, {r4}
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 947830a..401ceb7 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -84,6 +84,7 @@ HYPERCALL1(tmem_op);
 HYPERCALL1(platform_op_raw);
 HYPERCALL2(multicall);
 HYPERCALL2(vm_assist);
+HYPERCALL3(dm_op);
 
 ENTRY(privcmd_call)
mov x16, x0
diff --git a/arch/x86/include/asm/xen/hypercall.h 
b/arch/x86/include/asm/xen/hypercall.h
index a12a047..f6d20f6 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -472,6 +472,13 @@ HYPERVISOR_xenpmu_op(unsigned int op, void *arg)
return _hypercall2(int, xenpmu_op, op, arg);
 }
 
+static inline int
+HYPERVISOR_dm_op(
+   domid_t dom, unsigned int nr_bufs, void *bufs)
+{
+   return _hypercall3(int, dm_op, dom, nr_bufs, bufs);
+}
+
 static inline void
 MULTI_fpu_taskswitch(struct multicall_entry *mcl, int set)
 {
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e5c7ae..a33f17e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -32,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +45,17 @@ MODULE_LICENSE("GPL");
 
 #define PRIV_VMA_LOCKED ((void *)1)
 
+unsigned int privcmd_dm_op_max_num = 16;
+module_param_named(dm_op_max_nr_bufs, privcmd_dm_op_max_num, uint, 0644);
+MODULE_PARM_DESC(dm_op_max_nr_bufs,
+"Maximum number of buffers per dm_op hypercall");
+
+unsigned int privcmd_dm_op_buf_max_size = XEN_PAGE_SIZE;
+module_param_named(dm_op_buf_max_size, privcmd_dm_op_buf_max_size, uint,
+  0644);
+MODULE_PARM_DESC(dm_op_buf_max_size,
+"Maximum size of a dm_op hypercall buffer");
+
 static int privcmd_vma_range_is_mapped(
struct vm_area_struct *vma,
unsigned long addr,
@@ -548,6 +561,128 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, 
int version)
goto out;
 }
 
+static int lock_pages(
+   struct privcmd_dm_op_buf kbufs[], unsigned int num,
+   struct page *pages[], unsigned int nr_pages)
+{
+   unsigned int i;
+
+   for (i = 0; i < num; i++) {
+   unsigned int requested;
+   int pinned;
+
+   requested = DIV_ROUND_UP(
+   offset_in_page(kbufs[i].uptr) + kbufs[i].size,
+   PAGE_SIZE);
+   if (requested > nr_pages)
+   return -ENOSPC;
+
+   pinned = get_user_pages_fast(
+   (unsigned long) kbufs[i].uptr,
+   requested, FOLL_WRITE, 

[Xen-devel] [PATCH v3 1/3] xen/privcmd: return -ENOTTY for unimplemented IOCTLs

2017-02-13 Thread Paul Durrant
The code sets the default return code to -ENOSYS but then overrides this
to -EINVAL in the switch() statement's default case, which is clearly
silly.

This patch removes the override and sets the default return code to
-ENOTTY, which is the conventional return for an unimplemented ioctl.

Signed-off-by: Paul Durrant 
---
Cc: Boris Ostrovsky 
Cc: Juergen Gross 

v2:
- Use -ENOTTY rather than -ENOSYS
---
 drivers/xen/privcmd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 6e3306f..5e5c7ae 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -551,7 +551,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, 
int version)
 static long privcmd_ioctl(struct file *file,
  unsigned int cmd, unsigned long data)
 {
-   int ret = -ENOSYS;
+   int ret = -ENOTTY;
void __user *udata = (void __user *) data;
 
switch (cmd) {
@@ -572,7 +572,6 @@ static long privcmd_ioctl(struct file *file,
break;
 
default:
-   ret = -EINVAL;
break;
}
 
-- 
2.1.4


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


[Xen-devel] [PATCH v3 0/3] xen/privcmd: support for dm_op and restriction

2017-02-13 Thread Paul Durrant
This patch series follows on from my recent Xen series [1], to provide
support in privcmd for de-privileging of device emulators.

[1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg02558.html

Paul Durrant (3):
  xen/privcmd: return -ENOTTY for unimplemented IOCTLs
  xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
  xen/privcmd: add IOCTL_PRIVCMD_RESTRICT

 arch/arm/xen/enlighten.c |   1 +
 arch/arm/xen/hypercall.S |   1 +
 arch/arm64/xen/hypercall.S   |   1 +
 arch/x86/include/asm/xen/hypercall.h |   7 ++
 drivers/xen/privcmd.c| 226 +--
 include/uapi/xen/privcmd.h   |  15 +++
 include/xen/arm/hypercall.h  |   1 +
 include/xen/interface/hvm/dm_op.h|  32 +
 include/xen/interface/xen.h  |   1 +
 9 files changed, 276 insertions(+), 9 deletions(-)
 create mode 100644 include/xen/interface/hvm/dm_op.h

-- 
2.1.4


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


Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling

2017-02-13 Thread Andrew Cooper
On 13/02/17 16:49, Jan Beulich wrote:
 On 13.02.17 at 14:03,  wrote:
>> Sending an invalidation to the device model is an internal detail of
>> completing the hypercall; callers should not need to be responsible for it.
>> Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
>> appropriate.
>>
>> This makes the function boolean in nature, although the existing
>> HVM_HCALL_{completed,preempted} to aid code clarity.
> "are being retained" missing somewhere here?

Yes.  I already noticed and fixed that up.  It now reads

"This makes the function boolean in nature, although the existing
HVM_HCALL_{completed,preempted} constants are kept to aid code clarity."

>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3874,7 +3874,7 @@ static const hypercall_table_t hvm_hypercall_table[] = 
>> {
>>  #undef HYPERCALL
>>  #undef COMPAT_CALL
>>  
>> -int hvm_do_hypercall(struct cpu_user_regs *regs)
>> +bool hvm_hypercall(struct cpu_user_regs *regs)
> I don't think bool is a particularly good choice when the callers can't
> sensibly use the result without comparing with HVM_HCALL_*

Ok.  I will keep it as int.

>
>> @@ -4011,9 +4011,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>>  return HVM_HCALL_preempted;
>>  
>>  if ( unlikely(currd->arch.hvm_domain.qemu_mapcache_invalidate) &&
>> - test_and_clear_bool(currd->arch.hvm_domain.
>> - qemu_mapcache_invalidate) )
>> -return HVM_HCALL_invalidate;
>> + 
>> test_and_clear_bool(currd->arch.hvm_domain.qemu_mapcache_invalidate) )
>> +send_invalidate_req();
> I wonder why things were done the old way in the first place. Did
> something else change, making that old model no longer required?
> I'm somewhat afraid we overlook something here, and hence an
> attempt to understand why this more immediate model wasn't
> used (and perhaps usable) back then might help... That aside, the
> patch looks fine.

Looks like it has been the same since it was first introduced in aeb2e1298b7

While that change does indicate it was replacing various I/O port hacks,
I can't see any reason why HVM_HCALL_invalidate was exposed outside of
hvm_do_hypercall().

~Andrew

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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Mon, Feb 13, 2017 at 9:37 AM, Julien Grall  wrote:
> Hi Tamas,
>
>
> On 13/02/17 16:20, Tamas K Lengyel wrote:
>>
>> On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>>  wrote:
>>>
>>> Hello,
>>>
>>> This e-mail is sort of follow-up to the two threads: [1] (my thread
>>> about TEE interaction) and [2] (Edgar's thread regarding handling SMC
>>> calls in platform_hvc). I want to discuss more broad topic there.
>>>
>>> Obviously, there are growing number of SMC users and current state of
>>> SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
>>> secure way, Xilinx wants to forward some calls directly to Secure
>>> Monitor, while allowing to handle other in userspace, etc.
>>>
>>> My proposition is to gather all requirements to SMC (and HVC) handling
>>> in one place (e.g. in this mail thread). After we' will have clear
>>> picture of what we want, we will be able to develop some solution,
>>> that will satisfy us all. At least, I hope so :)
>>>
>>> Also I want to remind, that there are ARM document called "SMC Calling
>>> Convention" [3]. According to it, any aarch64 hypervisor "must
>>> implement the Standard Secure and Hypervisor Service calls". At this
>>> moment XEN does not conform to this.
>>>
>>> So, lets get started with the requirements:
>>> 0. There are no much difference between SMC and HVC handling (at least
>>> according to SMCCC).
>>> 1. Hypervisor should at least provide own UUID and version while
>>> called by SMC/HVC
>>> 2. Hypervisor should forward some calls from dom0 directly to Secure
>>> Monitor (Xilinx use case)
>>> 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
>>> architecture service calls, etc.
>>> 4. Hypervisor should handle TEE calls in a secure way (e.g. no
>>> untrusted handlers in Dom0 userspace).
>>> 5. Hypervisor should support multiple TEEs (at least at compilation
>>> time).
>>> 6. Hypervisor should do this as fast as possible (DRM playback use case).
>>> 7. All domains (including dom0) should be handled in the same way.
>>> 8. Not all domains will have right to issue certain SMCs.
>>> 9. Hypervisor will issue own SMCs in some cases.
>>
>>
>> 10. Domains on which the monitor privileged call feature is enabled
>> (which is by default disabled for all domains) should not be able to
>> issue SMCs such that it reaches the firmware directly. Xen should not
>> bounce such calls to the firmware on behalf of the domain. Xen should
>> not alter the state of the domain automatically (ie. incrementing PC).
>> These calls should be exclusively transfered to the monitor subscriber
>> for further processing. HVC calls need not be included in the monitor
>> forwarding as long as the HVC call can be governed by XSM.
>
>
> This should not be a strong requirement. Whilst in your use case you want to
> forward all the SMCs to the monitor app, there are use case where Xen would
> need to emulate SMCs on the behalf of the guest. For instance see PSCI
> (arch/arm/vpsci.c).

In my usecases it is a strong requirement. What happens when the
monitor system is disabled is beyond my concerns - Xen can emulate or
forward the call as it wishes. But when the monitor application is in
use - in my usecase - it needs to be in exclusive control. If that
breaks an in-guest application, that is acceptable in my usecase. As
soon as there is another usecase that would need to support such an
application while the monitor system is enabled, the monitor system
can be fine-tuned for those needs to allow Xen to emulate. I've said
it many times, I have nothing against doing that, but as I don't need
it I won't be able to spend time implementing it.

>
> Another valid use case is Xen handling power management for device assigned
> to the guest and having the monitor app acting as a "Trusted App".
>
> Regarding the HVC call governed by XSM. I think this is the wrong way to g.
> As it was mentioned a couple of time HVC is a valid conduit for Secure
> monitor call. You should not deny them on the basis that your monitor app is
> not able to handle it properly. A better way would be to have a list of
> Secure Monitor Call (HVC/SMC) that should be forwarded to the monitor app.

I disagree. XSM needs to be in complete control of all hypercalls.
Whether denying some of them will break an application or not is not
Xen's concern. That is up to me as a user of Xen and XSM. If Xen
overrides a XSM policy because we hard-coded HVCs that pass-through,
that is a huge security policy violation. So even if we make a list of
HVCs that should also fall under the monitor privileged call umbrella,
it should still not override XSM. So since I would not be looking to
emulate anything that gets forwarded as a result of an HVC call, XSM
works for me just fine as the only thing I would do anyway is deny
them. So why would that list help when I might as well just make my
list more efficiently using XSM?

Tamas


Re: [Xen-devel] PML causing race condition during guest bootstorm and host crash on Broadwell cpu.

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 17:32,  wrote:
> On 09/02/17 16:22, Jan Beulich wrote:
>> Mind giving the attached patch a try (which admittedly was only
>> lightly tested so far - in particular I haven't seen the second of
>> the debug messages being logged, yet)?
> Patch looks promising. I couldn't do much thorough testing, but initial 
> reboot cycles (around 20 reboots of 32 VMS) went fine.

Thanks. The most interesting part though is whether the 2nd of the
two log messages ever showed up. I any event I'll submit the cleaned
up patch, for more formal discussion of the approach to happen there.

Jan


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


Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 14:03,  wrote:
> Sending an invalidation to the device model is an internal detail of
> completing the hypercall; callers should not need to be responsible for it.
> Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
> appropriate.
> 
> This makes the function boolean in nature, although the existing
> HVM_HCALL_{completed,preempted} to aid code clarity.

"are being retained" missing somewhere here?

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3874,7 +3874,7 @@ static const hypercall_table_t hvm_hypercall_table[] = 
> {
>  #undef HYPERCALL
>  #undef COMPAT_CALL
>  
> -int hvm_do_hypercall(struct cpu_user_regs *regs)
> +bool hvm_hypercall(struct cpu_user_regs *regs)

I don't think bool is a particularly good choice when the callers can't
sensibly use the result without comparing with HVM_HCALL_*

> @@ -4011,9 +4011,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>  return HVM_HCALL_preempted;
>  
>  if ( unlikely(currd->arch.hvm_domain.qemu_mapcache_invalidate) &&
> - test_and_clear_bool(currd->arch.hvm_domain.
> - qemu_mapcache_invalidate) )
> -return HVM_HCALL_invalidate;
> + 
> test_and_clear_bool(currd->arch.hvm_domain.qemu_mapcache_invalidate) )
> +send_invalidate_req();

I wonder why things were done the old way in the first place. Did
something else change, making that old model no longer required?
I'm somewhat afraid we overlook something here, and hence an
attempt to understand why this more immediate model wasn't
used (and perhaps usable) back then might help... That aside, the
patch looks fine.

Jan


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


Re: [Xen-devel] PML causing race condition during guest bootstorm and host crash on Broadwell cpu.

2017-02-13 Thread anshul makkar

Hi Jan,


On 09/02/17 16:22, Jan Beulich wrote:

On 08.02.17 at 16:32,  wrote:

On 07.02.17 at 18:26,  wrote:

Facing a issue where bootstorm of guests leads to host crash. I debugged
and found that that enabling PML  introduces a  race condition during
guest teardown stage while disabling PML on a vcpu  and context switch
happening for another vcpu.

Crash happens only on Broadwell processors as PML got introduced in this
generation.

Here is my analysis:

Race condition:

vmcs.c vmx_vcpu_disable_pml (vcpu){ vmx_vmcs_enter() ; vm_write(
disable_PML); vmx_vmcx_exit();)

In between I have a callpath from another pcpu executing context
switch-> vmx_fpu_leave() and crash on vmwrite..

if ( !(v->arch.hvm_vmx.host_cr0 & X86_CR0_TS) )
{
   v->arch.hvm_vmx.host_cr0 |= X86_CR0_TS;
   __vmwrite(HOST_CR0, v->arch.hvm_vmx.host_cr0);  //crash
   }

So that's after current has changed already, so it's effectively
dealing with a foreign VMCS, but it doesn't use vmx_vmcs_enter().
The locking done in vmx_vmcs_try_enter() / vmx_vmcs_exit(),
however, assumes that any user of a VMCS either owns the lock
or has current as the owner of the VMCS. Of course such a call
also can't be added here, as a vcpu on the context-switch-from
path can't vcpu_pause() itself.

That taken together with the bypassing of __context_switch()
when the incoming vCPU is the idle one (which means that via
context_saved() ->is_running will be cleared before running
->ctxt_switch_from()), the vcpu_pause() invocation in
vmx_vmcs_try_enter() may not have to wait at all if the call
happens between the clearing of ->is_running and the
eventual invocation of vmx_ctxt_switch_from().

Mind giving the attached patch a try (which admittedly was only
lightly tested so far - in particular I haven't seen the second of
the debug messages being logged, yet)?
Patch looks promising. I couldn't do much thorough testing, but initial 
reboot cycles (around 20 reboots of 32 VMS) went fine.


Jan


Anshul

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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Mon, Feb 13, 2017 at 9:29 AM, Volodymyr Babchuk
 wrote:
> Tamas,
>
> On 13 February 2017 at 18:20, Tamas K Lengyel  
> wrote:
>> On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>>  wrote:
>>> Hello,
>>>
>>> This e-mail is sort of follow-up to the two threads: [1] (my thread
>>> about TEE interaction) and [2] (Edgar's thread regarding handling SMC
>>> calls in platform_hvc). I want to discuss more broad topic there.
>>>
>>> Obviously, there are growing number of SMC users and current state of
>>> SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
>>> secure way, Xilinx wants to forward some calls directly to Secure
>>> Monitor, while allowing to handle other in userspace, etc.
>>>
>>> My proposition is to gather all requirements to SMC (and HVC) handling
>>> in one place (e.g. in this mail thread). After we' will have clear
>>> picture of what we want, we will be able to develop some solution,
>>> that will satisfy us all. At least, I hope so :)
>>>
>>> Also I want to remind, that there are ARM document called "SMC Calling
>>> Convention" [3]. According to it, any aarch64 hypervisor "must
>>> implement the Standard Secure and Hypervisor Service calls". At this
>>> moment XEN does not conform to this.
>>>
>>> So, lets get started with the requirements:
>>> 0. There are no much difference between SMC and HVC handling (at least
>>> according to SMCCC).
>>> 1. Hypervisor should at least provide own UUID and version while
>>> called by SMC/HVC
>>> 2. Hypervisor should forward some calls from dom0 directly to Secure
>>> Monitor (Xilinx use case)
>>> 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
>>> architecture service calls, etc.
>>> 4. Hypervisor should handle TEE calls in a secure way (e.g. no
>>> untrusted handlers in Dom0 userspace).
>>> 5. Hypervisor should support multiple TEEs (at least at compilation time).
>>> 6. Hypervisor should do this as fast as possible (DRM playback use case).
>>> 7. All domains (including dom0) should be handled in the same way.
>>> 8. Not all domains will have right to issue certain SMCs.
>>> 9. Hypervisor will issue own SMCs in some cases.
>>
>> 10. Domains on which the monitor privileged call feature is enabled
>> (which is by default disabled for all domains) should not be able to
>> issue SMCs such that it reaches the firmware directly. Xen should not
>> bounce such calls to the firmware on behalf of the domain. Xen should
>> not alter the state of the domain automatically (ie. incrementing PC).
>> These calls should be exclusively transfered to the monitor subscriber
>> for further processing. HVC calls need not be included in the monitor
>> forwarding as long as the HVC call can be governed by XSM.
>
> Looks like you are describing how SMC handling implemented at this
> moment. I agree that one can use VM monitor feature to handle SMCs.
> But are there any use case for this? Probably, you can implement
> userspace-based TEE in privileged domain. But for me this ruins the
> whole idea of TEE.

Yes, I have two separate usecases for this exact setup. The first is
an experimental security setup for ARM (described in
https://www.sec.in.tum.de/publications/publication/322); the second is
stealthy malware analysis where untrusted code in a guest domain
should be only able to interact with Xen and not the firmware.

Also, not sure why having this option in Xen would ruin any other
system needing SMCs like TEE in your case. The two use-cases may not
be compatible with each other, ie. they could not be used
simultaneously. But having the option for the user to decide which one
it wants to use should have no detrimental effect.

Tamas

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


[Xen-devel] [xen-unstable-smoke test] 105771: tolerable trouble: broken/fail/pass - PUSHED

2017-02-13 Thread osstest service owner
flight 105771 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/105771/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   5 xen-buildfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  28722e98f7ff1ef0da1bef885f98e232f89a8ca7
baseline version:
 xen  3884d7900a1528eebeaf276aa7614d9026de5811

Last test of basis   105769  2017-02-13 12:01:13 Z0 days
Testing same since   105771  2017-02-13 15:01:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Boris Ostrovsky 
  Chao Gao 
  Jan Beulich 
  Kevin Tian 
  Roger Pau Monné 

jobs:
 build-amd64  pass
 build-arm64  fail
 build-armhf  pass
 build-amd64-libvirt  pass
 build-arm64-pvopsfail
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  broken  
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=28722e98f7ff1ef0da1bef885f98e232f89a8ca7
+ . ./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 xen-unstable-smoke 
28722e98f7ff1ef0da1bef885f98e232f89a8ca7
+ branch=xen-unstable-smoke
+ revision=28722e98f7ff1ef0da1bef885f98e232f89a8ca7
+ . ./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=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.8-testing
+ '[' x28722e98f7ff1ef0da1bef885f98e232f89a8ca7 = 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
++ : 

Re: [Xen-devel] [PATCH 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 17:22,  wrote:
> On 13/02/17 16:12, Andrew Cooper wrote:
>> On 13/02/17 16:01, Jan Beulich wrote:
>> On 13.02.17 at 15:32,  wrote:
 To avoid leaking host MSR state into guests, guest LSTAR, STAR and
 SYSCALL_MASK state is unconditionally loaded when switching into guest
 context.

 Attempting to dirty-track the state is pointless; host state is always
 restoring upon exit from guest context, meaning that guest state is always
 considered dirty.

 Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The 
 guests
 MSR values are stored plainly in arch_vmx_struct, in the same way as 
 shadow_gs
 and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() 
 ensure
 that the hardware MSR values are always up-to-date.

 Signed-off-by: Andrew Cooper 
>>> Reviewed-by: Jan Beulich 
>>>
>>> However, the description above made me think whether always
>>> saving/restoring these MSRs is really needed (and desirable):
>>> We don't need the host values in place unless we context switch
>>> to a PV guest, so perhaps we should rather write them in
>>> paravirt_ctxt_switch_to()?
>> That would leak the values between different HVM guests.
>>
>> In principle we could skip the update if context switching to the idle
>> cpu, but that would involve leaking a VT-x-ism into the common code. 
>> SVM on the other hand automatically switches these MSRs on all
>> vmentries/exits so Xen always has its MSRs in context.
> 
> Furthermore, I did consider whether we should allow the guest to write
> to those MSRs directly, and handle them like shadow_gs.
> 
> I don't expect a plain OS to change them after initial setup, but a
> nested hypervisor (particularly Xen) is taking quite a performance hit
> on its context switch path because of these MSRs being intercepted at L0.

Ah, yes, that would likely help quite a bit.

Jan


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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Volodymyr Babchuk
Tamas,

On 13 February 2017 at 18:20, Tamas K Lengyel  wrote:
> On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
>  wrote:
>> Hello,
>>
>> This e-mail is sort of follow-up to the two threads: [1] (my thread
>> about TEE interaction) and [2] (Edgar's thread regarding handling SMC
>> calls in platform_hvc). I want to discuss more broad topic there.
>>
>> Obviously, there are growing number of SMC users and current state of
>> SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
>> secure way, Xilinx wants to forward some calls directly to Secure
>> Monitor, while allowing to handle other in userspace, etc.
>>
>> My proposition is to gather all requirements to SMC (and HVC) handling
>> in one place (e.g. in this mail thread). After we' will have clear
>> picture of what we want, we will be able to develop some solution,
>> that will satisfy us all. At least, I hope so :)
>>
>> Also I want to remind, that there are ARM document called "SMC Calling
>> Convention" [3]. According to it, any aarch64 hypervisor "must
>> implement the Standard Secure and Hypervisor Service calls". At this
>> moment XEN does not conform to this.
>>
>> So, lets get started with the requirements:
>> 0. There are no much difference between SMC and HVC handling (at least
>> according to SMCCC).
>> 1. Hypervisor should at least provide own UUID and version while
>> called by SMC/HVC
>> 2. Hypervisor should forward some calls from dom0 directly to Secure
>> Monitor (Xilinx use case)
>> 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
>> architecture service calls, etc.
>> 4. Hypervisor should handle TEE calls in a secure way (e.g. no
>> untrusted handlers in Dom0 userspace).
>> 5. Hypervisor should support multiple TEEs (at least at compilation time).
>> 6. Hypervisor should do this as fast as possible (DRM playback use case).
>> 7. All domains (including dom0) should be handled in the same way.
>> 8. Not all domains will have right to issue certain SMCs.
>> 9. Hypervisor will issue own SMCs in some cases.
>
> 10. Domains on which the monitor privileged call feature is enabled
> (which is by default disabled for all domains) should not be able to
> issue SMCs such that it reaches the firmware directly. Xen should not
> bounce such calls to the firmware on behalf of the domain. Xen should
> not alter the state of the domain automatically (ie. incrementing PC).
> These calls should be exclusively transfered to the monitor subscriber
> for further processing. HVC calls need not be included in the monitor
> forwarding as long as the HVC call can be governed by XSM.

Looks like you are describing how SMC handling implemented at this
moment. I agree that one can use VM monitor feature to handle SMCs.
But are there any use case for this? Probably, you can implement
userspace-based TEE in privileged domain. But for me this ruins the
whole idea of TEE.

-- 
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 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 17:12,  wrote:
> On 13/02/17 16:01, Jan Beulich wrote:
> On 13.02.17 at 15:32,  wrote:
>>> To avoid leaking host MSR state into guests, guest LSTAR, STAR and
>>> SYSCALL_MASK state is unconditionally loaded when switching into guest
>>> context.
>>>
>>> Attempting to dirty-track the state is pointless; host state is always
>>> restoring upon exit from guest context, meaning that guest state is always
>>> considered dirty.
>>>
>>> Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The guests
>>> MSR values are stored plainly in arch_vmx_struct, in the same way as 
>>> shadow_gs
>>> and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure
>>> that the hardware MSR values are always up-to-date.
>>>
>>> Signed-off-by: Andrew Cooper 
>> Reviewed-by: Jan Beulich 
>>
>> However, the description above made me think whether always
>> saving/restoring these MSRs is really needed (and desirable):
>> We don't need the host values in place unless we context switch
>> to a PV guest, so perhaps we should rather write them in
>> paravirt_ctxt_switch_to()?
> 
> That would leak the values between different HVM guests.

How that? Each HVM guest gets its values written. If what you say
were the case, there would then continue to be a host value leak,
as you'd imply nothing writes these MSRs when entering a HVM
guest.

> In principle we could skip the update if context switching to the idle
> cpu,

Doesn't that happen already, by virtue of __context_switch()
being skipped in that case, yet ->ctxt_switch_from() being
invoked from there (and itself being guarded by an idle domain
check)?

> but that would involve leaking a VT-x-ism into the common code. 
> SVM on the other hand automatically switches these MSRs on all
> vmentries/exits so Xen always has its MSRs in context.

Yes, the difference to SVM makes this a little unwieldy.

Jan


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


Re: [Xen-devel] [ARM] SMC (and HVC) handling in hypervisor

2017-02-13 Thread Tamas K Lengyel
On Fri, Feb 10, 2017 at 5:14 PM, Volodymyr Babchuk
 wrote:
> Hello,
>
> This e-mail is sort of follow-up to the two threads: [1] (my thread
> about TEE interaction) and [2] (Edgar's thread regarding handling SMC
> calls in platform_hvc). I want to discuss more broad topic there.
>
> Obviously, there are growing number of SMC users and current state of
> SMC handling in Xen satisfies nobody. My team wants to handle SMCs in
> secure way, Xilinx wants to forward some calls directly to Secure
> Monitor, while allowing to handle other in userspace, etc.
>
> My proposition is to gather all requirements to SMC (and HVC) handling
> in one place (e.g. in this mail thread). After we' will have clear
> picture of what we want, we will be able to develop some solution,
> that will satisfy us all. At least, I hope so :)
>
> Also I want to remind, that there are ARM document called "SMC Calling
> Convention" [3]. According to it, any aarch64 hypervisor "must
> implement the Standard Secure and Hypervisor Service calls". At this
> moment XEN does not conform to this.
>
> So, lets get started with the requirements:
> 0. There are no much difference between SMC and HVC handling (at least
> according to SMCCC).
> 1. Hypervisor should at least provide own UUID and version while
> called by SMC/HVC
> 2. Hypervisor should forward some calls from dom0 directly to Secure
> Monitor (Xilinx use case)
> 3. Hypervisor should virtualize PSCI calls, CPU service calls, ARM
> architecture service calls, etc.
> 4. Hypervisor should handle TEE calls in a secure way (e.g. no
> untrusted handlers in Dom0 userspace).
> 5. Hypervisor should support multiple TEEs (at least at compilation time).
> 6. Hypervisor should do this as fast as possible (DRM playback use case).
> 7. All domains (including dom0) should be handled in the same way.
> 8. Not all domains will have right to issue certain SMCs.
> 9. Hypervisor will issue own SMCs in some cases.

10. Domains on which the monitor privileged call feature is enabled
(which is by default disabled for all domains) should not be able to
issue SMCs such that it reaches the firmware directly. Xen should not
bounce such calls to the firmware on behalf of the domain. Xen should
not alter the state of the domain automatically (ie. incrementing PC).
These calls should be exclusively transfered to the monitor subscriber
for further processing. HVC calls need not be included in the monitor
forwarding as long as the HVC call can be governed by XSM.

Tamas

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


Re: [Xen-devel] [PATCH 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Andrew Cooper
On 13/02/17 16:12, Andrew Cooper wrote:
> On 13/02/17 16:01, Jan Beulich wrote:
> On 13.02.17 at 15:32,  wrote:
>>> To avoid leaking host MSR state into guests, guest LSTAR, STAR and
>>> SYSCALL_MASK state is unconditionally loaded when switching into guest
>>> context.
>>>
>>> Attempting to dirty-track the state is pointless; host state is always
>>> restoring upon exit from guest context, meaning that guest state is always
>>> considered dirty.
>>>
>>> Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The guests
>>> MSR values are stored plainly in arch_vmx_struct, in the same way as 
>>> shadow_gs
>>> and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure
>>> that the hardware MSR values are always up-to-date.
>>>
>>> Signed-off-by: Andrew Cooper 
>> Reviewed-by: Jan Beulich 
>>
>> However, the description above made me think whether always
>> saving/restoring these MSRs is really needed (and desirable):
>> We don't need the host values in place unless we context switch
>> to a PV guest, so perhaps we should rather write them in
>> paravirt_ctxt_switch_to()?
> That would leak the values between different HVM guests.
>
> In principle we could skip the update if context switching to the idle
> cpu, but that would involve leaking a VT-x-ism into the common code. 
> SVM on the other hand automatically switches these MSRs on all
> vmentries/exits so Xen always has its MSRs in context.

Furthermore, I did consider whether we should allow the guest to write
to those MSRs directly, and handle them like shadow_gs.

I don't expect a plain OS to change them after initial setup, but a
nested hypervisor (particularly Xen) is taking quite a performance hit
on its context switch path because of these MSRs being intercepted at L0.

~Andrew

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


Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document

2017-02-13 Thread Julien Grall

Hi Roger,

On 02/02/17 15:40, Roger Pau Monné wrote:

On Wed, Feb 01, 2017 at 07:04:43PM +, Julien Grall wrote:

Or maybe we could avoid mapping the doorbell in the guest and let Xen
receive an SMMU abort. When receiving the SMMU abort, Xen could sanitize the
value and write into the real MSI doorbell. Not sure if it would works
thought.


AFAIK (and I might be wrong) you can only know the address that caused the
fault, but not the data that was attempted to be written there. TBH, I wouldn't
expect this approach to work.


You are right, I got confused with the data abort path. So I guess there 
is no way to do secure MSI in this case :/


Cheers,



Roger.



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


Re: [Xen-devel] [PATCH v2 01/11] x86emul: catch exceptions occurring in stubs

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 14:58,  wrote:
> On 13/02/17 11:40, Jan Beulich wrote:
> On 10.02.17 at 17:38,  wrote:
>>> On 01/02/17 11:12, Jan Beulich wrote:
 Before adding more use of stubs cloned from decoded guest insns, guard
 ourselves against mistakes there: Should an exception (with the
 noteworthy exception of #PF) occur inside the stub, forward it to the
 guest.
>>> Why exclude #PF ? Nothing in a stub should be hitting a pagefault in the
>>> first place.
>> To be honest, I've been considering to limit this to just #UD. We
>> clearly shouldn't hide memory addressing issues, as them going
>> by silently means information leaks. Nevertheless including #PF
>> here would be a trivial change to the patch.
> 
> When I considered this first, my plan was to catch the fault and crash
> the domain, rather than allow it to continue (FPU exceptions being the
> exception).
> 
> One way or another, by the time we encounter a fault in the stubs,
> something has gone wrong, and crashing the domain is better than
> crashing the host.  (In fact, I am looking to extend this principle
> further, e.g. with vmread/vmwrite failures.)
> 
> I don't see #PF being meaningfully different to #GP or #SS here.  If we
> get a fault, an action was stopped, but we can never catch the issues
> which don't fault in the first place.
> 
> #UD is a little more tricky.  It either means that we created a
> malformed stub, or we didn't have sufficient feature checking, both of
> which are emulation bugs.  This could be passed back to the domain, but
> I'd err on the side of making it more obvious by crashing the domain. 

Generally yes, but I think here we really should forward at least
#UD. I can agree with other faults being terminal to the domain,
which will the also allow #PF to be handled uniformly (as there
won't be a need to propagate some CR2 value).

> (Perhaps changing behaviour based on debug?)

Not here, I would say - this logic should be tested the way it is
meant to be run in production.

 ---
 There's one possible caveat here: A stub invocation immediately
 followed by another instruction having fault revovery attached to it
 would not work properly, as the table lookup can only ever find one of
 the two entries. Such CALL instructions would then need to be followed
 by a NOP for disambiguation (even if only a slim chance exists for the
 compiler to emit things that way).
>>> Why key on return address at all?  %rip being in the stubs should be
>>> good enough.
>> Well, we need unique (key-address, recovery-address) tuples,
>> and key-address can't possibly be an address inside the stub
>> (for both the address varying between CPUs and said uniqueness
>> requirement).
> 
> We don't necessarily need to use the extable infrastructure, and you
> don't appear to be using a unique key at all.

How am I not? How would both the self test and the emulator
uses work without unique addresses to key off of?

 TBD: Instead of adding a 2nd search_exception_table() invocation to
  do_trap(), we may want to consider moving the existing one down:
  Xen code (except when executing stubs) shouldn't be raising #MF
  or #XM, and hence fixups attached to instructions shouldn't care
  about getting invoked for those. With that, doing the HVM special
  case for them before running search_exception_table() would be
  fine.
>> No opinion at all on this aspect?
> 
> Sorry - I was thinking it over and forgot to comment before sending. 
> Your suggestion is fine, but doesn't it disappear if/when we fold the
> existing fpu_exception_callback() into this more generic infrastructure.

Well, I simply didn't mean to do this folding, as I think the way it
is being handled right now is acceptable for the purpose. We can
certainly revisit this later.

 @@ -85,15 +86,88 @@ search_one_extable(const struct exceptio
  }
  
  unsigned long
 -search_exception_table(unsigned long addr)
 +search_exception_table(const struct cpu_user_regs *regs, bool check_stub)
  {
 -const struct virtual_region *region = find_text_region(addr);
 +const struct virtual_region *region = find_text_region(regs->rip);
 +unsigned long stub = this_cpu(stubs.addr);
  
  if ( region && region->ex )
 -return search_one_extable(region->ex, region->ex_end - 1, addr);
 +return search_one_extable(region->ex, region->ex_end - 1, 
 regs->rip);
 +
 +if ( check_stub &&
 + regs->rip >= stub + STUB_BUF_SIZE / 2 &&
 + regs->rip < stub + STUB_BUF_SIZE &&
 + regs->rsp > (unsigned long)_stub &&
 + regs->rsp < (unsigned long)get_cpu_info() )
>>> How much do we care about accidentally clobbering %rsp in a stub?
>> I think we can't guard against everything, but we should do the
>> best we reasonably can. I.e. in the case 

Re: [Xen-devel] [PATCH 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Andrew Cooper
On 13/02/17 16:01, Jan Beulich wrote:
 On 13.02.17 at 15:32,  wrote:
>> To avoid leaking host MSR state into guests, guest LSTAR, STAR and
>> SYSCALL_MASK state is unconditionally loaded when switching into guest
>> context.
>>
>> Attempting to dirty-track the state is pointless; host state is always
>> restoring upon exit from guest context, meaning that guest state is always
>> considered dirty.
>>
>> Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The guests
>> MSR values are stored plainly in arch_vmx_struct, in the same way as 
>> shadow_gs
>> and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure
>> that the hardware MSR values are always up-to-date.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 
>
> However, the description above made me think whether always
> saving/restoring these MSRs is really needed (and desirable):
> We don't need the host values in place unless we context switch
> to a PV guest, so perhaps we should rather write them in
> paravirt_ctxt_switch_to()?

That would leak the values between different HVM guests.

In principle we could skip the update if context switching to the idle
cpu, but that would involve leaking a VT-x-ism into the common code. 
SVM on the other hand automatically switches these MSRs on all
vmentries/exits so Xen always has its MSRs in context.

~Andrew

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


Re: [Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Roger Pau Monne
On Mon, Feb 13, 2017 at 04:04:43PM +, Andrew Cooper wrote:
> On 13/02/17 15:59, Wei Liu wrote:
> > On Mon, Feb 13, 2017 at 03:49:14PM +, Roger Pau Monne wrote:
> >> Sorry, I've forgot to re-generate the patch after adding the maintainers...
> >>
> >> On Mon, Feb 13, 2017 at 03:47:38PM +, Roger Pau Monne wrote:
> >>> Bash it's not used on FreeBSD.
> >>>
> >>> Signed-off-by: Roger Pau Monné 
> >>> ---
> >>> Please re-run autoconf after applying
> >>> ---
> >>>  tools/configure.ac | 6 +-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/configure.ac b/tools/configure.ac
> >>> index 873e18d..28a539c 100644
> >>> --- a/tools/configure.ac
> >>> +++ b/tools/configure.ac
> >>> @@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
> >>>  xsmpolicy="n"
> >>>  ])
> >>>  ])
> >>> -AX_PATH_PROG_OR_FAIL([BASH], [bash])
> >>> +dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)
> > I am not sure I follow this comment.  It implies hotplug scripts are the
> > only shell scripts that we ship or care and this check here is
> > specifically for that purpose.
> >
> > If this comment is correct, isn't it better to just  remove this check?
> > AFAICT sh is standard.  If this comment is not correct, should we check
> > for the desired shell in FreeBSD?
> 
> As an upstream, everything we publish should be sh-compatible.  Even on
> some Linux distros, it is common for the default shell to be dash rather
> than bash.
> 
> If our scripts aren't sh-compatible, we should make them so.

IIRC the hotplug scripts on Linux are full of bashisms.

Roger.

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


Re: [Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Roger Pau Monne
On Mon, Feb 13, 2017 at 03:59:15PM +, Wei Liu wrote:
> On Mon, Feb 13, 2017 at 03:49:14PM +, Roger Pau Monne wrote:
> > Sorry, I've forgot to re-generate the patch after adding the maintainers...
> > 
> > On Mon, Feb 13, 2017 at 03:47:38PM +, Roger Pau Monne wrote:
> > > Bash it's not used on FreeBSD.
> > > 
> > > Signed-off-by: Roger Pau Monné 
> > > ---
> > > Please re-run autoconf after applying
> > > ---
> > >  tools/configure.ac | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/configure.ac b/tools/configure.ac
> > > index 873e18d..28a539c 100644
> > > --- a/tools/configure.ac
> > > +++ b/tools/configure.ac
> > > @@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
> > >  xsmpolicy="n"
> > >  ])
> > >  ])
> > > -AX_PATH_PROG_OR_FAIL([BASH], [bash])
> > > +dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)
> 
> I am not sure I follow this comment.  It implies hotplug scripts are the
> only shell scripts that we ship or care and this check here is
> specifically for that purpose.
> 
> If this comment is correct, isn't it better to just  remove this check?
> AFAICT sh is standard.  If this comment is not correct, should we check
> for the desired shell in FreeBSD?

Hotplug scripts are specific to each OS, and the FreeBSD ones use /bin/sh as
the parser, which is present in base (ie: there's no reason to explicitly check
for it, also because it's the same that's used in configure). Linux OTOH uses
bash as the hotplug script parser, hence this check is needed there.

Hope this makes sense, Roger.

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


Re: [Xen-devel] [PATCH 4/4] x86/vmx: Drop vmx_msr_state infrastructure

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 15:32,  wrote:
> To avoid leaking host MSR state into guests, guest LSTAR, STAR and
> SYSCALL_MASK state is unconditionally loaded when switching into guest
> context.
> 
> Attempting to dirty-track the state is pointless; host state is always
> restoring upon exit from guest context, meaning that guest state is always
> considered dirty.
> 
> Drop struct vmx_msr_state, enum VMX_INDEX_MSR_* and msr_index[].  The guests
> MSR values are stored plainly in arch_vmx_struct, in the same way as shadow_gs
> and cstar are.  vmx_restore_guest_msrs() and long_mode_do_msr_write() ensure
> that the hardware MSR values are always up-to-date.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

However, the description above made me think whether always
saving/restoring these MSRs is really needed (and desirable):
We don't need the host values in place unless we context switch
to a PV guest, so perhaps we should rather write them in
paravirt_ctxt_switch_to()?

Jan


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


Re: [Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Andrew Cooper
On 13/02/17 15:59, Wei Liu wrote:
> On Mon, Feb 13, 2017 at 03:49:14PM +, Roger Pau Monne wrote:
>> Sorry, I've forgot to re-generate the patch after adding the maintainers...
>>
>> On Mon, Feb 13, 2017 at 03:47:38PM +, Roger Pau Monne wrote:
>>> Bash it's not used on FreeBSD.
>>>
>>> Signed-off-by: Roger Pau Monné 
>>> ---
>>> Please re-run autoconf after applying
>>> ---
>>>  tools/configure.ac | 6 +-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/configure.ac b/tools/configure.ac
>>> index 873e18d..28a539c 100644
>>> --- a/tools/configure.ac
>>> +++ b/tools/configure.ac
>>> @@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
>>>  xsmpolicy="n"
>>>  ])
>>>  ])
>>> -AX_PATH_PROG_OR_FAIL([BASH], [bash])
>>> +dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)
> I am not sure I follow this comment.  It implies hotplug scripts are the
> only shell scripts that we ship or care and this check here is
> specifically for that purpose.
>
> If this comment is correct, isn't it better to just  remove this check?
> AFAICT sh is standard.  If this comment is not correct, should we check
> for the desired shell in FreeBSD?

As an upstream, everything we publish should be sh-compatible.  Even on
some Linux distros, it is common for the default shell to be dash rather
than bash.

If our scripts aren't sh-compatible, we should make them so.

~Andrew

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


Re: [Xen-devel] [PATCH v2 2/4] x86/setup: Intoduce XEN_MSR_STAR

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 16:56,  wrote:
> Xen's choice of the MSR_STAR value is constant across all pcpus.  Introduce a
> new define and use it to avoid the opencoding in subarch_percpu_traps_init()
> and restore_rest_processor_state().
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



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


Re: [Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Wei Liu
On Mon, Feb 13, 2017 at 03:49:14PM +, Roger Pau Monne wrote:
> Sorry, I've forgot to re-generate the patch after adding the maintainers...
> 
> On Mon, Feb 13, 2017 at 03:47:38PM +, Roger Pau Monne wrote:
> > Bash it's not used on FreeBSD.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Please re-run autoconf after applying
> > ---
> >  tools/configure.ac | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/configure.ac b/tools/configure.ac
> > index 873e18d..28a539c 100644
> > --- a/tools/configure.ac
> > +++ b/tools/configure.ac
> > @@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
> >  xsmpolicy="n"
> >  ])
> >  ])
> > -AX_PATH_PROG_OR_FAIL([BASH], [bash])
> > +dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)

I am not sure I follow this comment.  It implies hotplug scripts are the
only shell scripts that we ship or care and this check here is
specifically for that purpose.

If this comment is correct, isn't it better to just  remove this check?
AFAICT sh is standard.  If this comment is not correct, should we check
for the desired shell in FreeBSD?

Wei.

> > +case "$host_os" in
> > +  freebsd*) ;;
> > +  *) AX_PATH_PROG_OR_FAIL([BASH], [bash]);;
> > +esac
> >  AS_IF([echo "$PYTHON" | grep -q "^/"], [
> >  PYTHONPATH=$PYTHON
> >  PYTHON=`basename $PYTHONPATH`
> > -- 
> > 2.10.1 (Apple Git-78)
> > 

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


[Xen-devel] [PATCH v2 2/4] x86/setup: Intoduce XEN_MSR_STAR

2017-02-13 Thread Andrew Cooper
Xen's choice of the MSR_STAR value is constant across all pcpus.  Introduce a
new define and use it to avoid the opencoding in subarch_percpu_traps_init()
and restore_rest_processor_state().

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2:
 * Drop all the adjustments to MSR_CSTAR
---
 xen/arch/x86/acpi/suspend.c | 4 ++--
 xen/arch/x86/x86_64/traps.c | 4 ++--
 xen/include/asm-x86/processor.h | 3 +++
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c
index d5c67ee..0dba3e5 100644
--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -54,8 +54,8 @@ void restore_rest_processor_state(void)
 /* Recover syscall MSRs */
 wrmsrl(MSR_LSTAR, saved_lstar);
 wrmsrl(MSR_CSTAR, saved_cstar);
-wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
-wrmsr(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK, 0U);
+wrmsrl(MSR_STAR, XEN_MSR_STAR);
+wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
 
 wrfsbase(saved_fs_base);
 wrgsbase(saved_gs_base);
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index fc8cde6..252d2d0 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -415,8 +415,8 @@ void subarch_percpu_traps_init(void)
 unmap_domain_page(stub_page);
 
 /* Common SYSCALL parameters. */
-wrmsr(MSR_STAR, 0, ((unsigned int)FLAT_RING3_CS32 << 16) | 
__HYPERVISOR_CS);
-wrmsr(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK, 0U);
+wrmsrl(MSR_STAR, XEN_MSR_STAR);
+wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
 }
 
 void init_int80_direct_trap(struct vcpu *v)
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 7735bc2..843f072 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -87,6 +87,9 @@
 
 #define XEN_CR4_PV32_BITS (X86_CR4_SMEP|X86_CR4_SMAP)
 
+/* Common SYSCALL parameters. */
+#define XEN_MSR_STAR (((uint64_t)FLAT_RING3_CS32 << 48) |   \
+  ((uint64_t)__HYPERVISOR_CS << 32))
 #define XEN_SYSCALL_MASK (X86_EFLAGS_AC|X86_EFLAGS_VM|X86_EFLAGS_RF|\
   X86_EFLAGS_NT|X86_EFLAGS_DF|X86_EFLAGS_IF|\
   X86_EFLAGS_TF)
-- 
2.1.4


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


Re: [Xen-devel] [PATCH 3/4] x86/vmx: Remove vmx_save_host_msrs() and host_msr_state

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 15:32,  wrote:
> A pcpu's LSTAR, STAR and SYSCALL_MASK MSRs are unconditionally switched when
> moving in and out of HVM vcpu context.  Two of these values are compile time
> constants, and the third is directly available in an existing per-cpu
> variable.
> 
> There is no need to save host state in vmx_cpu_up() into a different per-cpu
> structure, so drop all the infrastructure.  vmx_restore_host_msrs() is
> simplified to 3 plain WRMSR instructions.
> 
> Signed-off-by: Andrew Cooper 

Nice.

Reviewed-by: Jan Beulich 

Jan


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


Re: [Xen-devel] [PATCH 2/4] x86/setup: Minor cleanup to host SYSCALL MSR handling

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 16:33,  wrote:
> On 13/02/17 15:30, Jan Beulich wrote:
> On 13.02.17 at 16:26,  wrote:
>>> On 13/02/17 15:19, Jan Beulich wrote:
>>> On 13.02.17 at 15:32,  wrote:
> Xen's choice of the MSR_STAR value is constant across all pcpus.  
> Introduce a
> new define and use it to avoid the opencoding in 
> subarch_percpu_traps_init()
> and restore_rest_processor_state().
>
> Despite Intel hardware having an MSR_CSTAR register, nothing actually 
> uses it
> as the SYSCALL instruction raises #UD out of 64bit mode, meaning that
 Did you mean "32-bit"?
>>> SYSCALL on Intel will #UD if CS.L == 0, so you can't even use it from a
>>> compatability mode segment.
>> Right - so my comment stands. Or did you mean "outside of 64-bit
>> mode"?
> 
> I did, but I don't see how that is meaningfully different from what I wrote.

Interesting, you'll have to correct my English understanding then:
"out of " to me so far meant originating in ,
quite the opposite of "outside of ".

Jan


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


Re: [Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Roger Pau Monne
Sorry, I've forgot to re-generate the patch after adding the maintainers...

On Mon, Feb 13, 2017 at 03:47:38PM +, Roger Pau Monne wrote:
> Bash it's not used on FreeBSD.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Please re-run autoconf after applying
> ---
>  tools/configure.ac | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 873e18d..28a539c 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
>  xsmpolicy="n"
>  ])
>  ])
> -AX_PATH_PROG_OR_FAIL([BASH], [bash])
> +dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)
> +case "$host_os" in
> +  freebsd*) ;;
> +  *) AX_PATH_PROG_OR_FAIL([BASH], [bash]);;
> +esac
>  AS_IF([echo "$PYTHON" | grep -q "^/"], [
>  PYTHONPATH=$PYTHON
>  PYTHON=`basename $PYTHONPATH`
> -- 
> 2.10.1 (Apple Git-78)
> 

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


[Xen-devel] [PATCH] configure: disable bash check for FreeBSD

2017-02-13 Thread Roger Pau Monne
Bash it's not used on FreeBSD.

Signed-off-by: Roger Pau Monné 
---
Please re-run autoconf after applying
---
 tools/configure.ac | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/configure.ac b/tools/configure.ac
index 873e18d..28a539c 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -320,7 +320,11 @@ AS_IF([test "x$xsmpolicy" = "xy"], [
 xsmpolicy="n"
 ])
 ])
-AX_PATH_PROG_OR_FAIL([BASH], [bash])
+dnl FreeBSD doesn't require bash (hotplug scripts are in plain sh)
+case "$host_os" in
+  freebsd*) ;;
+  *) AX_PATH_PROG_OR_FAIL([BASH], [bash]);;
+esac
 AS_IF([echo "$PYTHON" | grep -q "^/"], [
 PYTHONPATH=$PYTHON
 PYTHON=`basename $PYTHONPATH`
-- 
2.10.1 (Apple Git-78)


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


Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document

2017-02-13 Thread Julien Grall

Hi Stefano,

On 10/02/17 01:01, Stefano Stabellini wrote:

On Fri, 3 Feb 2017, Edgar E. Iglesias wrote:

A possible hack could be to allocate a chunk of DDR dedicated for PCI DMA.
PCI DMA devs could be locked in to only be able to access this mem + MSI 
doorbell.
Guests can still screw each other up but at least it becomes harder to 
read/write directly from each others OS memory.
It may not be worth the effort though


Actually, we do have the swiotlb in Dom0, which can be used to bounce
DMA requests over a buffer that has been previously setup to be DMA safe
using an hypercall. That is how the swiotlb is used on x86. On ARM it is
used to issue cache flushes via hypercall, but it could be adapted to do
both. It would degrade performance, due to the additional memcpy, but it
would work, I believe.


A while ago, Globallogic suggested to use direct memory mapping for the 
guest to allow guest using DMA on platform not supporting SMMU.


I believe we can use the same trick on platform where SMMUs can not 
distinguish PCI devices.


Cheers,

--
Julien Grall

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


Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document

2017-02-13 Thread Julien Grall

On 02/02/17 15:33, Edgar E. Iglesias wrote:

On Wed, Feb 01, 2017 at 07:04:43PM +, Julien Grall wrote:

On 31/01/2017 19:06, Edgar E. Iglesias wrote:

On Tue, Jan 31, 2017 at 05:09:53PM +, Julien Grall wrote:

Thank you for the documentation. I am trying to understand if we could move
initialization in Xen as suggested by Stefano. I looked at the driver in
Linux and the code looks simple not many dependencies. However, I was not
able to find where the Gigabit Transceivers are configured. Do you have any
link to the code for that?


Hi Julien,


Hi Edgar,



I suspect that this setup has previously been done by the initial bootloader
auto-generated from design configuration tools.

Now, this is moving into Linux.


Do you know why they decide to move the code in Linux? What would be the 
problem to let the bootloader configuring the GT?



There's a specific driver that does that but AFAICS, it has not been upstreamed 
yet.
You can see it here:
https://github.com/Xilinx/linux-xlnx/blob/master/drivers/phy/phy-zynqmp.c

DTS nodes that need a PHY can then just refer to it, here's an example from 
SATA:
 {
phy-names = "sata-phy";
phys = < PHY_TYPE_SATA 1 3 15000>;
};

I'll see if I can find working examples for PCIe on the ZCU102. Then I'll share
DTS, Kernel etc.


I've found a device tree on the github from the ZCU102: 
zynqmp-zcu102.dts, it looks like there is no use of PHY for the pcie so 
far.


Lets imagine in the future, pcie will use the PHY. If we decide to 
initialize the hostbridge in Xen, we would also have to pull the PHY 
code in the hypervisor. Leaving aside the problem to pull more code in 
Xen, this is not nice because the PHY is used by different components 
(e.g SATA, USB). So Xen and DOM0 would have to share the PHY.


For Xen POV, the best solution would be the bootloader initializing the 
PHY because starting Xen. So we can keep all the hostbridge 
(initialization + access) in Xen.


If it is not possible, then I would prefer to see the hostbridge 
initialization in DOM0.




If you are looking for a platform to get started, an option could be if I get 
you a build of
our QEMU that includes models for the PCIe controller, MSI and SMMU connections.
These models are friendly wrt. PHY configs and initialization sequences, it will
accept pretty much any sequence and still work. This would allow you to focus on
architectural issues rather than exact details of init sequences (which we can
deal with later).


From my understanding the problem is where the hostbridge should be 
initialized. In an ideal world, I think this is the goal of the 
bootloader. If it is not possible then depending on the complexity, the 
initialization would have to be done either in Xen or DOM0.


I guess this could be decided on case by case basis. I will suggest 
different possibility in the design document.


[...]



From a design point of view, it would make more sense to have the MSI
controller driver in Xen as the hostbridge emulation for guest will also
live there.

So if we receive MSI in Xen, we need to figure out a way for DOM0 and guest
to receive MSI. The same way would be the best, and I guess non-PV if
possible. I know you are looking to boot unmodified OS in a VM. This would
mean we need to emulate the MSI controller and potentially xilinx PCI
controller. How much are you willing to modify the OS?


Today, we have not yet implemented PCIe drivers for our baremetal SDK. So
things are very open and we could design with pretty much anything in mind.

Yes, we could perhaps include a very small model with most registers dummied.
Implementing the MSI read FIFO would allow us to:

1. Inject the MSI doorbell SPI into guests. The guest will then see the same
   IRQ as on real HW.

2. Guest reads host-controller registers (MSI FIFO) to get the signaled MSI.


The Xilinx PCIe hostbridge is not the only hostbridge having MSI 
controller embedded. So I would like to see a generic solution if 
possible. This would avoid to increase the code required for emulation 
in Xen.


My concern with a FIFO is it will require an upper bound to avoid using 
to much memory in Xen. What if the FIFO is full? Will you drop MSI?



Regarding the MSI doorbell, I have seen it is configured by the software
using a physical address of a page allocated in the RAM. When the PCI
devices is writing into the doorbell does the access go through the SMMU?


That's a good question. On our QEMU model it does, but I'll have to dig a 
little to see if that is the case on real HW aswell.


Regardless the answer, I think we would need to map the MSI doorbell page in
the guest. Meaning that even if we trap MSI configuration access, a guess
could DMA in the page. So if I am not mistaken, MSI would be insecure in
this case :/.

Or maybe we could avoid mapping the doorbell in the guest and let Xen
receive an SMMU abort. When receiving the SMMU abort, Xen could sanitize the
value and write into the real MSI doorbell. Not sure 

Re: [Xen-devel] [PATCH 2/4] x86/setup: Minor cleanup to host SYSCALL MSR handling

2017-02-13 Thread Andrew Cooper
On 13/02/17 15:30, Jan Beulich wrote:
 On 13.02.17 at 16:26,  wrote:
>> On 13/02/17 15:19, Jan Beulich wrote:
>> On 13.02.17 at 15:32,  wrote:
 Xen's choice of the MSR_STAR value is constant across all pcpus.  
 Introduce a
 new define and use it to avoid the opencoding in 
 subarch_percpu_traps_init()
 and restore_rest_processor_state().

 Despite Intel hardware having an MSR_CSTAR register, nothing actually uses 
 it
 as the SYSCALL instruction raises #UD out of 64bit mode, meaning that
>>> Did you mean "32-bit"?
>> SYSCALL on Intel will #UD if CS.L == 0, so you can't even use it from a
>> compatability mode segment.
> Right - so my comment stands. Or did you mean "outside of 64-bit
> mode"?

I did, but I don't see how that is meaningfully different from what I wrote.

OTOH, It doesn't matter as I am dropping that half of the patch.

~Andrew

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


Re: [Xen-devel] [PATCH 2/4] x86/setup: Minor cleanup to host SYSCALL MSR handling

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 16:26,  wrote:
> On 13/02/17 15:19, Jan Beulich wrote:
> On 13.02.17 at 15:32,  wrote:
>>> Xen's choice of the MSR_STAR value is constant across all pcpus.  Introduce 
>>> a
>>> new define and use it to avoid the opencoding in subarch_percpu_traps_init()
>>> and restore_rest_processor_state().
>>>
>>> Despite Intel hardware having an MSR_CSTAR register, nothing actually uses 
>>> it
>>> as the SYSCALL instruction raises #UD out of 64bit mode, meaning that
>> Did you mean "32-bit"?
> 
> SYSCALL on Intel will #UD if CS.L == 0, so you can't even use it from a
> compatability mode segment.

Right - so my comment stands. Or did you mean "outside of 64-bit
mode"?

Jan


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


Re: [Xen-devel] [PATCH 2/4] x86/setup: Minor cleanup to host SYSCALL MSR handling

2017-02-13 Thread Andrew Cooper
On 13/02/17 15:19, Jan Beulich wrote:
 On 13.02.17 at 15:32,  wrote:
>> Xen's choice of the MSR_STAR value is constant across all pcpus.  Introduce a
>> new define and use it to avoid the opencoding in subarch_percpu_traps_init()
>> and restore_rest_processor_state().
>>
>> Despite Intel hardware having an MSR_CSTAR register, nothing actually uses it
>> as the SYSCALL instruction raises #UD out of 64bit mode, meaning that
> Did you mean "32-bit"?

SYSCALL on Intel will #UD if CS.L == 0, so you can't even use it from a
compatability mode segment.

>
>> MSR_LSTAR is the only %rip value hardware will use.
>>
>> Therefore, only create a CSTAR trampoline stub, and save/restore the CSTAR
>> value across suspend/resume on AMD hardware.  MSR_CSTAR should now be
>> consistently 0 on Intel hardware.
> It's not documented to be zero when coming out of reset, and
> you never write zero to it.
>
> Plus, if there would be an Intel CPU supporting compat mode
> SYSCALL, we'd end up with PV guests having a way to transfer
> control to ring 0 execution at RIP 0. Sadly the instruction doesn't
> #GP(0) on a null selector found in STAR, so I think we need to
> keep storing a valid pointer into CSTAR as long as we set
> EFER.SCE (which we can't really avoid).

Ok.  All good points.  I will drop this part of the patch and keep it
set up unconditionally.  This doesn't actually affect the rest of the
series.

~Andrew

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


Re: [Xen-devel] [PATCH v2 2/2] build/printf: fix incorrect format specifiers

2017-02-13 Thread Roger Pau Monne
On Mon, Feb 13, 2017 at 07:50:07AM -0700, Jan Beulich wrote:
> >>> On 13.02.17 at 15:34,  wrote:
> > --- a/xen/arch/x86/cpu/mcheck/mce.c
> > +++ b/xen/arch/x86/cpu/mcheck/mce.c
> > @@ -595,9 +595,8 @@ int show_mca_info(int inited, struct cpuinfo_x86 *c)
> >  [mcheck_intel] = "Intel"
> >  };
> >  
> > -snprintf(prefix, ARRAY_SIZE(prefix),
> > - g_type != mcheck_unset ? XENLOG_WARNING "CPU%i: "
> > - : XENLOG_INFO,
> > +snprintf(prefix, ARRAY_SIZE(prefix), "%sCPU%i: ",
> > + g_type != mcheck_unset ? XENLOG_WARNING : XENLOG_INFO,
> >   smp_processor_id());
> 
> Should I end up committing this, I'll take the liberty of using %u
> instead of %i here.

Done, I will be sending a new version shortly.

Roger.

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


Re: [Xen-devel] [PATCH v2 1/2] xen/grants: print grant number and handle in hex format

2017-02-13 Thread Andrew Cooper
On 13/02/17 15:17, Roger Pau Monne wrote:
> On Mon, Feb 13, 2017 at 02:40:00PM +, Andrew Cooper wrote:
>> On 13/02/17 14:34, Roger Pau Monne wrote:
>>> @@ -1706,7 +1706,7 @@ gnttab_prepare_for_transfer(
>>>  if ( unlikely(ref >= nr_grant_entries(rgt)) )
>>>  {
>>>  gdprintk(XENLOG_INFO,
>>> -"Bad grant reference (%d) for transfer to domain(%d).\n",
>>> +"Bad grant reference (%#x) for transfer to domain(%d).\n",
>>>  ref, rd->domain_id);
>> We can shorten this to just d%d, matching our prevailing domain notation.
> OK, so now it's: "Bad grant reference (%#x) for transfer to d%d\n"

TBH, I'd also drop the brackets here.  They don't add anything but
length to the message.

~Andrew

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


Re: [Xen-devel] [PATCH 0/5] Improvements to HVM Hypercall dispatching

2017-02-13 Thread Wei Liu
On Mon, Feb 13, 2017 at 01:03:43PM +, Andrew Cooper wrote:
> Andrew Cooper (5):
>   x86/hvm: Rework HVM_HCALL_invalidate handling
>   x86/hvm: Split the hypercall dispatching infrastructure out of hvm.c
>   x86/hvm: Improve memory_op hypercall dispatching
>   x86/hvm: Improve grant_table_op hypercall dispatching
>   x86/hvm: Improve physdev_op hypercall dispatching

Reviewed-by: Wei Liu 

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


Re: [Xen-devel] [PATCH 2/4] x86/setup: Minor cleanup to host SYSCALL MSR handling

2017-02-13 Thread Jan Beulich
>>> On 13.02.17 at 15:32,  wrote:
> Xen's choice of the MSR_STAR value is constant across all pcpus.  Introduce a
> new define and use it to avoid the opencoding in subarch_percpu_traps_init()
> and restore_rest_processor_state().
> 
> Despite Intel hardware having an MSR_CSTAR register, nothing actually uses it
> as the SYSCALL instruction raises #UD out of 64bit mode, meaning that

Did you mean "32-bit"?

> MSR_LSTAR is the only %rip value hardware will use.
> 
> Therefore, only create a CSTAR trampoline stub, and save/restore the CSTAR
> value across suspend/resume on AMD hardware.  MSR_CSTAR should now be
> consistently 0 on Intel hardware.

It's not documented to be zero when coming out of reset, and
you never write zero to it.

Plus, if there would be an Intel CPU supporting compat mode
SYSCALL, we'd end up with PV guests having a way to transfer
control to ring 0 execution at RIP 0. Sadly the instruction doesn't
#GP(0) on a null selector found in STAR, so I think we need to
keep storing a valid pointer into CSTAR as long as we set
EFER.SCE (which we can't really avoid).

Jan


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


Re: [Xen-devel] [PATCH v2 1/2] xen/grants: print grant number and handle in hex format

2017-02-13 Thread Roger Pau Monne
On Mon, Feb 13, 2017 at 02:40:00PM +, Andrew Cooper wrote:
> On 13/02/17 14:34, Roger Pau Monne wrote:
> > Due to the large number of grants in use it seems more useful to print the
> > grant references and handlers in hex format rather than decimal.
> >
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> > ---
> >  xen/common/grant_table.c | 26 +-
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> > index d3ea805..eca07b9 100644
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -814,7 +814,7 @@ __gnttab_map_grant_ref(
> >  
> >  /* Bounds check on the grant ref */
> >  if ( unlikely(op->ref >= nr_grant_entries(rgt)))
> > -PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref (%d).\n", op->ref);
> > +PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref (%#x).\n", 
> > op->ref);
> 
> I'd drop the brackets and full stop, as you are making changes here. 
> Similarly below.

OK, I think I've fixed the occurrences of this kind of pattern in the messages
I'm touching.

> >  
> >  act = active_entry_acquire(rgt, op->ref);
> >  shah = shared_entry_header(rgt, op->ref);
> > @@ -1087,7 +1087,7 @@ __gnttab_unmap_common(
> >  
> >  if ( unlikely(op->handle >= lgt->maptrack_limit) )
> >  {
> > -gdprintk(XENLOG_INFO, "Bad handle (%d).\n", op->handle);
> > +gdprintk(XENLOG_INFO, "Bad handle (%#x).\n", op->handle);
> >  op->status = GNTST_bad_handle;
> >  return;
> >  }
> > @@ -1099,7 +1099,7 @@ __gnttab_unmap_common(
> >  if ( unlikely(!read_atomic(>map->flags)) )
> >  {
> >  grant_read_unlock(lgt);
> > -gdprintk(XENLOG_INFO, "Zero flags for handle (%d).\n", op->handle);
> > +gdprintk(XENLOG_INFO, "Zero flags for handle (%#x).\n", 
> > op->handle);
> >  op->status = GNTST_bad_handle;
> >  return;
> >  }
> > @@ -1132,7 +1132,7 @@ __gnttab_unmap_common(
> >  op->flags = read_atomic(>map->flags);
> >  if ( unlikely(!op->flags) || unlikely(op->map->domid != dom) )
> >  {
> > -gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle);
> > +gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
> >  rc = GNTST_bad_handle;
> >  goto unmap_out;
> >  }
> > @@ -1706,7 +1706,7 @@ gnttab_prepare_for_transfer(
> >  if ( unlikely(ref >= nr_grant_entries(rgt)) )
> >  {
> >  gdprintk(XENLOG_INFO,
> > -"Bad grant reference (%d) for transfer to domain(%d).\n",
> > +"Bad grant reference (%#x) for transfer to domain(%d).\n",
> >  ref, rd->domain_id);
> 
> We can shorten this to just d%d, matching our prevailing domain notation.

OK, so now it's: "Bad grant reference (%#x) for transfer to d%d\n"

> >  goto fail;
> >  }
> > @@ -2672,7 +2672,7 @@ 
> > gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
> >  break;
> >  default:
> >  gdprintk(XENLOG_INFO,
> > - "bad flags %#x in grant %u when switching 
> > version\n",
> > + "bad flags %#x in grant %#x when switching 
> > version\n",
> >   flags, i);
> >  /* fall through */
> >  case GTF_invalid:
> > @@ -2836,9 +2836,9 @@ __gnttab_swap_grant_ref(grant_ref_t ref_a, 
> > grant_ref_t ref_b)
> >  
> >  /* Bounds check on the grant refs */
> >  if ( unlikely(ref_a >= nr_grant_entries(d->grant_table)))
> > -PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-a (%d).\n", ref_a);
> > +PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-a (%#x).\n", ref_a);
> >  if ( unlikely(ref_b >= nr_grant_entries(d->grant_table)))
> > -PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-b (%d).\n", ref_b);
> > +PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-b (%#x).\n", ref_b);
> >  
> >  /* Swapping the same ref is a no-op. */
> >  if ( ref_a == ref_b )
> > @@ -2846,11 +2846,11 @@ __gnttab_swap_grant_ref(grant_ref_t ref_a, 
> > grant_ref_t ref_b)
> >  
> >  act_a = active_entry_acquire(gt, ref_a);
> >  if ( act_a->pin )
> > -PIN_FAIL(out, GNTST_eagain, "ref a %ld busy\n", (long)ref_a);
> > +PIN_FAIL(out, GNTST_eagain, "ref a %#x busy\n", ref_a);
> >  
> >  act_b = active_entry_acquire(gt, ref_b);
> >  if ( act_b->pin )
> > -PIN_FAIL(out, GNTST_eagain, "ref b %ld busy\n", (long)ref_b);
> > +PIN_FAIL(out, GNTST_eagain, "ref b %#x busy\n", 

Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling

2017-02-13 Thread Boris Ostrovsky



On 02/13/2017 08:03 AM, Andrew Cooper wrote:

Sending an invalidation to the device model is an internal detail of
completing the hypercall; callers should not need to be responsible for it.
Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
appropriate.

This makes the function boolean in nature, although the existing
HVM_HCALL_{completed,preempted} to aid code clarity.  While updating the
return type, drop _do from the name, as it is redundant.

Signed-off-by: Andrew Cooper 


Reviewed-by: Boris Ostrovsky 


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


Re: [Xen-devel] [PATCH] MAINTAINERS: drop Jinsong Liu

2017-02-13 Thread Andrew Cooper
On 13/02/17 15:03, Jan Beulich wrote:
> Mails to his listed address are bouncing.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

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


[Xen-devel] [PATCH] MAINTAINERS: drop Jinsong Liu

2017-02-13 Thread Jan Beulich
Mails to his listed address are bouncing.

Signed-off-by: Jan Beulich 

--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -272,7 +272,6 @@ F:  xen/test/livepatch/*
 
 MACHINE CHECK (MCA) & RAS
 M: Christoph Egger 
-M: Liu Jinsong 
 S: Supported
 F: xen/arch/x86/cpu/mcheck/
 
@@ -296,7 +295,6 @@ T:  git git://xenbits.xen.org/ovmf.git
 
 POWER MANAGEMENT
 M: Jan Beulich 
-M: Liu Jinsong 
 S: Supported
 F: xen/arch/x86/acpi/
 X: xen/arch/x86/acpi/boot.c




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


  1   2   >