date:20170508

There were some bits missing in docs/misc/xenstore.txt, so add them.

We might want to include this in 4.9, but I'm not feeling really
strong about this.

Juergen Gross (3):
  docs: specify endianess of xenstore protocol header
  docs: add DIRECTORY_PART specification do xenstore protocol doc
  docs: document CONTROL command of xenstore protocol

 docs/misc/xenstore.txt | 41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

-- 
2.12.0


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

[Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.

Signed-off-by: Juergen Gross 
---
 docs/misc/xenstore.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 5051340227..fd38d781e2 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -153,6 +153,15 @@ DIRECTORY  | 
|*
leafnames.  The resulting children are each named
/.
 
+DIRECTORY_PART | ||*
+   Same as DIRECTORY, but to be used for children lists longer than
+   XENSTORE_PAYLOAD_MAX. Input are  and the byte offset into
+   the list of children to return. Return values are the generation
+   count  of the node (to be used to ensure the node hasn't
+   changed between two reads:  being the same for multiple
+   reads guarantees the node hasn't changed) and the list of children
+   starting at the specified  of the complete list.
+
 GET_PERMS  | |+
 SET_PERMS  ||+?
 is one of the following
-- 
2.12.0


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

[Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.

Signed-off-by: Juergen Gross 
---
 docs/misc/xenstore.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 5051340227..fd38d781e2 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -153,6 +153,15 @@ DIRECTORY  | 
|*
leafnames.  The resulting children are each named
/.
 
+DIRECTORY_PART | ||*
+   Same as DIRECTORY, but to be used for children lists longer than
+   XENSTORE_PAYLOAD_MAX. Input are  and the byte offset into
+   the list of children to return. Return values are the generation
+   count  of the node (to be used to ensure the node hasn't
+   changed between two reads:  being the same for multiple
+   reads guarantees the node hasn't changed) and the list of children
+   starting at the specified  of the complete list.
+
 GET_PERMS  | |+
 SET_PERMS  ||+?
 is one of the following
-- 
2.12.0


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

[Xen-devel] [PATCH 3/3] docs: document CONTROL command of xenstore protocol

The CONTROL command (former DEBUG command) isn't specified in the
xenstore protocol doc. Add it.

Signed-off-by: Juergen Gross 
---
 docs/misc/xenstore.txt | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index fd38d781e2..074650f144 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -319,12 +319,29 @@ SET_TARGET||
 
 -- Miscellaneous --
 
-DEBUG  print||??   sends  to debug log
-DEBUG  print|   EINVAL
-DEBUG  check|??checks xenstored innards
-DEBUG  no-op (future extension)
+CONTROL|[|]
+   Send a control command  with optional parameters
+   () to Xenstore daemon.  support is
+   implementation specific and might change in future.
 
-   These requests should not generally be used and may be
-   withdrawn in the future.
+   Current commands are:
+   check
+   checks xenstored innards
+   log|on
+   turn xenstore logging on
+   log|off
+   turn xenstore logging off
+   logfile|
+   log to specified file
+   memreport|[]
+   print memory statistics to logfile (no 
+   specified) or to specific file
+   print|
+   print  to syslog (xenstore runs as daemon) or
+   to console (xenstore runs as stubdom)
+   help
+   return list of supported commands for CONTROL
 
+DEBUG
+   Deprecated, now named CONTROL
 
-- 
2.12.0


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

[Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

The endianess of the xenstore protocol header should be specified.

Signed-off-by: Juergen Gross 
---
 docs/misc/xenstore.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index ae1b6a8c6e..5051340227 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -46,7 +46,8 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in 
xs_wire.h.)
 Communication with xenstore is via either sockets, or event channel
 and shared memory, as specified in io/xs_wire.h: each message in
 either direction is a header formatted as a struct xsd_sockmsg
-followed by xsd_sockmsg.len bytes of payload.
+followed by xsd_sockmsg.len bytes of payload. The header fields are
+all in little endian byte order.
 
 The payload syntax varies according to the type field.  Generally
 requests each generate a reply with an identical type, req_id and
-- 
2.12.0


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

[Xen-devel] [PATCH 0/3] docs: add some missing xenstore documentation

There were some bits missing in docs/misc/xenstore.txt, so add them.

We might want to include this in 4.9, but I'm not feeling really
strong about this.

Resending with correct email address of Julien.

Juergen Gross (3):
  docs: specify endianess of xenstore protocol header
  docs: add DIRECTORY_PART specification do xenstore protocol doc
  docs: document CONTROL command of xenstore protocol

 docs/misc/xenstore.txt | 41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

-- 
2.12.0


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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

>>> On 06.05.17 at 03:51,  wrote:
>> >>> On 05.05.17 at 05:52,  wrote:
>> > 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
>> > outstanding p2m_ioreq_server entries")' will call
>> > p2m_change_entry_type_global() which set entry.recalc=1. Then
>> > the following get_entry(p2m_ioreq_server) will return
>> > p2m_ram_rw type.
>> > But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
>> > outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
>> > assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
>> > type, then reset p2m_ioreq_server entries. The fact is the assumption
>> > isn't true, and sysnchronously reset function couldn't work. Then
>> > ioreq.entry_count is larger than zero after an ioreq server unmaps,
>> > finally this results DomU couldn't reboot.
>> 
>> I've had trouble understanding this part already on v1 (btw, why is
>> this one not tagged v2?), and since I still can't figure it I have to ask:
>> Why is it that guest reboot is being impacted here? From what I recall
>> a non-zero count should only prevent migration.
> [Zhang, Xiong Y] Sorry, although they solve the same issue, the solution is
> totally different, so I didn't mark this as V2, I will mark the following
> as v2 with this solution.
> During DomU reboot, it will first unmap ioreq server in shutdown process,
> then it call map ioreq server in boot process. The following sentence in
> p2m_set_ioreq_server() result mapping ioreq server failure, then DomU
> couldn't continue booting.
> If ( read_atomic(&p->ioreq.entry_count))
>goto out;

It is clear that it would be this statement to be the problem one,
but I continue to not see why this would affect reboot: The rebooted
guest runs in another VM with, hence, a different p2m. I cannot see
why there would be a non-zero ioreq.entry_count the first time an
ioreq server claims the p2m_ioreq_server type for this new domain.

Jan


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

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

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

Regressions :-(

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

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-xsm 4 host-ping-check-native fail in 109112 pass in 
109136
 test-amd64-amd64-xl-qemut-debianhvm-amd64 15 guest-localmigrate/x10 fail in 
109112 pass in 109136
 test-amd64-i386-xl-qemut-winxpsp3 16 guest-stop  fail in 109112 pass in 109136
 test-armhf-armhf-xl-rtds  5 xen-installfail pass in 109112

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

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

>>> On 05.05.17 at 18:49,  wrote:
> On 05/05/2017 12:05 PM, Jan Beulich wrote:
> On 05.05.17 at 17:23,  wrote:
>>> On 05/05/2017 10:51 AM, Jan Beulich wrote:
>>> On 05.05.17 at 16:27,  wrote:
> On 05/05/2017 10:14 AM, Jan Beulich wrote:
> On 05.05.17 at 16:10,  wrote:
>> On 05.05.17 at 15:42,  wrote:
>>> Otoh there's not much to scrub yet until Dom0 had all its memory
>>> allocated, and we know which pages truly remain free (wanting
>>> what is currently the boot time scrubbing done on them). But that
>>> point in time may still be earlier than when we switch to
>>> SYS_STATE_active.
> IOW I think boot scrubbing could be kicked off as soon as Dom0
> had the bulk of its memory allocated.
 Since we only are trying to avoid mapcache vcpu override can't we just
 scrub whenever override is NULL (per-cpu or not)?
>>> But how do you know? The variable should remain static in
>>> domain_page.c, so I think we'd instead need a notification to
>>> the scrubber when it gets set back to NULL.
> Why not just have in domain_page.c
>
> bool mapcache_override() {return override != NULL;}
>
> (or appropriate per-cpu variant)?
 And you would mean to permanently poll this?
>>> We have to permanently poll on/check something. Either it will be local
>>> to page_alloc.c or to domain_page.c.
>> Why can't this be kicked off right after zapping the override (if we
>> go that route in the first place; I think the per-cpu override would
>> be the more seamless approach)?
> 
> Either global or per-cpu override will need to be checked by the
> scrubber when it is called from the idle loop. Or I don't understand
> what you mean by "kicking off".

I think some confusion arises from there being two different
proposals (of which I'd prefer the first):

The first is to make the override per-cpu. No kicking off is needed
in the case afaict, scrubbing can start right away if desired (albeit,
as was said, it may not make much sense prior to Dom0 having had
the bulk of its memory allocated, at least if its share isn't negligible
compared to the overall amount of memory).

The second is to leave the override as is, instead adding a call to
tell the scrubber to start doing its work. No need for any override
check in this case, as the override won't be used anymore from
the time of the mapcache_override_current(NULL) call (it is for
this reason that the kickoff call is to happen after this one).

Jan

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

Re: [Xen-devel] [PATCH v2 for-next] x86/mm: Fix the odd indentation of the pin_page block of do_mmuext_op()

>>> On 05.05.17 at 18:32,  wrote:
> The pin_page block is missing one level of indentation, which makes the
> MMUEXT_UNPIN_TABLE case label appear to be outside of the switch statement.
> 
> However, the block isn't needed at all if page is declared with switch level
> scope.  This allows for the removal of the identical local declarations for
> MMUEXT_UNPIN_TABLE, MMUEXT_NEW_USER_BASEPTR and MMUEXT_CLEAR_PAGE.
> 
> While making this adjustment, delete one other piece of trailing whitespace.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



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

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

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 3654c4623c9b37ddc64426995fa399d8626b7ced
baseline version:
 ovmf d7b96017ccf5922b798f496fbcdcac4067d04c6d

Last test of basis   109147  2017-05-08 01:18:41 Z0 days
Testing same since   109159  2017-05-08 05:47:09 Z0 days1 attempts


People who touched revisions under test:
  Jeff Fan 
  Ruiyu Ni 

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



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

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

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

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


Pushing revision :

+ branch=ovmf
+ revision=3654c4623c9b37ddc64426995fa399d8626b7ced
+ . ./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 
3654c4623c9b37ddc64426995fa399d8626b7ced
+ branch=ovmf
+ revision=3654c4623c9b37ddc64426995fa399d8626b7ced
+ . ./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
+ '[' x3654c4623c9b37ddc64426995fa399d8626b7ced = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/git/lin

[Xen-devel] [ovmf baseline-only test] 71262: tolerable trouble: blocked/broken

2017-05-08 Thread Platform Team regression test user

This run is configured for baseline tests only.

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-i386-pvops  3 host-install(3)   broken baseline untested
 build-i386-xsm3 host-install(3)   broken baseline untested
 build-amd64-xsm   3 host-install(3)   broken baseline untested
 build-i3863 host-install(3)   broken baseline untested
 build-amd64-pvops 3 host-install(3)   broken baseline untested
 build-amd64   3 host-install(3)   broken baseline untested

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

version targeted for testing:
 ovmf d7b96017ccf5922b798f496fbcdcac4067d04c6d
baseline version:
 ovmf 97cdb33b575a80ca5c20ad862331f3c6d9415575

Last test of basis71261  2017-05-07 02:16:53 Z1 days
Testing same since71262  2017-05-08 03:16:38 Z0 days1 attempts


People who touched revisions under test:
  Star Zeng 

jobs:
 build-amd64-xsm  broken  
 build-i386-xsm   broken  
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 



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

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

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

broken-step build-i386-pvops host-install(3)
broken-step build-i386-xsm host-install(3)
broken-step build-amd64-xsm host-install(3)
broken-step build-i386 host-install(3)
broken-step build-amd64-pvops host-install(3)
broken-step build-amd64 host-install(3)

Push not applicable.


commit d7b96017ccf5922b798f496fbcdcac4067d04c6d
Author: Star Zeng 
Date:   Fri May 5 16:11:57 2017 +0800

MdePkg DxeServicesLib: Handle potential NULL FvHandle

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

The FvHandle input to InternalGetSectionFromFv() may be NULL,
then ASSERT will appear. It is because the LoadedImage->DeviceHandle
returned from InternalImageHandleToFvHandle() may be NULL.
For example for DxeCore, there is LoadedImage protocol installed
for it, but the LoadedImage->DeviceHandle could not be initialized
before the FV2 (contain DxeCore) protocol is installed.

This patch is to update InternalGetSectionFromFv() to return
EFI_NOT_FOUND directly for NULL FvHandle.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Michael Turner 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Reviewed-by: Liming Gao 

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

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

2017-05-08 Thread Chao Gao

On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
 On 03.05.17 at 12:08,  wrote:
>> On 02/05/17 06:45, Chao Gao wrote:
>>> On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
 On 26/04/17 01:52, Chao Gao wrote:
> I compared the maximum of #entry in one list and #event (adding entry to
> PI blocking list) with and without the three latter patches. Here
> is the result:
> -
> |   |  ||
> |Items  |   Maximum of #entry  |  #event|
> |   |  ||
> -
> |   |  ||
> |W/ the patches | 6|   22740|
> |   |  ||
> -
> |   |  ||
> |W/O the patches|128   |   46481|
> |   |  ||
> -

 Any chance you could trace how long the list traversal took?  It would
 be good for future reference to have an idea what kinds of timescales
 we're talking about.
>>> 
>>> Hi.
>>> 
>>> I made a simple test to get the time consumed by the list traversal.
>>> Apply below patch and create one hvm guest with 128 vcpus and a passthrough 
>>> 40 NIC.
>>> All guest vcpu are pinned to one pcpu. collect data by
>>> 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
>>> xentrace_format. When the list length is about 128, the traversal time
>>> is in the range of 1750 cycles to 39330 cycles. The physical cpu's
>>> frequence is 1795.788MHz, therefore the time consumed is in the range of 1us
>>> to 22us. If 0.5ms is the upper bound the system can tolerate, at most
>>> 2900 vcpus can be added into the list.
>> 
>> Great, thanks Chao Gao, that's useful.
>
>Looks like Chao Gao has been dropped ...
>
>>  I'm not sure a fixed latency --
>> say 500us -- is the right thing to look at; if all 2900 vcpus arranged
>> to have interrupts staggered at 500us intervals it could easily lock up
>> the cpu for nearly a full second.  But I'm having trouble formulating a
>> good limit scenario.
>> 
>> In any case, 22us should be safe from a security standpoint*, and 128
>> should be pretty safe from a "make the common case fast" standpoint:
>> i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
>> traffic will be the least of your performance problems I should think.
>> 
>>  -George
>> 
>> * Waiting for Jan to contradict me on this one. :-)
>
>22us would certainly be fine, if this was the worst case scenario.
>I'm not sure the value measured for 128 list entries can be easily
>scaled to several thousands of them, due cache and/or NUMA
>effects. I continue to think that we primarily need theoretical
>proof of an upper boundary on list length being enforced, rather
>than any measurements or randomized balancing. And just to be
>clear - if someone overloads their system, I do not see a need to
>have a guaranteed maximum list traversal latency here. All I ask
>for is that list traversal time scales with total vCPU count divided
>by pCPU count.

Thanks, Jan & George.

I think it is more clear to me about what should I do next step.

In my understanding, we should distribute the wakeup interrupts like
this:
1. By default, distribute it to the local pCPU ('local' means the vCPU
is on the pCPU) to make the common case fast.
2. With the list grows to a point where we think it may consumers too
much time to traverse the list, also distribute wakeup interrupt to local
pCPU, ignoring admin intentionally overloads their system.
3. When the list length reachs the theoretic average maximum (means
maximal vCPU count divided by pCPU count), distribute wakeup interrupt
to another underutilized pCPU.

But, I am confused about that If we don't care someone overload their
system, why we need the stage #3?  If not, I have no idea to meet Jan's
request, the list traversal time scales with total vCPU count divided by
pCPU count. Or we will reach stage #3 before stage #2?

Thanks
Chao

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

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

>>> On 08.05.17 at 18:15,  wrote:
> On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
> On 03.05.17 at 12:08,  wrote:
>>> On 02/05/17 06:45, Chao Gao wrote:
 On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
> On 26/04/17 01:52, Chao Gao wrote:
>> I compared the maximum of #entry in one list and #event (adding entry to
>> PI blocking list) with and without the three latter patches. Here
>> is the result:
>> -
>> |   |  ||
>> |Items  |   Maximum of #entry  |  #event|
>> |   |  ||
>> -
>> |   |  ||
>> |W/ the patches | 6|   22740|
>> |   |  ||
>> -
>> |   |  ||
>> |W/O the patches|128   |   46481|
>> |   |  ||
>> -
>
> Any chance you could trace how long the list traversal took?  It would
> be good for future reference to have an idea what kinds of timescales
> we're talking about.
 
 Hi.
 
 I made a simple test to get the time consumed by the list traversal.
 Apply below patch and create one hvm guest with 128 vcpus and a 
 passthrough 
> 40 NIC.
 All guest vcpu are pinned to one pcpu. collect data by
 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
 xentrace_format. When the list length is about 128, the traversal time
 is in the range of 1750 cycles to 39330 cycles. The physical cpu's
 frequence is 1795.788MHz, therefore the time consumed is in the range of 
 1us
 to 22us. If 0.5ms is the upper bound the system can tolerate, at most
 2900 vcpus can be added into the list.
>>> 
>>> Great, thanks Chao Gao, that's useful.
>>
>>Looks like Chao Gao has been dropped ...
>>
>>>  I'm not sure a fixed latency --
>>> say 500us -- is the right thing to look at; if all 2900 vcpus arranged
>>> to have interrupts staggered at 500us intervals it could easily lock up
>>> the cpu for nearly a full second.  But I'm having trouble formulating a
>>> good limit scenario.
>>> 
>>> In any case, 22us should be safe from a security standpoint*, and 128
>>> should be pretty safe from a "make the common case fast" standpoint:
>>> i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
>>> traffic will be the least of your performance problems I should think.
>>> 
>>>  -George
>>> 
>>> * Waiting for Jan to contradict me on this one. :-)
>>
>>22us would certainly be fine, if this was the worst case scenario.
>>I'm not sure the value measured for 128 list entries can be easily
>>scaled to several thousands of them, due cache and/or NUMA
>>effects. I continue to think that we primarily need theoretical
>>proof of an upper boundary on list length being enforced, rather
>>than any measurements or randomized balancing. And just to be
>>clear - if someone overloads their system, I do not see a need to
>>have a guaranteed maximum list traversal latency here. All I ask
>>for is that list traversal time scales with total vCPU count divided
>>by pCPU count.
> 
> Thanks, Jan & George.
> 
> I think it is more clear to me about what should I do next step.
> 
> In my understanding, we should distribute the wakeup interrupts like
> this:
> 1. By default, distribute it to the local pCPU ('local' means the vCPU
> is on the pCPU) to make the common case fast.
> 2. With the list grows to a point where we think it may consumers too
> much time to traverse the list, also distribute wakeup interrupt to local
> pCPU, ignoring admin intentionally overloads their system.
> 3. When the list length reachs the theoretic average maximum (means
> maximal vCPU count divided by pCPU count), distribute wakeup interrupt
> to another underutilized pCPU.
> 
> But, I am confused about that If we don't care someone overload their
> system, why we need the stage #3?  If not, I have no idea to meet Jan's
> request, the list traversal time scales with total vCPU count divided by
> pCPU count. Or we will reach stage #3 before stage #2?

Things is that imo point 2 is too fuzzy to be of any use, i.e. 3 should
take effect immediately. We don't mean to ignore any admin decisions
here, it is just that if they overload their systems, the net effect of 3
may still not be good enough to provide smooth behavior. But that's
then a result of them overloading their systems in the first place. IOW,
you should try to evenly distribute vCPU-s as soon as their count on
a given pC

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

2017-05-08 Thread Chao Gao

On Mon, May 08, 2017 at 02:39:25AM -0600, Jan Beulich wrote:
 On 08.05.17 at 18:15,  wrote:
>> On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
>> On 03.05.17 at 12:08,  wrote:
 On 02/05/17 06:45, Chao Gao wrote:
> On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
>> On 26/04/17 01:52, Chao Gao wrote:
>>> I compared the maximum of #entry in one list and #event (adding entry to
>>> PI blocking list) with and without the three latter patches. Here
>>> is the result:
>>> -
>>> |   |  ||
>>> |Items  |   Maximum of #entry  |  #event|
>>> |   |  ||
>>> -
>>> |   |  ||
>>> |W/ the patches | 6|   22740|
>>> |   |  ||
>>> -
>>> |   |  ||
>>> |W/O the patches|128   |   46481|
>>> |   |  ||
>>> -
>>
>> Any chance you could trace how long the list traversal took?  It would
>> be good for future reference to have an idea what kinds of timescales
>> we're talking about.
> 
> Hi.
> 
> I made a simple test to get the time consumed by the list traversal.
> Apply below patch and create one hvm guest with 128 vcpus and a 
> passthrough 
>> 40 NIC.
> All guest vcpu are pinned to one pcpu. collect data by
> 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
> xentrace_format. When the list length is about 128, the traversal time
> is in the range of 1750 cycles to 39330 cycles. The physical cpu's
> frequence is 1795.788MHz, therefore the time consumed is in the range of 
> 1us
> to 22us. If 0.5ms is the upper bound the system can tolerate, at most
> 2900 vcpus can be added into the list.
 
 Great, thanks Chao Gao, that's useful.
>>>
>>>Looks like Chao Gao has been dropped ...
>>>
  I'm not sure a fixed latency --
 say 500us -- is the right thing to look at; if all 2900 vcpus arranged
 to have interrupts staggered at 500us intervals it could easily lock up
 the cpu for nearly a full second.  But I'm having trouble formulating a
 good limit scenario.
 
 In any case, 22us should be safe from a security standpoint*, and 128
 should be pretty safe from a "make the common case fast" standpoint:
 i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
 traffic will be the least of your performance problems I should think.
 
  -George
 
 * Waiting for Jan to contradict me on this one. :-)
>>>
>>>22us would certainly be fine, if this was the worst case scenario.
>>>I'm not sure the value measured for 128 list entries can be easily
>>>scaled to several thousands of them, due cache and/or NUMA
>>>effects. I continue to think that we primarily need theoretical
>>>proof of an upper boundary on list length being enforced, rather
>>>than any measurements or randomized balancing. And just to be
>>>clear - if someone overloads their system, I do not see a need to
>>>have a guaranteed maximum list traversal latency here. All I ask
>>>for is that list traversal time scales with total vCPU count divided
>>>by pCPU count.
>> 
>> Thanks, Jan & George.
>> 
>> I think it is more clear to me about what should I do next step.
>> 
>> In my understanding, we should distribute the wakeup interrupts like
>> this:
>> 1. By default, distribute it to the local pCPU ('local' means the vCPU
>> is on the pCPU) to make the common case fast.
>> 2. With the list grows to a point where we think it may consumers too
>> much time to traverse the list, also distribute wakeup interrupt to local
>> pCPU, ignoring admin intentionally overloads their system.
>> 3. When the list length reachs the theoretic average maximum (means
>> maximal vCPU count divided by pCPU count), distribute wakeup interrupt
>> to another underutilized pCPU.
>> 
>> But, I am confused about that If we don't care someone overload their
>> system, why we need the stage #3?  If not, I have no idea to meet Jan's
>> request, the list traversal time scales with total vCPU count divided by
>> pCPU count. Or we will reach stage #3 before stage #2?
>
>Things is that imo point 2 is too fuzzy to be of any use, i.e. 3 should
>take effect immediately. We don't mean to ignore any admin decisions
>here, it is just that if they overload their systems, the net effect of 3
>may still not be good enough to provide smooth behavior. But th

Re: [Xen-devel] null domains after xl destroy

On 04/05/17 00:17, Glenn Enright wrote:
> On 04/05/17 04:58, Steven Haigh wrote:
>> On 04/05/17 01:53, Juergen Gross wrote:
>>> On 03/05/17 12:45, Steven Haigh wrote:
 Just wanted to give this a little nudge now people seem to be back on
 deck...
>>>
>>> Glenn, could you please give the attached patch a try?
>>>
>>> It should be applied on top of the other correction, the old debug
>>> patch should not be applied.
>>>
>>> I have added some debug output to make sure we see what is happening.
>>
>> This patch is included in kernel-xen-4.9.26-1
>>
>> It should be in the repos now.
>>
> 
> Still seeing the same issue. Without the extra debug patch all I see in
> the logs after destroy is this...
> 
> xen-blkback: xen_blkif_disconnect: busy
> xen-blkback: xen_blkif_free: delayed = 0

Hmm, to me it seems as if some grant isn't being unmapped.

Looking at gnttab_unmap_refs_async() I wonder how this is supposed to
work:

I don't see how a grant would ever be unmapped in case of
page_count(item->pages[pc]) > 1 in __gnttab_unmap_refs_async(). All it
does is deferring the call to the unmap operation again and again. Or
am I missing something here?


Juergen

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

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

On 08/05/17 17:15, Chao Gao wrote:
> On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
> On 03.05.17 at 12:08,  wrote:
>>> On 02/05/17 06:45, Chao Gao wrote:
 On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
> On 26/04/17 01:52, Chao Gao wrote:
>> I compared the maximum of #entry in one list and #event (adding entry to
>> PI blocking list) with and without the three latter patches. Here
>> is the result:
>> -
>> |   |  ||
>> |Items  |   Maximum of #entry  |  #event|
>> |   |  ||
>> -
>> |   |  ||
>> |W/ the patches | 6|   22740|
>> |   |  ||
>> -
>> |   |  ||
>> |W/O the patches|128   |   46481|
>> |   |  ||
>> -
>
> Any chance you could trace how long the list traversal took?  It would
> be good for future reference to have an idea what kinds of timescales
> we're talking about.

 Hi.

 I made a simple test to get the time consumed by the list traversal.
 Apply below patch and create one hvm guest with 128 vcpus and a 
 passthrough 40 NIC.
 All guest vcpu are pinned to one pcpu. collect data by
 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
 xentrace_format. When the list length is about 128, the traversal time
 is in the range of 1750 cycles to 39330 cycles. The physical cpu's
 frequence is 1795.788MHz, therefore the time consumed is in the range of 
 1us
 to 22us. If 0.5ms is the upper bound the system can tolerate, at most
 2900 vcpus can be added into the list.
>>>
>>> Great, thanks Chao Gao, that's useful.
>>
>> Looks like Chao Gao has been dropped ...
>>
>>>  I'm not sure a fixed latency --
>>> say 500us -- is the right thing to look at; if all 2900 vcpus arranged
>>> to have interrupts staggered at 500us intervals it could easily lock up
>>> the cpu for nearly a full second.  But I'm having trouble formulating a
>>> good limit scenario.
>>>
>>> In any case, 22us should be safe from a security standpoint*, and 128
>>> should be pretty safe from a "make the common case fast" standpoint:
>>> i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
>>> traffic will be the least of your performance problems I should think.
>>>
>>>  -George
>>>
>>> * Waiting for Jan to contradict me on this one. :-)
>>
>> 22us would certainly be fine, if this was the worst case scenario.
>> I'm not sure the value measured for 128 list entries can be easily
>> scaled to several thousands of them, due cache and/or NUMA
>> effects. I continue to think that we primarily need theoretical
>> proof of an upper boundary on list length being enforced, rather
>> than any measurements or randomized balancing. And just to be
>> clear - if someone overloads their system, I do not see a need to
>> have a guaranteed maximum list traversal latency here. All I ask
>> for is that list traversal time scales with total vCPU count divided
>> by pCPU count.
> 
> Thanks, Jan & George.
> 
> I think it is more clear to me about what should I do next step.
> 
> In my understanding, we should distribute the wakeup interrupts like
> this:
> 1. By default, distribute it to the local pCPU ('local' means the vCPU
> is on the pCPU) to make the common case fast.
> 2. With the list grows to a point where we think it may consumers too
> much time to traverse the list, also distribute wakeup interrupt to local
> pCPU, ignoring admin intentionally overloads their system.
> 3. When the list length reachs the theoretic average maximum (means
> maximal vCPU count divided by pCPU count), distribute wakeup interrupt
> to another underutilized pCPU.

By "maximal vCPU count" do you mean, "total number of active vcpus on
the system"?  Or some other theoretical maximum vcpu count (e.g., 32k
domans * 512 vcpus each or something)?

What about saying that the limit of vcpus for any given pcpu will be:
 (v_tot / p_tot) + K
where v_tot is the total number of vcpus on the system, p_tot is the
total number of pcpus in the system, and K is a fixed number (such as
128) such that 1) the additional time walking the list is minimal, and
2) in the common case we should never come close to reaching that number?

Then the algorithm for choosing which pcpu to have the interrupt
delivered to would be:
 1. Set p = current_pcpu
 2. if len(list(p)) < v_tot / p_tot + k, cho

Re: [Xen-devel] [RFC PATCH 01/10] ARM: vGIC: remove rank lock from IRQ routing functions

2017-05-08 Thread Andre Przywara

Hi,

On 04/05/17 16:53, Julien Grall wrote:
> Hi Andre,
> 
> On 04/05/17 16:31, Andre Przywara wrote:
>> gic_route_irq_to_guest() and gic_remove_irq_from_guest() take the rank
>> lock, however never actually access the rank structure.
>> Avoid taking the lock in those two functions and remove some more
>> unneeded code on the way.
> 
> The rank is here to protect p->desc when checking that the virtual
> interrupt was not yet routed to another physical interrupt.

Really? To me that sounds quite surprising.
My understanding is that the VGIC VCPU lock protected the pending_irq
(and thus the desc pointer?) so far, and the desc itself has its own lock.
According to the comment in the struct irq_rank declaration the lock
protects the members of this struct only.

Looking briefly at users of pending_irq->desc (for instance
gicv[23]_update_lr() or gic_update_one_lr()) I can't see any hint that
they care about the lock.

So should that be fixed or at least documented?

> Without this locking, you can have two concurrent call of
> gic_route_irq_to_guest that will update the same virtual interrupt but
> with different physical interrupts.
> 
> You would have to replace the rank lock by the per-pending_irq lock to
> keep the code safe.

That indeed sounds reasonable.

Cheers,
Andre.

>>
>> Signed-off-by: Andre Przywara 
>> ---
>>  xen/arch/arm/gic.c | 28 
>>  1 file changed, 4 insertions(+), 24 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index da19130..c734f66 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -136,25 +136,19 @@ void gic_route_irq_to_xen(struct irq_desc *desc,
>> unsigned int priority)
>>  int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>> struct irq_desc *desc, unsigned int priority)
>>  {
>> -unsigned long flags;
>>  /* Use vcpu0 to retrieve the pending_irq struct. Given that we only
>>   * route SPIs to guests, it doesn't make any difference. */
>> -struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
>> -struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
>> -struct pending_irq *p = irq_to_pending(v_target, virq);
>> -int res = -EBUSY;
>> +struct pending_irq *p = irq_to_pending(d->vcpu[0], virq);
>>
>>  ASSERT(spin_is_locked(&desc->lock));
>>  /* Caller has already checked that the IRQ is an SPI */
>>  ASSERT(virq >= 32);
>>  ASSERT(virq < vgic_num_irqs(d));
>>
>> -vgic_lock_rank(v_target, rank, flags);
>> -
>>  if ( p->desc ||
>>   /* The VIRQ should not be already enabled by the guest */
>>   test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
>> -goto out;
>> +return -EBUSY;
>>
>>  desc->handler = gic_hw_ops->gic_guest_irq_type;
>>  set_bit(_IRQ_GUEST, &desc->status);
>> @@ -164,29 +158,20 @@ int gic_route_irq_to_guest(struct domain *d,
>> unsigned int virq,
>>  gic_set_irq_priority(desc, priority);
>>
>>  p->desc = desc;
>> -res = 0;
>>
>> -out:
>> -vgic_unlock_rank(v_target, rank, flags);
>> -
>> -return res;
>> +return 0;
>>  }
>>
>>  /* This function only works with SPIs for now */
>>  int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
>>struct irq_desc *desc)
>>  {
>> -struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
>> -struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
>> -struct pending_irq *p = irq_to_pending(v_target, virq);
>> -unsigned long flags;
>> +struct pending_irq *p = irq_to_pending(d->vcpu[0], virq);
>>
>>  ASSERT(spin_is_locked(&desc->lock));
>>  ASSERT(test_bit(_IRQ_GUEST, &desc->status));
>>  ASSERT(p->desc == desc);
>>
>> -vgic_lock_rank(v_target, rank, flags);
>> -
>>  if ( d->is_dying )
>>  {
>>  desc->handler->shutdown(desc);
>> @@ -204,10 +189,7 @@ int gic_remove_irq_from_guest(struct domain *d,
>> unsigned int virq,
>>   */
>>  if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
>>   !test_bit(_IRQ_DISABLED, &desc->status) )
>> -{
>> -vgic_unlock_rank(v_target, rank, flags);
>>  return -EBUSY;
>> -}
>>  }
>>
>>  clear_bit(_IRQ_GUEST, &desc->status);
>> @@ -215,8 +197,6 @@ int gic_remove_irq_from_guest(struct domain *d,
>> unsigned int virq,
>>
>>  p->desc = NULL;
>>
>> -vgic_unlock_rank(v_target, rank, flags);
>> -
>>  return 0;
>>  }
>>
>>
> 

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

Re: [Xen-devel] [RFC PATCH 4/4] arm/mem_access: Add software guest-page-table walk

2017-05-08 Thread Sergej Proskurin

Hi Julien,


On 05/02/2017 05:17 PM, Julien Grall wrote:
> Hi Sergej,
>
> On 30/04/17 20:48, Sergej Proskurin wrote:
>> The function p2m_mem_access_check_and_get_page in mem_access.c
>> translates a gva to an ipa by means of the hardware functionality
>> implemented in the function gva_to_ipa. If mem_access is active,
>> hardware-based gva to ipa translation might fail, as gva_to_ipa uses the
>> guest's translation tables, access to which might be restricted by the
>> active VTTBR. To address this issue, we perform the gva to ipa
>> translation in software.
>>
>> Signed-off-by: Sergej Proskurin 
>> ---
>> Cc: Razvan Cojocaru 
>> Cc: Tamas K Lengyel 
>> Cc: Stefano Stabellini 
>> Cc: Julien Grall 
>> ---
>>  xen/arch/arm/mem_access.c | 140
>> +-
>>  1 file changed, 139 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
>> index 04b1506b00..352eb6073f 100644
>> --- a/xen/arch/arm/mem_access.c
>> +++ b/xen/arch/arm/mem_access.c
>> @@ -20,6 +20,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -90,6 +91,129 @@ static int __p2m_get_mem_access(struct domain *d,
>> gfn_t gfn,
>>  return 0;
>>  }
>>
>> +static int
>> +p2m_gva_to_ipa(struct p2m_domain *p2m, vaddr_t gva,
>> +   paddr_t *ipa, unsigned int flags)
>
> I don't think this function belongs to mem_access.c. It would be
> better in p2m.c. Also, the name is a bit confusing, a user would not
> know the difference between p2m_gva_to_ipa and gva_to_ipa.

Fair enough, thank you.

>
>> +{
>> +int level=0, t0_sz, t1_sz;
>
> Coding style level = 0.
>
> Also please use unsigned int for level.
>
> t0_sz and t1_sz should be stay signed.
>
>> +unsigned long t0_max, t1_min;
>> +lpae_t pte, *table;
>> +mfn_t root_mfn;
>> +uint64_t ttbr;
>> +uint32_t sctlr = READ_SYSREG(SCTLR_EL1);
>> +register_t ttbcr = READ_SYSREG(TCR_EL1);
>
> So you are assuming that the current vCPU is running, right? If so,
> this should be clarified in the commit message and in a comment above
> the function.

I will state that in a comment, thank you.

>
> Also s/ttbcr/tcr/
>
>> +struct domain *d = p2m->domain;
>> +
>> +const unsigned int offsets[4] = {
>> +#ifdef CONFIG_ARM_64
>> +zeroeth_table_offset(gva),
>> +#endif
>> +first_table_offset(gva),
>> +second_table_offset(gva),
>> +third_table_offset(gva)
>> +};
>
> Offsets are based on the granularity of Xen (currently 4KB). However
> the guests can support 4KB, 16KB, 64KB. Please handle everything
> correctly.

True, ARM ist quite flexible. Yet, I have not seen an OS implementation
that is supported by Xen and makes use of page sizes other than 4KB and
their supersets (2/4MB, 1GB) (please let me know, if you know some),
which is why I doubt that we need that. Please let me know why you think
we need that kind of flexibility in software?

If you should nevertheless insist on that functionality, I would include
it in the next patch and try not to blow up the code too much.

>
>> +
>> +const paddr_t masks[4] = {
>> +#ifdef CONFIG_ARM_64
>> +ZEROETH_SIZE - 1,
>> +#endif
>> +FIRST_SIZE - 1,
>> +SECOND_SIZE - 1,
>> +THIRD_SIZE - 1
>> +};
>> +
>> +/* If the MMU is disabled, there is no need to translate the
>> gva. */
>> +if ( !(sctlr & SCTLR_M) )
>> +{
>> +*ipa = gva;
>> +
>> +return 0;
>> +}
>> +
>> +if ( is_32bit_domain(d) )
>> +{
>> +/*
>> + * XXX: We do not support 32-bit domain translation table
>> walks for
>> + * domains using the short-descriptor translation table
>> format, yet.
>> + */
>
> Debian ARM 32bit is using short-descriptor translation table. So are
> you suggesting that memaccess will not correctly with Debian guest?
>

Yes, as stated in the comment, the current implementation does not
support the short-descriptor translation table format. As this is an RFC
patch, I wanted to see your opinion on that functionality in general
before implementing all corner cases of the ARM architecture.

As mentioned in my previous reply in patch (patch 2/4), I would prefer
to separate the long-descriptor from the short-descriptor translation in
the future implementation.

>> +if ( !(ttbcr & TTBCR_EAE) )
>
> See my comment on patch #2 about the naming convention.
>

Done, thanks.

>> +return -EFAULT;
>> +
>> +#ifdef CONFIG_ARM_64
>> +level = 1;
>
> This sounds wrong to me. The first lookup level is determined by the
> value of T0SZ and T1SZ (see B3-1352 in ARM DDI 0406C.c).
>
> For the AArch64 version see D4.2.5 in ARM DDI 0487A.k_iss10775.
>

Thank you for the hint.

>> +#endif
>> +}
>> +
>> +#ifdef CONFIG_ARM_64
>> +if ( is_64bit_domain(d) )
>> +{
>> +/* Get the max GVA that can be translated by TTBR0. */
>> +t0_sz = (ttbcr >> TCR_T0SZ_SHIFT) & TCR_SZ_MASK;

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

(Chao Gao got lost from the recipients list again; re-adding)

>>> On 08.05.17 at 11:13,  wrote:
> On 08/05/17 17:15, Chao Gao wrote:
>> On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
>> On 03.05.17 at 12:08,  wrote:
 On 02/05/17 06:45, Chao Gao wrote:
> On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
>> On 26/04/17 01:52, Chao Gao wrote:
>>> I compared the maximum of #entry in one list and #event (adding entry to
>>> PI blocking list) with and without the three latter patches. Here
>>> is the result:
>>> -
>>> |   |  ||
>>> |Items  |   Maximum of #entry  |  #event|
>>> |   |  ||
>>> -
>>> |   |  ||
>>> |W/ the patches | 6|   22740|
>>> |   |  ||
>>> -
>>> |   |  ||
>>> |W/O the patches|128   |   46481|
>>> |   |  ||
>>> -
>>
>> Any chance you could trace how long the list traversal took?  It would
>> be good for future reference to have an idea what kinds of timescales
>> we're talking about.
>
> Hi.
>
> I made a simple test to get the time consumed by the list traversal.
> Apply below patch and create one hvm guest with 128 vcpus and a 
> passthrough 
> 40 NIC.
> All guest vcpu are pinned to one pcpu. collect data by
> 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
> xentrace_format. When the list length is about 128, the traversal time
> is in the range of 1750 cycles to 39330 cycles. The physical cpu's
> frequence is 1795.788MHz, therefore the time consumed is in the range of 
> 1us
> to 22us. If 0.5ms is the upper bound the system can tolerate, at most
> 2900 vcpus can be added into the list.

 Great, thanks Chao Gao, that's useful.
>>>
>>> Looks like Chao Gao has been dropped ...
>>>
  I'm not sure a fixed latency --
 say 500us -- is the right thing to look at; if all 2900 vcpus arranged
 to have interrupts staggered at 500us intervals it could easily lock up
 the cpu for nearly a full second.  But I'm having trouble formulating a
 good limit scenario.

 In any case, 22us should be safe from a security standpoint*, and 128
 should be pretty safe from a "make the common case fast" standpoint:
 i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
 traffic will be the least of your performance problems I should think.

  -George

 * Waiting for Jan to contradict me on this one. :-)
>>>
>>> 22us would certainly be fine, if this was the worst case scenario.
>>> I'm not sure the value measured for 128 list entries can be easily
>>> scaled to several thousands of them, due cache and/or NUMA
>>> effects. I continue to think that we primarily need theoretical
>>> proof of an upper boundary on list length being enforced, rather
>>> than any measurements or randomized balancing. And just to be
>>> clear - if someone overloads their system, I do not see a need to
>>> have a guaranteed maximum list traversal latency here. All I ask
>>> for is that list traversal time scales with total vCPU count divided
>>> by pCPU count.
>> 
>> Thanks, Jan & George.
>> 
>> I think it is more clear to me about what should I do next step.
>> 
>> In my understanding, we should distribute the wakeup interrupts like
>> this:
>> 1. By default, distribute it to the local pCPU ('local' means the vCPU
>> is on the pCPU) to make the common case fast.
>> 2. With the list grows to a point where we think it may consumers too
>> much time to traverse the list, also distribute wakeup interrupt to local
>> pCPU, ignoring admin intentionally overloads their system.
>> 3. When the list length reachs the theoretic average maximum (means
>> maximal vCPU count divided by pCPU count), distribute wakeup interrupt
>> to another underutilized pCPU.
> 
> By "maximal vCPU count" do you mean, "total number of active vcpus on
> the system"?  Or some other theoretical maximum vcpu count (e.g., 32k
> domans * 512 vcpus each or something)?

The former.

> What about saying that the limit of vcpus for any given pcpu will be:
>  (v_tot / p_tot) + K
> where v_tot is the total number of vcpus on the system, p_tot is the
> total number of pcpus in the system, and K is a fixed number (such as
> 128) such that 1) the additional time walking the list is minimal, and
> 2) in the com

[Xen-devel] Xen Project Developer Summit Design Sessions

Dear community members (I CC'ed people who submitted design sessions so far)

I got a few questions regarding "acceptance" and scheduling of Design Sessions 
for the Developer and Design Summit. As you may recall, the Developer and 
Design Summit mixes the formats of past Summits and Hackathons. The afternoons 
are reserved for Design sessions, which will follow the format of Hackathons.

Thus, in the tradition of the Xen Project Hackathons, we will not run Design 
Sessions through the Program Management Committee, as long as we have enough 
space to host sessions. The submission system is still open for Design Sessions 
at 
http://events.linuxfoundation.org/events/xen-developer-and-design-summit/program/cfp
 

 (although the website says otherwise, which I will get fixed) and sessions can 
be submitted until Friday 7th of July. Sessions can still be proposed on the 
day of the event (using A5 post-it-notes) as in the past. 

Whenever new sessions come in, I will once a week put them live the afternoon 
Design Sessions on the summit schedule, using the following criteria
Schedule sessions on a day which has similar topics to the ones already 
proposed (makes the live scheduling sessions at the event easier)
Schedule sessions on (or after) a day where related talks take place (creates 
more valuable discussions)
Schedule such that people which have multiple sessions get some breathing space 
(aka avoid that everyone has all their sessions on one day)

Note that we will have room rooms with a projector, others won't. 
If you submit design sessions and you need a projector, please add this under 
"List any technical requirements that you have for your presentation over and 
above the standard projector, screen and wireless internet" 
If you need to have your session on a specific day due to travel constraints, 
please add this under "List any technical requirements..."
The purpose of publishing design sessions as they come in is twofold: 
let you know what others are planning to do (avoids duplicate sessions)
marketing (attracts attendees)
You can propose new design sessions every day of the event, but we prefer if 
you proposed them using the submission system. 

As in the past at Hackathons we will try and balance the schedule and merge 
related sessions as a group in a 30 minute scheduling session.  I am also happy 
getting feedback from community members on this thread: to facilitate this, I 
will post schedule updates every other week or so. 

As we have Linux Foundation staff on hand, we should also be able to update the 
on-line schedule immediately afterwards. But we will also have a paper schedule 
(using post-it notes), as it is easier to manage the scheduling session.

I attached the schedule for Design sessions as we will publish by the end of 
this week

Legend  Additional Notes
AV, Projector, Podium, Recording
AV, Projector, Podium, NO Recording SCHEDULING sessions
AV, Projector, Podium, Recording if we can afford itIf not, we may have to 
drop or maybe find a vendor to do a special sponsorship
Projector only, Flipchart or whiteboard 
Flipchart or whiteboard, NO AV  

Time / Duration July 11
14:00 - 14:30   ANNOUNCEMENT OF NEW DESIGN SESSIONS, SCHEDULING OF DESIGN 
SESSIONS
Tas Huba/TohotomDery/Jokai  Krudy/Arany Mikszath/Petrofi
Valletta II
14:30 - 15:15   45 min Design   45 min Design   45 min Design   EFI secure boot 
+ shim + GRUB2 + Xen - how to make it work - Daniel Kiper - Oracle  45 min 
Design   Contributing to Xen: An Introduction - George Dunlap - Citrix
15:20 - 16:05   45 min Design   45 min Design   45 min Design   EFI + Intel TXT 
and TPM + Xen/Linux - how to make it work - Daniel Kiper - Oracle   
PV-IOMMU - Paul Durrant - Citrix45 min Design
16:05 - 16:35   BREAK - Foyer
16:35 - 17:35   60 min Design   60 min Design   60 min Design   60 min Design   
Graphics Virtualization - Rich Persaud - BAE Systems60 min Design

Time / Duration July 12
14:00 - 14:30   ANNOUNCEMENT OF NEW DESIGN SESSIONS, SCHEDULING OF DESIGN 
SESSIONS
Tas Huba/TohotomDery/Jokai  Krudy/Arany Mikszath/Petrofi
Valletta II
14:30 - 15:15   45 min Design   45 min Design   45 min Design   45 min Design   
Improvements to Continuous Integration Workflow - Doug Goldstein - Star Lab 
45 min Design
15:20 - 16:05   45 min Design   45 min Design   45 min Design   45 min Design   
Default Tests and Configuration of Server and Edge Hypervisors - Rich Persaud - 
BAE Systems 45 min Design
16:05 - 16:35   BREAK - Foyer
16:35 - 17:35   60 min Design   60 min Design   60 min Design   60 min Design   
Unikernel support for NFV-like applications on Xen ARM 64bit - Anastassios 
Nanos - OnAppZerocopy on Xen PV drivers - João Martins - Oracle

Time / Duration July 13
14:00 - 14:30   ANNOUNCEMENT OF NEW DESIGN SESSIONS, SCHEDULING OF DESIGN 
SESSIONS
T

Re: [Xen-devel] [PATCH 0/4] mitigate the per-pCPU blocking list may be too long

2017-05-08 Thread Chao Gao

On Mon, May 08, 2017 at 03:24:47AM -0600, Jan Beulich wrote:
>(Chao Gao got lost from the recipients list again; re-adding)
>
 On 08.05.17 at 11:13,  wrote:
>> On 08/05/17 17:15, Chao Gao wrote:
>>> On Wed, May 03, 2017 at 04:21:27AM -0600, Jan Beulich wrote:
>>> On 03.05.17 at 12:08,  wrote:
> On 02/05/17 06:45, Chao Gao wrote:
>> On Wed, Apr 26, 2017 at 05:39:57PM +0100, George Dunlap wrote:
>>> On 26/04/17 01:52, Chao Gao wrote:
 I compared the maximum of #entry in one list and #event (adding entry 
 to
 PI blocking list) with and without the three latter patches. Here
 is the result:
 -
 |   |  ||
 |Items  |   Maximum of #entry  |  #event|
 |   |  ||
 -
 |   |  ||
 |W/ the patches | 6|   22740|
 |   |  ||
 -
 |   |  ||
 |W/O the patches|128   |   46481|
 |   |  ||
 -
>>>
>>> Any chance you could trace how long the list traversal took?  It would
>>> be good for future reference to have an idea what kinds of timescales
>>> we're talking about.
>>
>> Hi.
>>
>> I made a simple test to get the time consumed by the list traversal.
>> Apply below patch and create one hvm guest with 128 vcpus and a 
>> passthrough 
>> 40 NIC.
>> All guest vcpu are pinned to one pcpu. collect data by
>> 'xentrace -D -e 0x82000 -T 300 trace.bin' and decode data by
>> xentrace_format. When the list length is about 128, the traversal time
>> is in the range of 1750 cycles to 39330 cycles. The physical cpu's
>> frequence is 1795.788MHz, therefore the time consumed is in the range of 
>> 1us
>> to 22us. If 0.5ms is the upper bound the system can tolerate, at most
>> 2900 vcpus can be added into the list.
>
> Great, thanks Chao Gao, that's useful.

 Looks like Chao Gao has been dropped ...

>  I'm not sure a fixed latency --
> say 500us -- is the right thing to look at; if all 2900 vcpus arranged
> to have interrupts staggered at 500us intervals it could easily lock up
> the cpu for nearly a full second.  But I'm having trouble formulating a
> good limit scenario.
>
> In any case, 22us should be safe from a security standpoint*, and 128
> should be pretty safe from a "make the common case fast" standpoint:
> i.e., if you have 128 vcpus on a single runqueue, the IPI wake-up
> traffic will be the least of your performance problems I should think.
>
>  -George
>
> * Waiting for Jan to contradict me on this one. :-)

 22us would certainly be fine, if this was the worst case scenario.
 I'm not sure the value measured for 128 list entries can be easily
 scaled to several thousands of them, due cache and/or NUMA
 effects. I continue to think that we primarily need theoretical
 proof of an upper boundary on list length being enforced, rather
 than any measurements or randomized balancing. And just to be
 clear - if someone overloads their system, I do not see a need to
 have a guaranteed maximum list traversal latency here. All I ask
 for is that list traversal time scales with total vCPU count divided
 by pCPU count.
>>> 
>>> Thanks, Jan & George.
>>> 
>>> I think it is more clear to me about what should I do next step.
>>> 
>>> In my understanding, we should distribute the wakeup interrupts like
>>> this:
>>> 1. By default, distribute it to the local pCPU ('local' means the vCPU
>>> is on the pCPU) to make the common case fast.
>>> 2. With the list grows to a point where we think it may consumers too
>>> much time to traverse the list, also distribute wakeup interrupt to local
>>> pCPU, ignoring admin intentionally overloads their system.
>>> 3. When the list length reachs the theoretic average maximum (means
>>> maximal vCPU count divided by pCPU count), distribute wakeup interrupt
>>> to another underutilized pCPU.
>> 
>> By "maximal vCPU count" do you mean, "total number of active vcpus on
>> the system"?  Or some other theoretical maximum vcpu count (e.g., 32k
>> domans * 512 vcpus each or something)?
>
>The former.

Ok. Actually I meant the latter. But now, I realize I was wrong.

>
>> What about saying that the limit of vcpus for any given pcpu will be:
>>  (v_tot / p_tot)

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

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit2   6 xen-boot fail REGR. vs. 107358
 test-amd64-i386-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 
107358

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail in 109020 
pass in 109142
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 12 guest-saverestore fail in 109020 
pass in 109142
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 17 guest-start/win.repeat fail in 
109020 pass in 109142
 test-amd64-i386-xl-xsm 19 guest-start/debian.repeat fail in 109095 pass in 
109142
 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail in 109095 pass 
in 109142
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail in 109095 
pass in 109142
 test-amd64-amd64-xl-qcow29 debian-di-install fail in 109095 pass in 109142
 test-amd64-amd64-xl-qemut-winxpsp3 16 guest-stop fail in 109117 pass in 109142
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail pass in 
109020
 test-amd64-i386-xl-qemut-winxpsp3 16 guest-stopfail pass in 109020
 test-amd64-i386-xl-qemuu-winxpsp3 16 guest-stopfail pass in 109095
 test-amd64-amd64-xl-qemuu-winxpsp3 17 guest-start/win.repeat fail pass in 
109117

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

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

version targeted for testing:
 linuxd071951e08ee23cd725c2336d7ab4582bb93b0af
baseline version:
 linux37feaf8095d352014555b82adb4a04609ca17d3f

Last test of bas

[Xen-devel] [for-4.9] Re: [PATCH v2] libxl/libxl_dm.c: u.hvm.usbdevice_list is checked for emptiness

Robin Lee writes ("[PATCH v2] libxl/libxl_dm.c: u.hvm.usbdevice_list is checked 
for emptiness"):
> Currently usbdevice_list is only checked for nullity. But the OCaml binding
> will convert empty list to a pointer to NULL, instead of a NULL pointer. That
> means the OCaml binding will fail to disable USB.
> 
> This patch will check emptiness of usbdevice_list. And NULL is still a valid 
> empty list.

Acked-by: Ian Jackson 

Julien, this is a candidate for 4.9.  It's a minor bugfix.

Ian.

>  tools/libxl/libxl_dm.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index f8ba859..44ebd70 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -559,9 +559,9 @@ static int libxl__build_device_model_args_old(libxl__gc 
> *gc,
>  }
>  if (libxl_defbool_val(b_info->u.hvm.usb)
>  || b_info->u.hvm.usbdevice
> -|| b_info->u.hvm.usbdevice_list) {
> -if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
> -{
> +|| libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
> +if (b_info->u.hvm.usbdevice
> +&& libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
>  LOGD(ERROR, domid, "Both usbdevice and usbdevice_list set");
>  return ERROR_INVAL;
>  }
> @@ -1149,9 +1149,9 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>  }
>  if (libxl_defbool_val(b_info->u.hvm.usb)
>  || b_info->u.hvm.usbdevice
> -|| b_info->u.hvm.usbdevice_list) {
> -if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
> -{
> +|| libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
> +if (b_info->u.hvm.usbdevice
> +&& libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
>  LOGD(ERROR, guest_domid, "Both usbdevice and usbdevice_list 
> set");
>  return ERROR_INVAL;
>  }
> -- 
> 2.9.3
> 

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

[Xen-devel] [PATCH for-4.9 0/2] x86/pv: Misc fixes

Two fixes for bugs which I have found while trying to raise
{compat_,}create_bounce_frame() up into C.  They should both be fixed in all
releases of Xen.

Andrew Cooper (2):
  x86/pv: Fix bugs with the handling of int80_bounce
  x86/pv: Align %rsp before pushing the failsafe stack frame

 xen/arch/x86/domain.c  | 5 ++---
 xen/arch/x86/x86_64/compat/traps.c | 1 +
 xen/arch/x86/x86_64/traps.c| 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH 2/2] x86/pv: Align %rsp before pushing the failsafe stack frame

Architecturally, all 64bit stacks are aligned on a 16 byte boundary before an
exception frame is pushed.  The failsafe frame is not special in this regard.

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

This change should be backported to all releases, and included in Xen 4.9
---
 xen/arch/x86/domain.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 6e1474b..2ef1c9f 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1280,9 +1280,8 @@ static void load_segments(struct vcpu *n)
 struct pv_vcpu *pv = &n->arch.pv_vcpu;
 struct cpu_user_regs *regs = guest_cpu_user_regs();
 unsigned long *rsp =
-(n->arch.flags & TF_kernel_mode) ?
-(unsigned long *)regs->rsp :
-(unsigned long *)pv->kernel_sp;
+(unsigned long *)(((n->arch.flags & TF_kernel_mode)
+   ? regs->rsp : pv->kernel_sp) & ~0xf);
 unsigned long cs_and_mask, rflags;
 
 /* Fold upcall mask and architectural IOPL into RFLAGS.IF. */
-- 
2.1.4


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

[Xen-devel] [PATCH 1/2] x86/pv: Fix bugs with the handling of int80_bounce

Testing has revealed two issues:

 1) Passing a NULL handle to set_trap_table() is intended to flush the entire
table.  The 64bit guest case (and 32bit guest on 32bit Xen, when it
existed) called init_int80_direct_trap() to reset int80_bounce, but c/s
cda335c279 which introduced the 32bit guest on 64bit Xen support omitted
this step.  Previously therefore, it was impossible for a 32bit guest to
reset its registered int80_bounce details.

 2) init_int80_direct_trap() doesn't honour the guests request to have
interrupts disabled on entry.  PVops Linux requests that interrupts are
disabled, but Xen currently leaves them enabled when following the int80
fastpath.

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

This change should be backported to all releases, and included in Xen 4.9
---
 xen/arch/x86/x86_64/compat/traps.c | 1 +
 xen/arch/x86/x86_64/traps.c| 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/compat/traps.c 
b/xen/arch/x86/x86_64/compat/traps.c
index 8e9a11c..1751ec6 100644
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -339,6 +339,7 @@ int 
compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
 if ( guest_handle_is_null(traps) )
 {
 memset(dst, 0, NR_VECTORS * sizeof(*dst));
+init_int80_direct_trap(current);
 return 0;
 }
 
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index ad4d6c1..78f4105 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -427,12 +427,13 @@ void init_int80_direct_trap(struct vcpu *v)
 struct trap_info *ti = &v->arch.pv_vcpu.trap_ctxt[0x80];
 struct trap_bounce *tb = &v->arch.pv_vcpu.int80_bounce;
 
-tb->flags = TBF_EXCEPTION;
 tb->cs= ti->cs;
 tb->eip   = ti->address;
 
 if ( null_trap_bounce(v, tb) )
 tb->flags = 0;
+else
+tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
 }
 
 static long register_guest_callback(struct callback_register *reg)
-- 
2.1.4


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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol 
header"):
> The endianess of the xenstore protocol header should be specified.
...
> -followed by xsd_sockmsg.len bytes of payload.
> +followed by xsd_sockmsg.len bytes of payload. The header fields are
> +all in little endian byte order.

Yes, but this is not correct.  On a big-endian cpu, they would be in
big-endian.

On a bytesexual cpu, the endianness should be specified but it will be
the same endianness as shared ring fields, etc.  So this doc probably
ought not to contain a list of endiannesses.  Best just to say that
the fields are all in host native byte order.

Ian.

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

Re: [Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

Juergen Gross writes ("[PATCH 2/3] docs: add DIRECTORY_PART specification do 
xenstore protocol doc"):
> DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.
...
> +DIRECTORY_PART   | 
> ||*
> + Same as DIRECTORY, but to be used for children lists longer than
> + XENSTORE_PAYLOAD_MAX. Input are  and the byte offset into
> + the list of children to return. Return values are the generation
> + count  of the node (to be used to ensure the node hasn't
> + changed between two reads:  being the same for multiple
> + reads guarantees the node hasn't changed) and the list of children
> + starting at the specified  of the complete list.

The "generation count" is not defined anywhere else in this protocol
spec, so shouldn't be referred to here without definition.  We should
explicitly state whether using a transaction is sufficient to ensure
that this check will never fail.

Taken together, I think the right approach is to specify a method to
use this and to say that if a different method is used, the results
are not reliable.

Ian.

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

Re: [Xen-devel] [PATCH 3/3] docs: document CONTROL command of xenstore protocol

Juergen Gross writes ("[PATCH 3/3] docs: document CONTROL command of xenstore 
protocol"):
> The CONTROL command (former DEBUG command) isn't specified in the
> xenstore protocol doc. Add it.
...
> -DEBUGprint||??   sends  to 
> debug log
> -DEBUGprint|   EINVAL
> -DEBUGcheck|??checks xenstored 
> innards
> -DEBUGno-op (future 
> extension)
> +CONTROL  |[|]
> + Send a control command  with optional parameters
> + () to Xenstore daemon.  support is
> + implementation specific and might change in future.
>  
> - These requests should not generally be used and may be
> - withdrawn in the future.

I should retain a stronger imprecation.  How about:

  The set of commands and their semantics is implementation
  specific and is likely to change from one Xen version to the
  next.  Out-of-tree users will encounter compatibility issues.

Ian.

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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

(resending this one with Julien's right address.)

Ian Jackson writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore 
protocol header"):
> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore 
> protocol header"):
> > The endianess of the xenstore protocol header should be specified.
> ...
> > -followed by xsd_sockmsg.len bytes of payload.
> > +followed by xsd_sockmsg.len bytes of payload. The header fields are
> > +all in little endian byte order.
> 
> Yes, but this is not correct.  On a big-endian cpu, they would be in
> big-endian.
> 
> On a bytesexual cpu, the endianness should be specified but it will be
> the same endianness as shared ring fields, etc.  So this doc probably
> ought not to contain a list of endiannesses.  Best just to say that
> the fields are all in host native byte order.

Also: in the subject, `endianess' should be `endianness'.

Ian.

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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

On 08/05/17 12:07, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore 
> protocol header"):
>> The endianess of the xenstore protocol header should be specified.
> ...
>> -followed by xsd_sockmsg.len bytes of payload.
>> +followed by xsd_sockmsg.len bytes of payload. The header fields are
>> +all in little endian byte order.
> 
> Yes, but this is not correct.  On a big-endian cpu, they would be in
> big-endian.

We don't support big-endian cpus, right? Do we want to specify the
protocol for unsupported cpus?

> On a bytesexual cpu, the endianness should be specified but it will be
> the same endianness as shared ring fields, etc.  So this doc probably
> ought not to contain a list of endiannesses.  Best just to say that
> the fields are all in host native byte order.

Hmm, this is problematic. How does a guest started e.g. big-endian on a
cpu capable of both byte orders know which endianess the host has? I
think specifying one endianess in this case is the better approach.

BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
either, do we?

I could reword the paragraph to:

"The header fields are in the default endianess of the processor, e.g.
little endian on x86 and ARM."

Juergen

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

Re: [Xen-devel] [PATCH 3/3] docs: document CONTROL command of xenstore protocol

On 08/05/17 12:13, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 3/3] docs: document CONTROL command of xenstore 
> protocol"):
>> The CONTROL command (former DEBUG command) isn't specified in the
>> xenstore protocol doc. Add it.
> ...
>> -DEBUG   print||??   sends  to 
>> debug log
>> -DEBUG   print|   EINVAL
>> -DEBUG   check|??checks xenstored 
>> innards
>> -DEBUG   no-op (future 
>> extension)
>> +CONTROL |[|]
>> +Send a control command  with optional parameters
>> +() to Xenstore daemon.  support is
>> +implementation specific and might change in future.
>>  
>> -These requests should not generally be used and may be
>> -withdrawn in the future.
> 
> I should retain a stronger imprecation.  How about:
> 
>   The set of commands and their semantics is implementation
>   specific and is likely to change from one Xen version to the
>   next.  Out-of-tree users will encounter compatibility issues.

I'm fine with this.


Juergen

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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

Juergen Gross writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore 
protocol header"):
> On 08/05/17 12:07, Ian Jackson wrote:
> > Yes, but this is not correct.  On a big-endian cpu, they would be in
> > big-endian.
> 
> We don't support big-endian cpus, right? Do we want to specify the
> protocol for unsupported cpus?

We have in the past supported big-endian CPUs.  There is no
particular reason to think that a future Xen port will be to only a
little-endian CPU.

> > On a bytesexual cpu, the endianness should be specified but it will be
> > the same endianness as shared ring fields, etc.  So this doc probably
> > ought not to contain a list of endiannesses.  Best just to say that
> > the fields are all in host native byte order.
> 
> Hmm, this is problematic. How does a guest started e.g. big-endian on a
> cpu capable of both byte orders know which endianess the host has? I
> think specifying one endianess in this case is the better approach.

The same way that the guest knows the endianness of the other cpu
structures.

> BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
> either, do we?

I have no idea.  If we do, they will need to byteswap things when
talking PV protocols.

> I could reword the paragraph to:
> 
> "The header fields are in the default endianess of the processor, e.g.
> little endian on x86 and ARM."

What information about endianness is in xen/include/public ?

I don't think the xenstore doc should contain its own indication of
endianness.  That leaves open the possibility that the docs might
specify (and someone might implement!) a mixed-endian system, where
the public headers and PV protocols are in one endianness, but
xenstore in another, because of differences in docs wording.

How about if xenstore.txt says something like `the endianness is the
same as that of the structures in the Xen public headers and the Xen
PV protocols' ?

Ian.

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

[Xen-devel] [linux-linus bisection] complete test-amd64-i386-qemuu-rhel6hvm-intel

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-qemuu-rhel6hvm-intel
testid xen-boot

Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
  Bug introduced:  13e0988140374123bead1dd27c287354cb95108e
  Bug not present: 2763f92f858f7c4c3198335c0542726eaed07ba3
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/109169/


  (Revision log too long, omitted.)


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-i386-qemuu-rhel6hvm-intel.xen-boot.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-linus/test-amd64-i386-qemuu-rhel6hvm-intel.xen-boot
 --summary-out=tmp/109169.bisection-summary --basis-template=59254 
--blessings=real,real-bisect linux-linus test-amd64-i386-qemuu-rhel6hvm-intel 
xen-boot
Searching for failure / basis pass:
 109127 fail [host=nobling1] / 108124 [host=chardonnay0] 108093 [host=baroque1] 
108051 [host=fiano0] 108022 [host=elbling0] 107980 [host=huxelrebe1] 107925 
[host=fiano1] 107819 [host=italia0] 107753 [host=baroque0] 107710 
[host=huxelrebe0] 107660 [host=fiano0] 107638 [host=chardonnay1] 107630 
[host=baroque1] 107621 [host=elbling1] 107607 [host=huxelrebe1] 107592 
[host=chardonnay0] 107579 [host=italia1] 107571 [host=fiano1] 107555 
[host=elbling0] 107529 [host=italia0] 107497 [host=italia0] 107486 
[host=baroque1] 107474 [host=elbling1] 107469 [host=huxelrebe1] 107462 
[host=fiano0] 107452 [host=chardonnay0] 107443 [host=italia1] 107428 
[host=elbling0] 107406 [host=baroque0] 107383 [host=huxelrebe0] 107367 
[host=italia0] 107350 [host=chardonnay1] 107253 [host=baroque1] 107221 
[host=huxelrebe1] 107206 [host=italia1] 107169 [host=baroque0] 107159 
[host=chardonnay0] 107142 [host=fiano0] 107123 [host=huxelrebe0] 107106 
[host=elbling0] 107090 [host=chardonnay1] 107075 [host=fiano1] 107053 
[host=italia1] 107024 [host=elbling1] 107010 [host=huxelrebe1] 106998 
[host=italia0] 106987 [host=baroque1] 106976 [host=baroque0] 106946 
[host=chardonnay1] 106939 [host=huxelrebe0] 106930 [host=elbling0] 106927 
[host=fiano0] 106926 [host=italia1] 106922 [host=fiano1] 106919 [host=elbling1] 
106902 [host=huxelrebe1] 106886 [host=italia0] 106857 [host=baroque0] 106825 
[host=baroque1] 106806 [host=huxelrebe0] 106799 [host=chardonnay1] 106788 
[host=chardonnay0] 106761 [host=huxelrebe1] 106752 [host=baroque0] 106744 
[host=baroque1] 106729 [host=fiano1] 106714 [host=italia0] 106699 
[host=huxelrebe0] 106674 [host=chardonnay0] 106660 [host=chardonnay1] 106636 
[host=huxelrebe1] 106630 [host=italia1] 106625 [host=baroque1] 106623 
[host=elbling0] 106600 [host=baroque0] 106597 [host=fiano0] 106591 
[host=italia0] 106589 [host=huxelrebe0] 106570 [host=elbling1] 106537 
[host=huxelrebe1] 106520 [host=baroque0] 106509 [host=baroque1] 106486 
[host=italia0] 106480 [host=huxelrebe0] 106478 [host=elbling0] 106471 
[host=chardonnay0] 106457 [host=fiano0] 106406 [host=italia1] 106388 
[host=huxelrebe1] 106375 [host=baroque0] 106354 [host=italia0] 106312 
[host=baroque1] 106285 [host=huxelrebe0] 106265 [host=fiano1] 106124 
[host=italia1] 106083 [host=elbling1] 105984 [host=chardonnay0] 105960 
[host=huxelrebe0] 105941 [host=baroque0] 105929 [host=huxelrebe1] 105922 
[host=baroque1] 105905 [host=chardonnay1] 105901 ok.
Failure / basis pass flights: 109127 / 105901
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 13e0988140374123bead1dd27c287354cb95108e 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
8051789e982499050680a26febeada7467e18a8d 
e97832ec6b2a7ddd48b8e6d1d848ffdfee6a31c7 
0a5370ee1f9808fbb16bb03d7f349921cf73a2d4
Basis pass 2763f92f858f7c4c3198335c0542726eaed07ba3 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
b669e922b37b8957248798a5eb7aa96a666cd3fe 
08c008de9c7d3ac71f71c87cc04a47819ca228dc 
75da1b150e646bb9aaa26c5b2452f0c3e782d302
Generating revisions with ./adhoc-revtuple-generator  
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#2763f92f858f7c4c3198335c0542726eaed07ba3-13e0988140374123bead1dd27c287354cb95108e
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f

Re: [Xen-devel] [PATCH v2 0/3] xen/arm: Survive unknown traps from guests


Hi Stefano,

On 05/05/17 20:09, Stefano Stabellini wrote:

On Fri, 5 May 2017, Julien Grall wrote:

Hi all,

This small patch series ensure that Xen will not die when receiving unknown
trap from the guests. I am not aware of any issue with platform we currently
support, so I am not sure whether it would be Xen 4.9 material.


Given that it is a bug fix and of small risk, I think it should be in
4.9.


I agree here, this would allow Xen to be used safely on new hardware if 
no other patch is required.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3] x86/vpmu_intel: Fix hypervisor crash by masking PC bit in MSR_P6_EVNTSEL


Hi Jan,

On 05/05/17 11:16, Jan Beulich wrote:

On 04.05.17 at 23:30,  wrote:

Setting Pin Control (PC) bit (19) in MSR_P6_EVNTSEL results in a General
Protection Fault and thus results in a hypervisor crash. This behavior has
been observed on two generations of Intel processors namely, Haswell and
Broadwell. Other Intel processor generations were not tested. However, it
does seem to be a possible erratum that hasn't yet been confirmed by Intel.

To fix the problem this patch masks PC bit and returns an error in
case any guest tries to write to it on any Intel processor. In addition
to the fact that setting this bit crashes the hypervisor on Haswell and
Broadwell, the PC flag bit toggles a hardware pin on the physical CPU
every time the programmed event occurs and the hardware behavior in
response to the toggle is undefined in the SDM, which makes this bit
unsafe to be used by guests and hence should be masked on all machines.

Signed-off-by: Mohit Gambhir 


Reviewed-by: Jan Beulich 

Iirc the intention was to have this in 4.9, in which case you should
have Cc-ed Juline (now added).


Release-acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

On 08/05/17 12:09, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 2/3] docs: add DIRECTORY_PART specification do 
> xenstore protocol doc"):
>> DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.
> ...
>> +DIRECTORY_PART  | 
>> ||*
>> +Same as DIRECTORY, but to be used for children lists longer than
>> +XENSTORE_PAYLOAD_MAX. Input are  and the byte offset into
>> +the list of children to return. Return values are the generation
>> +count  of the node (to be used to ensure the node hasn't
>> +changed between two reads:  being the same for multiple
>> +reads guarantees the node hasn't changed) and the list of children
>> +starting at the specified  of the complete list.
> 
> The "generation count" is not defined anywhere else in this protocol
> spec, so shouldn't be referred to here without definition.  We should
> explicitly state whether using a transaction is sufficient to ensure
> that this check will never fail.

As the generation count is if no interest anywhere else in this protocol
I don't see why the definition given in parentheses isn't enough.

The solution with  was explicitly demanded in order to _not_
have to use transactions. So referring to transactions now seems to be
counterproductive.

> Taken together, I think the right approach is to specify a method to
> use this and to say that if a different method is used, the results
> are not reliable.

So libxenstore is using DIRECTORY_PART without transactions and will
still deliver correct results.


Juergen

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

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

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

Regressions :-(

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

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

Re: [Xen-devel] [for-4.9] Re: [PATCH v2] libxl/libxl_dm.c: u.hvm.usbdevice_list is checked for emptiness


Hi Ian,

On 08/05/17 10:54, Ian Jackson wrote:

Robin Lee writes ("[PATCH v2] libxl/libxl_dm.c: u.hvm.usbdevice_list is checked for 
emptiness"):

Currently usbdevice_list is only checked for nullity. But the OCaml binding
will convert empty list to a pointer to NULL, instead of a NULL pointer. That
means the OCaml binding will fail to disable USB.

This patch will check emptiness of usbdevice_list. And NULL is still a valid 
empty list.


Acked-by: Ian Jackson 

Julien, this is a candidate for 4.9.  It's a minor bugfix.


Release-acked-by: Julien Grall 



Ian.


 tools/libxl/libxl_dm.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f8ba859..44ebd70 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -559,9 +559,9 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
 }
 if (libxl_defbool_val(b_info->u.hvm.usb)
 || b_info->u.hvm.usbdevice
-|| b_info->u.hvm.usbdevice_list) {
-if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
-{
+|| libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
+if (b_info->u.hvm.usbdevice
+&& libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
 LOGD(ERROR, domid, "Both usbdevice and usbdevice_list set");
 return ERROR_INVAL;
 }
@@ -1149,9 +1149,9 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 }
 if (libxl_defbool_val(b_info->u.hvm.usb)
 || b_info->u.hvm.usbdevice
-|| b_info->u.hvm.usbdevice_list) {
-if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
-{
+|| libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
+if (b_info->u.hvm.usbdevice
+&& libxl_string_list_length(&b_info->u.hvm.usbdevice_list)) {
 LOGD(ERROR, guest_domid, "Both usbdevice and usbdevice_list 
set");
 return ERROR_INVAL;
 }
--
2.9.3



--
Julien Grall

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

Re: [Xen-devel] [PATCH 0/3] docs: add some missing xenstore documentation


Hi Juergen,

On 08/05/17 08:00, Juergen Gross wrote:

There were some bits missing in docs/misc/xenstore.txt, so add them.

We might want to include this in 4.9, but I'm not feeling really
strong about this.


I think any documentation patch should go in the release. Better 
documentation will make happier users :).


Release-acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

On 08/05/17 12:24, Ian Jackson wrote:
> Juergen Gross writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore 
> protocol header"):
>> On 08/05/17 12:07, Ian Jackson wrote:
>>> Yes, but this is not correct.  On a big-endian cpu, they would be in
>>> big-endian.
>>
>> We don't support big-endian cpus, right? Do we want to specify the
>> protocol for unsupported cpus?
> 
> We have in the past supported big-endian CPUs.  There is no
> particular reason to think that a future Xen port will be to only a
> little-endian CPU.
> 
>>> On a bytesexual cpu, the endianness should be specified but it will be
>>> the same endianness as shared ring fields, etc.  So this doc probably
>>> ought not to contain a list of endiannesses.  Best just to say that
>>> the fields are all in host native byte order.
>>
>> Hmm, this is problematic. How does a guest started e.g. big-endian on a
>> cpu capable of both byte orders know which endianess the host has? I
>> think specifying one endianess in this case is the better approach.
> 
> The same way that the guest knows the endianness of the other cpu
> structures.
> 
>> BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
>> either, do we?
> 
> I have no idea.  If we do, they will need to byteswap things when
> talking PV protocols.
> 
>> I could reword the paragraph to:
>>
>> "The header fields are in the default endianess of the processor, e.g.
>> little endian on x86 and ARM."
> 
> What information about endianness is in xen/include/public ?

xen/include/public> grep -iRI endian .
./arch-arm.h: * hypercall arguments are always little endian.
./arch-arm.h:#define PSR_BIG_ENDIAN  (1<<9)/* arm32: Big Endian
Mode */
./arch-x86/hvm/start_info.h: * The address and sizes are always a 64bit
little endian unsigned integer.
./io/sndif.h: * XENSND_PCM_FORMAT_[_]
./io/sndif.h: * endian: , may be absent
./io/sndif.h: * LE - Little endian, BE - Big endian

So with the exception of sndif.h: always little endian.

> I don't think the xenstore doc should contain its own indication of
> endianness.  That leaves open the possibility that the docs might
> specify (and someone might implement!) a mixed-endian system, where
> the public headers and PV protocols are in one endianness, but
> xenstore in another, because of differences in docs wording.
> 
> How about if xenstore.txt says something like `the endianness is the
> same as that of the structures in the Xen public headers and the Xen
> PV protocols' ?

Same as hypercall argument endianness?


Juergen

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

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

On 05/05/17 20:28, Stefano Stabellini wrote:
> On Fri, 5 May 2017, Andrii Anisov wrote:
>> Hello Stefano,
>>
>> On 24.04.17 21:08, Stefano Stabellini wrote:
>>> Stubdomains (stubdoms in short) are small domains, each running a single
>>> application. Typically they run unikernels rather than a full fledged
>>> operating system. A classic example is QEMU stubdoms on x86: one QEMU
>>> stubdoms is started for each regular guest domain. Each QEMU stubdom
>>> instance provides emulation for one guest - it runs one instance of
>>> QEMU.
>> I'm wondering if there are any examples of practical usage of stub domains
>> with ARM?
> 
> Good question. I don't think so: there have been practical examples of
> unikernels running on Xen on ARM, but not stubdoms, because we haven't
> needed to run large emulation pieces yet.

So often when we say "stub domains" we mean specifically, "devicemodel
stub domains".  But there are many other stub domains for other
purposes.  You can run xenstored in a stubdomain rather than in dom0,
for instance; I think this probably already works on ARM.  I believe
that the PV vTPM architecture also has one vTPM "worker" per guest,
along with a "global" domain to control the physical TPM and multiplex
it over the various vTPMs.

 -George

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

Re: [Xen-devel] [PATCH 1/2] x86/pv: Fix bugs with the handling of int80_bounce

>>> On 08.05.17 at 12:04,  wrote:
> Testing has revealed two issues:
> 
>  1) Passing a NULL handle to set_trap_table() is intended to flush the entire
> table.  The 64bit guest case (and 32bit guest on 32bit Xen, when it
> existed) called init_int80_direct_trap() to reset int80_bounce, but c/s
> cda335c279 which introduced the 32bit guest on 64bit Xen support omitted
> this step.  Previously therefore, it was impossible for a 32bit guest to
> reset its registered int80_bounce details.
> 
>  2) init_int80_direct_trap() doesn't honour the guests request to have
> interrupts disabled on entry.  PVops Linux requests that interrupts are
> disabled, but Xen currently leaves them enabled when following the int80
> fastpath.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 
with a remark:

> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -427,12 +427,13 @@ void init_int80_direct_trap(struct vcpu *v)
>  struct trap_info *ti = &v->arch.pv_vcpu.trap_ctxt[0x80];
>  struct trap_bounce *tb = &v->arch.pv_vcpu.int80_bounce;
>  
> -tb->flags = TBF_EXCEPTION;
>  tb->cs= ti->cs;
>  tb->eip   = ti->address;
>  
>  if ( null_trap_bounce(v, tb) )
>  tb->flags = 0;
> +else
> +tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
>  }

This certainly is a correct change to make, but it's not without risk:
If some guest relies on previous buggy behavior (wrongly setting
the flag but expecting interrupts to be on), ugly misbehavior in the
guest could result. Initially I was afraid XenoLinux might be
affected, but I've checked and it isn't.

Jan


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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

2017-05-08 Thread Zhang, Xiong Y

> >>> On 06.05.17 at 03:51,  wrote:
> >> >>> On 05.05.17 at 05:52,  wrote:
> >> > 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
> >> > outstanding p2m_ioreq_server entries")' will call
> >> > p2m_change_entry_type_global() which set entry.recalc=1. Then
> >> > the following get_entry(p2m_ioreq_server) will return
> >> > p2m_ram_rw type.
> >> > But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
> >> > outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
> >> > assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
> >> > type, then reset p2m_ioreq_server entries. The fact is the assumption
> >> > isn't true, and sysnchronously reset function couldn't work. Then
> >> > ioreq.entry_count is larger than zero after an ioreq server unmaps,
> >> > finally this results DomU couldn't reboot.
> >>
> >> I've had trouble understanding this part already on v1 (btw, why is
> >> this one not tagged v2?), and since I still can't figure it I have to ask:
> >> Why is it that guest reboot is being impacted here? From what I recall
> >> a non-zero count should only prevent migration.
> > [Zhang, Xiong Y] Sorry, although they solve the same issue, the solution is
> > totally different, so I didn't mark this as V2, I will mark the following
> > as v2 with this solution.
> > During DomU reboot, it will first unmap ioreq server in shutdown process,
> > then it call map ioreq server in boot process. The following sentence in
> > p2m_set_ioreq_server() result mapping ioreq server failure, then DomU
> > couldn't continue booting.
> > If ( read_atomic(&p->ioreq.entry_count))
> >goto out;
> 
> It is clear that it would be this statement to be the problem one,
> but I continue to not see why this would affect reboot: The rebooted
> guest runs in another VM with, hence, a different p2m. I cannot see
> why there would be a non-zero ioreq.entry_count the first time an
> ioreq server claims the p2m_ioreq_server type for this new domain.
> 
[Zhang, Xiong Y] This is what I see from xl dmesg when a DomU reboot
1) unmap io_req_server with old domid
2) map io_req_server with old domid 
3)unmap io_req_server with old domid
4) map io_req_server with new domid

The 1) and 2) are triggered by our device reset handler in qemu, it will
destroy old device handler, then create device handler with the old domid
again. so we could see ioreq.entry_could > 0 with old domid, then reboot
process terminated.
> Jan


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

Re: [Xen-devel] [PATCH 2/2] x86/pv: Align %rsp before pushing the failsafe stack frame

>>> On 08.05.17 at 12:04,  wrote:
> Architecturally, all 64bit stacks are aligned on a 16 byte boundary before an
> exception frame is pushed.  The failsafe frame is not special in this regard.

I'd prefer "should not be" as effectively is has been so far, but other
than that ...

> Signed-off-by: Andrew Cooper 

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/2] x86/pv: Align %rsp before pushing the failsafe stack frame

On 08/05/17 11:56, Jan Beulich wrote:
 On 08.05.17 at 12:04,  wrote:
>> Architecturally, all 64bit stacks are aligned on a 16 byte boundary before an
>> exception frame is pushed.  The failsafe frame is not special in this regard.
> I'd prefer "should not be" as effectively is has been so far, but other
> than that ...

Certainly.

>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 

Thanks.

~Andrew

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

Re: [Xen-devel] [PATCH 1/3] docs: specify endianess of xenstore protocol header

Hi Juergen,

On 08/05/17 11:17, Juergen Gross wrote:

On 08/05/17 12:07, Ian Jackson wrote:

Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol
header"):

The endianess of the xenstore protocol header should be specified.

...

-followed by xsd_sockmsg.len bytes of payload.
+followed by xsd_sockmsg.len bytes of payload. The header fields are
+all in little endian byte order.

Yes, but this is not correct. On a big-endian cpu, they would be in
big-endian.

We don't support big-endian cpus, right? Do we want to specify the
protocol for unsupported cpus?

On a bytesexual cpu, the endianness should be specified but it will be
the same endianness as shared ring fields, etc. So this doc probably
ought not to contain a list of endiannesses. Best just to say that
the fields are all in host native byte order.

Hmm, this is problematic. How does a guest started e.g. big-endian on a
cpu capable of both byte orders know which endianess the host has? I
think specifying one endianess in this case is the better approach.

BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
either, do we?

At the moment, Xen is always little endian and all the structure between
Xen and the guests are little-endian.

We don't yet support big-end guests but there are nothing to prevent
that. The only change I am aware of is in the MMIO emulation (see [1]).
All the Xen hypercall argument will stay little-endian and the guest
would have to take care of passing the arguments with the correct
endianness.

I could reword the paragraph to:

"The header fields are in the default endianess of the processor, e.g.
little endian on x86 and ARM."

Whilst instruction fetches are always little-endian, the memory
endianness of data access does not have a particular default in the ARM
ARM. This is left up to the implementor.

Cheers,

[1]
https://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=commit;h=e49cecef96d57622e9dcbc6199be1f018d405fc0

--
Julien Grall

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

[Xen-devel] [distros-debian-sid test] 71263: trouble: blocked/broken/pass

2017-05-08 Thread Platform Team regression test user

flight 71263 distros-debian-sid real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71263/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   3 host-install(3) broken REGR. vs. 71244
 build-armhf-pvops 3 host-install(3) broken REGR. vs. 71244
 build-amd64   3 host-install(3) broken REGR. vs. 71244
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 71244

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-i386-sid-netboot-pygrub  1 build-check(1) blocked n/a
 test-amd64-amd64-amd64-sid-netboot-pvgrub  1 build-check(1)blocked n/a
 test-amd64-i386-i386-sid-netboot-pvgrub  1 build-check(1)  blocked n/a
 test-arm64-arm64-armhf-sid-netboot-pygrub  1 build-check(1)blocked n/a
 test-armhf-armhf-armhf-sid-netboot-pygrub  1 build-check(1)blocked n/a
 test-amd64-i386-amd64-sid-netboot-pygrub  1 build-check(1) blocked n/a
 build-arm64-pvops 2 hosts-allocate   broken never pass
 build-arm64   2 hosts-allocate   broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 build-arm64   3 capture-logs broken never pass

baseline version:
 flight   71244

jobs:
 build-amd64  broken  
 build-arm64  broken  
 build-armhf  broken  
 build-i386   pass
 build-amd64-pvopsbroken  
 build-arm64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops pass
 test-amd64-amd64-amd64-sid-netboot-pvgrubblocked 
 test-amd64-i386-i386-sid-netboot-pvgrub  blocked 
 test-amd64-i386-amd64-sid-netboot-pygrub blocked 
 test-arm64-arm64-armhf-sid-netboot-pygrubblocked 
 test-armhf-armhf-armhf-sid-netboot-pygrubblocked 
 test-amd64-amd64-i386-sid-netboot-pygrub blocked 



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

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

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


Push not applicable.


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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

On 08/05/17 11:52, Zhang, Xiong Y wrote:
> On 06.05.17 at 03:51,  wrote:
>>> On 05.05.17 at 05:52,  wrote:
> 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
> outstanding p2m_ioreq_server entries")' will call
> p2m_change_entry_type_global() which set entry.recalc=1. Then
> the following get_entry(p2m_ioreq_server) will return
> p2m_ram_rw type.
> But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
> outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
> assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
> type, then reset p2m_ioreq_server entries. The fact is the assumption
> isn't true, and sysnchronously reset function couldn't work. Then
> ioreq.entry_count is larger than zero after an ioreq server unmaps,
> finally this results DomU couldn't reboot.

 I've had trouble understanding this part already on v1 (btw, why is
 this one not tagged v2?), and since I still can't figure it I have to ask:
 Why is it that guest reboot is being impacted here? From what I recall
 a non-zero count should only prevent migration.
>>> [Zhang, Xiong Y] Sorry, although they solve the same issue, the solution is
>>> totally different, so I didn't mark this as V2, I will mark the following
>>> as v2 with this solution.
>>> During DomU reboot, it will first unmap ioreq server in shutdown process,
>>> then it call map ioreq server in boot process. The following sentence in
>>> p2m_set_ioreq_server() result mapping ioreq server failure, then DomU
>>> couldn't continue booting.
>>> If ( read_atomic(&p->ioreq.entry_count))
>>>goto out;
>>
>> It is clear that it would be this statement to be the problem one,
>> but I continue to not see why this would affect reboot: The rebooted
>> guest runs in another VM with, hence, a different p2m. I cannot see
>> why there would be a non-zero ioreq.entry_count the first time an
>> ioreq server claims the p2m_ioreq_server type for this new domain.
>>
> [Zhang, Xiong Y] This is what I see from xl dmesg when a DomU reboot
> 1) unmap io_req_server with old domid
> 2) map io_req_server with old domid 
> 3)unmap io_req_server with old domid
> 4) map io_req_server with new domid
> 
> The 1) and 2) are triggered by our device reset handler in qemu, it will
> destroy old device handler, then create device handler with the old domid
> again. so we could see ioreq.entry_could > 0 with old domid, then reboot
> process terminated.

Oh, so it prevents reboot of XenGT, but not of normal guests?

Why does a reboot cause the device to detach, re-attach, and then
re-detach again?

Also, I'm sorry for missing the bug during review, but it's a bit
annoying to find out that the core functionality of patch -- detaching
and re-attaching -- wasn't tested at all before submission.

 -George

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

Re: [Xen-devel] [RFC PATCH 2/4] arm/mem_access: Change value of TTBCR_SZ_MASK




On 08/05/17 07:40, Sergej Proskurin wrote:

Hi Julien,


On 05/02/2017 02:01 PM, Julien Grall wrote:



On 02/05/17 12:56, Julien Grall wrote:

Hi Sergej,

On 30/04/17 20:48, Sergej Proskurin wrote:

The TTBCR_SZ holds only 3 bits and thus must be masked with the value
0x7 instead of the previously used value 0xf.


Please quote the spec (paragaph + version) when you do a such change.

TTBCR_* flags are used for both TCR_EL1 (AArch64) and TTBCR (AArch32).
Looking at the spec (ARM DDI 0487A.k_iss10775) TCR_EL1.{T0SZ,T1SZ) is
encoded on 6 bits and TTBCR_EL1.{T0SZ,T1SZ} is encoded on 3 bits, with
the following 3 bits RES0.

So the mask here should be 0x3f.


Hmmm, I have just noticed we do a mix of TTBCR and TCR in the code. I
would prefer if we use only TCR_* everywhere.



Thank you. I have adopted my current implementation so that it uses
solely TCR_* defines.

This is fine for the current implementation as it supports only Aarch64
and the long-descriptor translation table format of Aarch32/ARMv7. Yet,
as soon as we provide support for the short-descriptor translation table
format, I believe it would make sense to make use of the TTBCR_*
defines, as well. Otherwise, the implementation would mixup the TCR_*
with, e.g., the TTBCR_N_MASK defines. Because of this, I would suggest
to use the TTBCR_* when it comes to the short-descriptor format. What do
you think about that?

Also, in order to reduce the complexity of the gpt-walk function, it
would probably make sense to move all short-descriptor translation table
related operations out of the suggested function in the patch 4/4.


It would be indeed better. But in that case, the LPAE page walk should 
be moved in a separate function too. The current helper would just 
select the correct one to call.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH 4/4] arm/mem_access: Add software guest-page-table walk


Hi,

On 08/05/17 10:22, Sergej Proskurin wrote:

On 05/02/2017 05:17 PM, Julien Grall wrote:

On 30/04/17 20:48, Sergej Proskurin wrote:
Also s/ttbcr/tcr/


+struct domain *d = p2m->domain;
+
+const unsigned int offsets[4] = {
+#ifdef CONFIG_ARM_64
+zeroeth_table_offset(gva),
+#endif
+first_table_offset(gva),
+second_table_offset(gva),
+third_table_offset(gva)
+};


Offsets are based on the granularity of Xen (currently 4KB). However
the guests can support 4KB, 16KB, 64KB. Please handle everything
correctly.


True, ARM ist quite flexible. Yet, I have not seen an OS implementation
that is supported by Xen and makes use of page sizes other than 4KB and
their supersets (2/4MB, 1GB) (please let me know, if you know some),
which is why I doubt that we need that. Please let me know why you think
we need that kind of flexibility in software?

If you should nevertheless insist on that functionality, I would include
it in the next patch and try not to blow up the code too much.


Linux is able to support 4KB, 16KB, 64KB page granularity for AArch64. 
Centos and RedHat are only shipped with 64KB page granularity.







+
+const paddr_t masks[4] = {
+#ifdef CONFIG_ARM_64
+ZEROETH_SIZE - 1,
+#endif
+FIRST_SIZE - 1,
+SECOND_SIZE - 1,
+THIRD_SIZE - 1
+};
+
+/* If the MMU is disabled, there is no need to translate the
gva. */
+if ( !(sctlr & SCTLR_M) )
+{
+*ipa = gva;
+
+return 0;
+}
+
+if ( is_32bit_domain(d) )
+{
+/*
+ * XXX: We do not support 32-bit domain translation table
walks for
+ * domains using the short-descriptor translation table
format, yet.
+ */


Debian ARM 32bit is using short-descriptor translation table. So are
you suggesting that memaccess will not correctly with Debian guest?



Yes, as stated in the comment, the current implementation does not
support the short-descriptor translation table format. As this is an RFC
patch, I wanted to see your opinion on that functionality in general
before implementing all corner cases of the ARM architecture.

As mentioned in my previous reply in patch (patch 2/4), I would prefer
to separate the long-descriptor from the short-descriptor translation in
the future implementation.


I agree here. See my answer on patch #2 about how I would like to see 
the implementation.


[...]


+
+*ipa = pfn_to_paddr(pte.walk.base) | (gva & masks[level]);
+
+return 0;
+}
+
+
 /*
  * If mem_access is in use it might have been the reason why
get_page_from_gva
  * failed to fetch the page, as it uses the MMU for the permission
checking.
@@ -109,9 +233,23 @@ p2m_mem_access_check_and_get_page(vaddr_t gva,
unsigned long flag,
 struct page_info *page = NULL;
 struct p2m_domain *p2m = &v->domain->arch.p2m;

+ASSERT(p2m->mem_access_enabled);
+


Why this ASSERT has been added?


The function p2m_mem_access_check_and_get_page is called only if
get_page_from_gva fails and mem_access is active. I can add a comment
and move this part into an individual commit.


Whilst I agree it is dumb to call this code without mem_access enabled, 
this code is able to cope with it. So why do you need this ASSERT?







 rc = gva_to_ipa(gva, &ipa, flag);
+
+/*
+ * In case mem_access is active, hardware-based gva_to_ipa
translation
+ * might fail. Since gva_to_ipa uses the guest's translation
tables, access
+ * to which might be restricted by the active VTTBR, we perform
a gva to
+ * ipa translation in software.
+ */
 if ( rc < 0 )
-goto err;
+if ( p2m_gva_to_ipa(p2m, gva, &ipa, flag) < 0 )
+/*
+ * The software gva to ipa translation can still fail,
if the the
+ * gva is not mapped or does not hold the requested
access rights.
+ */
+goto err;


Rather falling back, why don't we do software page table walk every time?


A software page table walk would (presumably) take more time to
translate the gva. Do we want that? Also, I am not sure what you meant
by "falling back" at this point. Thank you.


What you currently do is try gva_to_ipa and if it does not work you will 
call p2m_gva_to_ipa. This sounds a bit pointless to me and waste of time 
if the underlying memory of stage-1 page table is protected.


Before saying this is taking much more time, I would like to see actual 
numbers.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

Juergen Gross writes ("Re: [PATCH 2/3] docs: add DIRECTORY_PART specification 
do xenstore protocol doc"):
> On 08/05/17 12:09, Ian Jackson wrote:
> > The "generation count" is not defined anywhere else in this protocol
> > spec, so shouldn't be referred to here without definition.  We should
> > explicitly state whether using a transaction is sufficient to ensure
> > that this check will never fail.
> 
> As the generation count is if no interest anywhere else in this protocol
> I don't see why the definition given in parentheses isn't enough.

I think it's rather inexplicit.  How about if I propose an
alternative ?

> The solution with  was explicitly demanded in order to _not_
> have to use transactions. So referring to transactions now seems to be
> counterproductive.

The question is whether a client can use transactions instead.  Your
current wording seems to leave this question open.

Do you have an opinion about the answer this question ?

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

2017-05-08 Thread Zhang, Xiong Y

> On 08/05/17 11:52, Zhang, Xiong Y wrote:
> > On 06.05.17 at 03:51,  wrote:
> >>> On 05.05.17 at 05:52,  wrote:
> > 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
> > outstanding p2m_ioreq_server entries")' will call
> > p2m_change_entry_type_global() which set entry.recalc=1. Then
> > the following get_entry(p2m_ioreq_server) will return
> > p2m_ram_rw type.
> > But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
> > outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
> > assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
> > type, then reset p2m_ioreq_server entries. The fact is the assumption
> > isn't true, and sysnchronously reset function couldn't work. Then
> > ioreq.entry_count is larger than zero after an ioreq server unmaps,
> > finally this results DomU couldn't reboot.
> 
>  I've had trouble understanding this part already on v1 (btw, why is
>  this one not tagged v2?), and since I still can't figure it I have to 
>  ask:
>  Why is it that guest reboot is being impacted here? From what I recall
>  a non-zero count should only prevent migration.
> >>> [Zhang, Xiong Y] Sorry, although they solve the same issue, the solution 
> >>> is
> >>> totally different, so I didn't mark this as V2, I will mark the following
> >>> as v2 with this solution.
> >>> During DomU reboot, it will first unmap ioreq server in shutdown process,
> >>> then it call map ioreq server in boot process. The following sentence in
> >>> p2m_set_ioreq_server() result mapping ioreq server failure, then DomU
> >>> couldn't continue booting.
> >>> If ( read_atomic(&p->ioreq.entry_count))
> >>>goto out;
> >>
> >> It is clear that it would be this statement to be the problem one,
> >> but I continue to not see why this would affect reboot: The rebooted
> >> guest runs in another VM with, hence, a different p2m. I cannot see
> >> why there would be a non-zero ioreq.entry_count the first time an
> >> ioreq server claims the p2m_ioreq_server type for this new domain.
> >>
> > [Zhang, Xiong Y] This is what I see from xl dmesg when a DomU reboot
> > 1) unmap io_req_server with old domid
> > 2) map io_req_server with old domid
> > 3)unmap io_req_server with old domid
> > 4) map io_req_server with new domid
> >
> > The 1) and 2) are triggered by our device reset handler in qemu, it will
> > destroy old device handler, then create device handler with the old domid
> > again. so we could see ioreq.entry_could > 0 with old domid, then reboot
> > process terminated.
> 
> Oh, so it prevents reboot of XenGT, but not of normal guests?
[Zhang, Xiong Y] Yes, only XenGT has this issue.
> 
> Why does a reboot cause the device to detach, re-attach, and then
> re-detach again?
[Zhang, Xiong Y] As our vgt_reset() function will destroy a vgt instance,
then create a vgt instance. And this vgt_reset function is also used
in normal boot process. 
> 
> Also, I'm sorry for missing the bug during review, but it's a bit
> annoying to find out that the core functionality of patch -- detaching
> and re-attaching -- wasn't tested at all before submission.
[Zhang, Xiong Y] Sorry, it is mainly our fault that we miss this.
As we are preparing Xengt community release recently and are refactoring 
our code, then this issue are exposed.

> 
>  -George

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

Re: [Xen-devel] [PATCH v3] x86: correct create_bounce_frame


Hi,

On 05/05/17 15:41, Andrew Cooper wrote:

On 05/05/17 14:11, Jan Beulich wrote:

Commit d9b7ef209a7 ("x86: drop failsafe callback invocation from
assembly") didn't go quite far enough with the cleanup it did: The
changed maximum frame size should also have been reflected in the early
address range check (which has now been pointed out to have been wrong
anyway, using 60 instead of 0x60), and it should have updated the
comment ahead of the function.

Also adjust the lower bound - all is fine (for our purposes) if the
initial guest kernel stack pointer points right at the hypervisor base
address, as only memory _below_ that address is going to be written.

Additionally limit the number of times %rsi is being adjusted to what
is really needed.

Finally move exception fixup code into the designated .fixup section
and macroize the stores to guest stack.

Reported-by: Jann Horn 
Signed-off-by: Jan Beulich 


Reviewed-by: Andrew Cooper 


Release-acked-by: Julien grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

>>> On 08.05.17 at 13:59,  wrote:
>>  On 08/05/17 11:52, Zhang, Xiong Y wrote:
>> > On 06.05.17 at 03:51,  wrote:
>> >>> On 05.05.17 at 05:52,  wrote:
>> > 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
>> > outstanding p2m_ioreq_server entries")' will call
>> > p2m_change_entry_type_global() which set entry.recalc=1. Then
>> > the following get_entry(p2m_ioreq_server) will return
>> > p2m_ram_rw type.
>> > But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
>> > outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
>> > assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
>> > type, then reset p2m_ioreq_server entries. The fact is the assumption
>> > isn't true, and sysnchronously reset function couldn't work. Then
>> > ioreq.entry_count is larger than zero after an ioreq server unmaps,
>> > finally this results DomU couldn't reboot.
>> 
>>  I've had trouble understanding this part already on v1 (btw, why is
>>  this one not tagged v2?), and since I still can't figure it I have to 
>>  ask:
>>  Why is it that guest reboot is being impacted here? From what I recall
>>  a non-zero count should only prevent migration.
>> >>> [Zhang, Xiong Y] Sorry, although they solve the same issue, the solution 
>> >>> is
>> >>> totally different, so I didn't mark this as V2, I will mark the following
>> >>> as v2 with this solution.
>> >>> During DomU reboot, it will first unmap ioreq server in shutdown process,
>> >>> then it call map ioreq server in boot process. The following sentence in
>> >>> p2m_set_ioreq_server() result mapping ioreq server failure, then DomU
>> >>> couldn't continue booting.
>> >>> If ( read_atomic(&p->ioreq.entry_count))
>> >>>goto out;
>> >>
>> >> It is clear that it would be this statement to be the problem one,
>> >> but I continue to not see why this would affect reboot: The rebooted
>> >> guest runs in another VM with, hence, a different p2m. I cannot see
>> >> why there would be a non-zero ioreq.entry_count the first time an
>> >> ioreq server claims the p2m_ioreq_server type for this new domain.
>> >>
>> > [Zhang, Xiong Y] This is what I see from xl dmesg when a DomU reboot
>> > 1) unmap io_req_server with old domid
>> > 2) map io_req_server with old domid
>> > 3)unmap io_req_server with old domid
>> > 4) map io_req_server with new domid
>> >
>> > The 1) and 2) are triggered by our device reset handler in qemu, it will
>> > destroy old device handler, then create device handler with the old domid
>> > again. so we could see ioreq.entry_could > 0 with old domid, then reboot
>> > process terminated.
>> 
>> Oh, so it prevents reboot of XenGT, but not of normal guests?
> [Zhang, Xiong Y] Yes, only XenGT has this issue.

The patch description should say so then.

>> Why does a reboot cause the device to detach, re-attach, and then
>> re-detach again?
> [Zhang, Xiong Y] As our vgt_reset() function will destroy a vgt instance,
> then create a vgt instance. And this vgt_reset function is also used
> in normal boot process. 

Sounds like that's where the immediate problem is then. (Of course
we want the hypervisor side taken care of too.)

Jan


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

Re: [Xen-devel] [PATCH V3 0/2] Fix vm_event resume path race condition




On 05/05/17 15:19, Andrew Cooper wrote:

On 04/05/17 10:00, Razvan Cojocaru wrote:

This small series first creates hvm/vm_event.{h,c}, in order to bring
under vm_event maintainership the code that has previously lived in
hvm_do_resume(), and then fixes a __context_switch()-related race
condition when attempting to set registers via vm_event reply.

[PATCH V3 1/2] x86/vm_event: added hvm/vm_event.{h,c}
[PATCH V3 2/2] x86/vm_event: fix race between __context_switch() and


Julien: Could we get this fix into 4.9?  The content is reviewed/acked
(subject to a fixup on commit).


Release-acked-by: Julien Grall 

Cheers,



~Andrew



--
Julien Grall

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

Re: [Xen-devel] [PATCH] x86: correct boot time page table setup


Hi,

On 05/05/17 16:08, Andrew Cooper wrote:

On 05/05/17 14:12, Jan Beulich wrote:

While using alloc_domheap_pages() and assuming the allocated memory is
directly accessible is okay at boot time (as we run on the idle page
tables there), memory hotplug code too assumes it can access the
resulting page tables without using map_domain_page() or alike, and
hence we need to obtain memory suitable for ordinary page table use
here.

Signed-off-by: Jan Beulich 


Reviewed-by: Andrew Cooper 


Release-acked-by: Julien Grall 

Cheers,


--
Julien Grall


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

Re: [Xen-devel] [PATCH 1/2] x86/pv: Fix bugs with the handling of int80_bounce

On 08/05/17 11:52, Jan Beulich wrote:
 On 08.05.17 at 12:04,  wrote:
>> Testing has revealed two issues:
>>
>>  1) Passing a NULL handle to set_trap_table() is intended to flush the entire
>> table.  The 64bit guest case (and 32bit guest on 32bit Xen, when it
>> existed) called init_int80_direct_trap() to reset int80_bounce, but c/s
>> cda335c279 which introduced the 32bit guest on 64bit Xen support omitted
>> this step.  Previously therefore, it was impossible for a 32bit guest to
>> reset its registered int80_bounce details.
>>
>>  2) init_int80_direct_trap() doesn't honour the guests request to have
>> interrupts disabled on entry.  PVops Linux requests that interrupts are
>> disabled, but Xen currently leaves them enabled when following the int80
>> fastpath.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 
> with a remark:
>
>> --- a/xen/arch/x86/x86_64/traps.c
>> +++ b/xen/arch/x86/x86_64/traps.c
>> @@ -427,12 +427,13 @@ void init_int80_direct_trap(struct vcpu *v)
>>  struct trap_info *ti = &v->arch.pv_vcpu.trap_ctxt[0x80];
>>  struct trap_bounce *tb = &v->arch.pv_vcpu.int80_bounce;
>>  
>> -tb->flags = TBF_EXCEPTION;
>>  tb->cs= ti->cs;
>>  tb->eip   = ti->address;
>>  
>>  if ( null_trap_bounce(v, tb) )
>>  tb->flags = 0;
>> +else
>> +tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
>>  }
> This certainly is a correct change to make, but it's not without risk:
> If some guest relies on previous buggy behavior (wrongly setting
> the flag but expecting interrupts to be on), ugly misbehavior in the
> guest could result. Initially I was afraid XenoLinux might be
> affected, but I've checked and it isn't.

So c/s bfad55585 (which introduces int80_bounce) is a very interesting read.

(Aside from the point here, I didn't realise that we ever had a copy of
the FreeBSD kernel in tree, or that the reason we have a separate IDT
per pcpu was for the predecessor to int80_bouce.)

At the time, there was generic set_fast_trap() which rewrote the IDT to
move straight from ring3 to ring1.  It had a few restrictions such as
only tolerating a vector of 0x80, and rejecting the setup if interrupts
were requested to be disabled (as there was no way of clearing the
evtchn_upcall_mask with this mechanism).

That patch introduced init_int80_direct_trap(), along with a comment
explaining why interrupt gates were rejected, although it was restricted
to 32bit hypervisors at that point.

The direct-trap path was never available in a 64bit build of Xen (owing
to the inability to have non long mode code segments in the IDT), and
c/s 3e1b9525de introduced the int80_direct_trap() path (which looks
remarkably unchanged in the 10 years its been in the codebase).

At this point, the 32bit version of int80_direct_trap() explained why it
couldn't tolerate interrupt gates, but the newly-introduced 64bit
version omitted any comment/code on this point, and would have worked
correctly for interrupt gates if the adjustment in this patch had been
considered.

Overall, I expect that Xenolinux purposefully never asked for an
interrupt gate (as Xen would have rejected that in the 32bit case), and
this point was never considered at all when PVOps was developed, which
followed Linux's normal expectation that int80 was an interrupt gate.

~Andrew

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

[Xen-devel] [linux-3.18 test] 109161: tolerable FAIL - PUSHED

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop   fail REGR. vs. 108152

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
 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-i386-xl-qemut-win7-amd64 17 guest-start/win.repeat fail like 108037
 test-amd64-i386-qemut-rhel6hvm-amd 11 guest-start/redhat.repeat fail like 
108095
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail like 108095
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 108152
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 108152
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 108152
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 108152
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-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-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 build-arm64-pvops 5 kernel-build fail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   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-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 linux68e50dad01f491a0645b720d6bf5a2f00411fbec
baseline version:
 linuxce88f02714836c33a4f0173c29fbe378ea402275

Last test of basis   108152  2017-05-02 16:48:51 Z5 days
Testing same since   109161  2017-05-08 06:20:31 Z0 days1 attempts


People who touched revisions under test:
  Aaro Koskinen 
  Aaron Brown 
  Adrian Salido 
  Al Viro 
  Amir Vadai 
  Amit Pundir 
  Andreas Gruenbacher 
  Andrew Morton 
  Andrey Konovalov 
  Arnd Bergmann 
  Baozeng Ding 
  Benjamin Romer 
  Bjorn Helgaas 
  Bob Picco 
  Brad Mouring 
  Brian Norris 
  Catalin Marinas 
  Chao Yu 
  Corey Minyard 
  Dan Carpenter 
  David S. Miller 
  Dmitry Torokhov 
  Ed Bordin 
  Eli Cohen 
  Eric Dumazet 
  Fabio Estevam 
  Felipe Balbi 
  Felipe F. Tonello 
  Greg Kroah-Hartman 
  Guillaume Nault 
  Ingo Molnar 
  J. Bruce Fields 
  Jaegeuk Kim 
  James Bottomley 
  James Hogan 
  Jan Kara 
  Jeff Kirsher 
  Jens Axboe 
  Josh Poimboeuf 
  Juergen Gross 
  Khem Raj 
  Kirill A. Shutemov 
  Kyle Roeschley 
  Lars Ellenberg 
  Linus Torvalds 
  Marcelo Ricardo Leitner 
  Mark Brown 
  Masahiro Yamada 
  Mauro Carvalho Cheh

Re: [Xen-devel] [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

On 08/05/17 13:53, Ian Jackson wrote:
> Juergen Gross writes ("Re: [PATCH 2/3] docs: add DIRECTORY_PART specification 
> do xenstore protocol doc"):
>> On 08/05/17 12:09, Ian Jackson wrote:
>>> The "generation count" is not defined anywhere else in this protocol
>>> spec, so shouldn't be referred to here without definition.  We should
>>> explicitly state whether using a transaction is sufficient to ensure
>>> that this check will never fail.
>>
>> As the generation count is if no interest anywhere else in this protocol
>> I don't see why the definition given in parentheses isn't enough.
> 
> I think it's rather inexplicit.  How about if I propose an
> alternative ?
> 
>> The solution with  was explicitly demanded in order to _not_
>> have to use transactions. So referring to transactions now seems to be
>> counterproductive.
> 
> The question is whether a client can use transactions instead.  Your
> current wording seems to leave this question open.
> 
> Do you have an opinion about the answer this question ?

Using transactions instead will work, of course. Otherwise transaction
handling would be broken.


Juergen

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

Re: [Xen-devel] [PATCH 1/2] x86/pv: Fix bugs with the handling of int80_bounce

>>> On 08.05.17 at 14:23,  wrote:
> Overall, I expect that Xenolinux purposefully never asked for an
> interrupt gate (as Xen would have rejected that in the 32bit case), and
> this point was never considered at all when PVOps was developed, which
> followed Linux's normal expectation that int80 was an interrupt gate.

Yet I have a hard time seeing why 32-bit Linux needs it to be that
way.

Jan


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

[Xen-devel] [PATCH for-next] xen/x86: Remove APIC_INTEGRATED() checks

All 64bit processors have integrated APICs.  Xen has no need to attempt to
cope with external APICs.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/apic.c   | 9 +++--
 xen/arch/x86/smpboot.c| 6 +-
 xen/include/asm-x86/apicdef.h | 1 -
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index e32dcd7..1dd188a 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -522,7 +522,7 @@ static void resume_x2apic(void)
 
 void setup_local_APIC(void)
 {
-unsigned long oldvalue, value, ver, maxlvt;
+unsigned long oldvalue, value, maxlvt;
 int i, j;
 
 /* Pound the ESR really hard over the head with a big hammer - mbligh */
@@ -533,15 +533,12 @@ void setup_local_APIC(void)
 apic_write(APIC_ESR, 0);
 }
 
-value = apic_read(APIC_LVR);
-ver = GET_APIC_VERSION(value);
-
 BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
 
 /*
  * Double-check whether this APIC is really registered.
  */
-if (!APIC_INTEGRATED(ver) || !apic_id_registered())
+if (!apic_id_registered())
 BUG();
 
 /*
@@ -1358,7 +1355,7 @@ int __init APIC_init_uniprocessor (void)
 /*
  * Complain if the BIOS pretends there is one.
  */
-if (!cpu_has_apic && 
APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])) {
+if (!cpu_has_apic) {
 printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
boot_cpu_physical_apicid);
 skip_ioapic_setup = 1;
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 50b907b..92fe4f2 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -385,9 +385,6 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned 
long start_eip)
 /*
  * Be paranoid about clearing APIC errors.
  */
-if ( !APIC_INTEGRATED(apic_version[phys_apicid]) )
-return -ENODEV;
-
 apic_read_around(APIC_SPIV);
 apic_write(APIC_ESR, 0);
 apic_read(APIC_ESR);
@@ -835,8 +832,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 }
 
 /* If we couldn't find a local APIC, then get out of here now! */
-if ( !APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])
- || !cpu_has_apic )
+if ( !cpu_has_apic )
 {
 printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
boot_cpu_physical_apicid);
diff --git a/xen/include/asm-x86/apicdef.h b/xen/include/asm-x86/apicdef.h
index da7f4d3..1b2c452 100644
--- a/xen/include/asm-x86/apicdef.h
+++ b/xen/include/asm-x86/apicdef.h
@@ -19,7 +19,6 @@
 #defineAPIC_LVR_DIRECTED_EOI   (1 << 24)
 #defineGET_APIC_VERSION(x) ((x)&0xFF)
 #defineGET_APIC_MAXLVT(x)  (((x)>>16)&0xFF)
-#defineAPIC_INTEGRATED(x)  ((x)&0xF0)
 #defineAPIC_XAPIC(x)   ((x) >= 0x14)
 #defineAPIC_TASKPRI0x80
 #defineAPIC_TPRI_MASK  0xFF
-- 
2.1.4


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

Re: [Xen-devel] [PATCH V3 1/2] x86/vm_event: added hvm/vm_event.{h, c}

>>> On 04.05.17 at 11:00,  wrote:
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,7 @@ S:Supported
>  F:   tools/tests/xen-access
>  F:   xen/arch/*/monitor.c
>  F:   xen/arch/*/vm_event.c
> +F:   xen/arch/*/hvm/vm_event.c
>  F:   xen/arch/arm/mem_access.c
>  F:   xen/arch/x86/mm/mem_access.c
>  F:   xen/arch/x86/hvm/monitor.c
> @@ -413,6 +414,7 @@ F:xen/common/vm_event.c
>  F:   xen/include/*/mem_access.h
>  F:   xen/include/*/monitor.h
>  F:   xen/include/*/vm_event.h
> +F:   xen/include/*/hvm/vm_event.h
>  F:   xen/include/asm-x86/hvm/monitor.h

Btw., I've noticed only now that these additions would better be
x86-specific, just like their hvm/monitor.[ch] counterparts. I intend
to adjust this while committing.

Jan


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

Re: [Xen-devel] [PATCH V3 1/2] x86/vm_event: added hvm/vm_event.{h, c}

2017-05-08 Thread Razvan Cojocaru

On 05/08/17 15:44, Jan Beulich wrote:
 On 04.05.17 at 11:00,  wrote:
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -404,6 +404,7 @@ S:   Supported
>>  F:  tools/tests/xen-access
>>  F:  xen/arch/*/monitor.c
>>  F:  xen/arch/*/vm_event.c
>> +F:  xen/arch/*/hvm/vm_event.c
>>  F:  xen/arch/arm/mem_access.c
>>  F:  xen/arch/x86/mm/mem_access.c
>>  F:  xen/arch/x86/hvm/monitor.c
>> @@ -413,6 +414,7 @@ F:   xen/common/vm_event.c
>>  F:  xen/include/*/mem_access.h
>>  F:  xen/include/*/monitor.h
>>  F:  xen/include/*/vm_event.h
>> +F:  xen/include/*/hvm/vm_event.h
>>  F:  xen/include/asm-x86/hvm/monitor.h
> 
> Btw., I've noticed only now that these additions would better be
> x86-specific, just like their hvm/monitor.[ch] counterparts. I intend
> to adjust this while committing.

Fair enough.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH 1/2] x86/pv: Fix bugs with the handling of int80_bounce

On 08/05/17 13:39, Jan Beulich wrote:
 On 08.05.17 at 14:23,  wrote:
>> Overall, I expect that Xenolinux purposefully never asked for an
>> interrupt gate (as Xen would have rejected that in the 32bit case), and
>> this point was never considered at all when PVOps was developed, which
>> followed Linux's normal expectation that int80 was an interrupt gate.
> Yet I have a hard time seeing why 32-bit Linux needs it to be that
> way.

I don't understand what you mean by this.

Is it a counterpoint to my statement, or just an observation about the
way Linux chooses to set things up?

~Andrew

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

Re: [Xen-devel] [PATCH for-next] xen/x86: Remove APIC_INTEGRATED() checks

>>> On 08.05.17 at 14:40,  wrote:
> All 64bit processors have integrated APICs.  Xen has no need to attempt to
> cope with external APICs.
> 
> 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 1/2] x86/pv: Fix bugs with the handling of int80_bounce

>>> On 08.05.17 at 14:54,  wrote:
> On 08/05/17 13:39, Jan Beulich wrote:
> On 08.05.17 at 14:23,  wrote:
>>> Overall, I expect that Xenolinux purposefully never asked for an
>>> interrupt gate (as Xen would have rejected that in the 32bit case), and
>>> this point was never considered at all when PVOps was developed, which
>>> followed Linux's normal expectation that int80 was an interrupt gate.
>> Yet I have a hard time seeing why 32-bit Linux needs it to be that
>> way.
> 
> I don't understand what you mean by this.
> 
> Is it a counterpoint to my statement, or just an observation about the
> way Linux chooses to set things up?

Just an observation, including the implication that at least the 32-bit
Linux case has no issue (and quite likely never had one) with us not
masking events despite this having been requested. Otoh it's a little
surprising that 64-bit Linux never had this reported as an issue, as
there's a non-zero chance for its entry code to be hit before the GS
switch.

Jan

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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

On 05/05/17 15:40, Jan Beulich wrote:
 On 05.05.17 at 05:52,  wrote:
>> 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
>> outstanding p2m_ioreq_server entries")' will call
>> p2m_change_entry_type_global() which set entry.recalc=1. Then
>> the following get_entry(p2m_ioreq_server) will return
>> p2m_ram_rw type.
>> But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
>> outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
>> assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
>> type, then reset p2m_ioreq_server entries. The fact is the assumption
>> isn't true, and sysnchronously reset function couldn't work. Then
>> ioreq.entry_count is larger than zero after an ioreq server unmaps,
>> finally this results DomU couldn't reboot.
> 
> I've had trouble understanding this part already on v1 (btw, why is
> this one not tagged v2?), and since I still can't figure it I have to ask:
> Why is it that guest reboot is being impacted here? From what I recall
> a non-zero count should only prevent migration.
> 
>> This patch add a P2M_PRE_RECALC flag to p2m_query_t, then
>> get_entry(P2M_PRE_RECALC) will return p2m_ioreq_server type
>> for p2m_ioreq_server pfn, and finally change mem type through set_entry.
> 
> This looks to be a relatively little impact change, but nevertheless
> I'm wondering whether someone else (George?) may be able to
> think of some more elegant solution (I have to admit that, having
> suggested the one here, I can't).

So the basic problem is that the implementation-specific get_entry()
will return the theoretical new type without changing it, and
finish_type_change() is in the generic code which calls these
implementation-specific p2m functions.

I think the possible solutions are:

1. Add a flag to the get_entry() call to return the real entry (i.e.,
the solution in this patch)

2. Have get_entry() update entries as it reads them

3. Implement an implementation-specific finish_type_change()

Of the three, there's something attractive about #2; but it would take a
lot of careful thought.  In theory there shouldn't be any harm in
changing the type of an entry marked "recalc", but with so many callers
it's difficult to be sure.  I'd never want to make such a change this
close to a release.

#3 will introduce a lot of unnecessary code duplication; so I think for
now #1 is probably the best idea (in particular during a code freeze).

 -George

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

Re: [Xen-devel] [PATCH for-next] xen/x86: Remove APIC_INTEGRATED() checks

2017-05-08 Thread Wei Liu

On Mon, May 08, 2017 at 01:40:36PM +0100, Andrew Cooper wrote:
> All 64bit processors have integrated APICs.  Xen has no need to attempt to
> cope with external APICs.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [for-4.9] Re: [PATCH v2] libxl/libxl_dm.c: u.hvm.usbdevice_list is checked for emptiness

Julien Grall writes ("Re: [for-4.9] Re: [PATCH v2] libxl/libxl_dm.c: 
u.hvm.usbdevice_list is checked for emptiness"):
> On 08/05/17 10:54, Ian Jackson wrote:
> > Julien, this is a candidate for 4.9.  It's a minor bugfix.
> 
> Release-acked-by: Julien Grall 

Thanks.  Committed.

Ian.

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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

On 08/05/17 14:26, George Dunlap wrote:
> On 05/05/17 15:40, Jan Beulich wrote:
> On 05.05.17 at 05:52,  wrote:
>>> 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
>>> outstanding p2m_ioreq_server entries")' will call
>>> p2m_change_entry_type_global() which set entry.recalc=1. Then
>>> the following get_entry(p2m_ioreq_server) will return
>>> p2m_ram_rw type.
>>> But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
>>> outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
>>> assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
>>> type, then reset p2m_ioreq_server entries. The fact is the assumption
>>> isn't true, and sysnchronously reset function couldn't work. Then
>>> ioreq.entry_count is larger than zero after an ioreq server unmaps,
>>> finally this results DomU couldn't reboot.
>>
>> I've had trouble understanding this part already on v1 (btw, why is
>> this one not tagged v2?), and since I still can't figure it I have to ask:
>> Why is it that guest reboot is being impacted here? From what I recall
>> a non-zero count should only prevent migration.
>>
>>> This patch add a P2M_PRE_RECALC flag to p2m_query_t, then
>>> get_entry(P2M_PRE_RECALC) will return p2m_ioreq_server type
>>> for p2m_ioreq_server pfn, and finally change mem type through set_entry.
>>
>> This looks to be a relatively little impact change, but nevertheless
>> I'm wondering whether someone else (George?) may be able to
>> think of some more elegant solution (I have to admit that, having
>> suggested the one here, I can't).
> 
> So the basic problem is that the implementation-specific get_entry()
> will return the theoretical new type without changing it, and
> finish_type_change() is in the generic code which calls these
> implementation-specific p2m functions.
> 
> I think the possible solutions are:
> 
> 1. Add a flag to the get_entry() call to return the real entry (i.e.,
> the solution in this patch)
> 
> 2. Have get_entry() update entries as it reads them
> 
> 3. Implement an implementation-specific finish_type_change()

Actually -- there are already implementation-specific versions for
individual gpas: p2m-pt.c:do_recalc() and p2m-ept.c:resolve_misconfig().
 They even already have the same function signature

If we made a new p2m hook, p2m->recalc(), we could simply loop around
calling that instead of doing the get_entry() / set_entry() cycle.

One advantage of doing this is that those functions automatically handle
superpages, whereas the patch you send here resets all entries to 4k.

Ideally we'd modify the functions to return the order of the entry
changed to make the loop more efficient, but that should probably wait
until after 4.9.

 -George

> 
> Of the three, there's something attractive about #2; but it would take a
> lot of careful thought.  In theory there shouldn't be any harm in
> changing the type of an entry marked "recalc", but with so many callers
> it's difficult to be sure.  I'd never want to make such a change this
> close to a release.
> 
> #3 will introduce a lot of unnecessary code duplication; so I think for
> now #1 is probably the best idea (in particular during a code freeze).
> 
>  -George
> 


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

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

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl  11 guest-start  fail REGR. vs. 109036

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  8bf68dca65e2d61f4dfc6715cca51ad3dd5aadf1
baseline version:
 xen  8839be5c1fe339a1310b4e05e88c5a0230b7959d

Last test of basis   109036  2017-05-05 16:03:12 Z2 days
Testing same since   109174  2017-05-08 12:01:41 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Kevin Tian 
  Mohit Gambhir 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  fail
 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


Not pushing.


commit 8bf68dca65e2d61f4dfc6715cca51ad3dd5aadf1
Author: Mohit Gambhir 
Date:   Mon May 8 13:37:17 2017 +0200

x86/vpmu_intel: fix hypervisor crash by masking PC bit in MSR_P6_EVNTSEL

Setting Pin Control (PC) bit (19) in MSR_P6_EVNTSEL results in a General
Protection Fault and thus results in a hypervisor crash. This behavior has
been observed on two generations of Intel processors namely, Haswell and
Broadwell. Other Intel processor generations were not tested. However, it
does seem to be a possible erratum that hasn't yet been confirmed by Intel.

To fix the problem this patch masks PC bit and returns an error in
case any guest tries to write to it on any Intel processor. In addition
to the fact that setting this bit crashes the hypervisor on Haswell and
Broadwell, the PC flag bit toggles a hardware pin on the physical CPU
every time the programmed event occurs and the hardware behavior in
response to the toggle is undefined in the SDM, which makes this bit
unsafe to be used by guests and hence should be masked on all machines.

Signed-off-by: Mohit Gambhir 
Reviewed-by: Jan Beulich 
Acked-by: Kevin Tian 
Release-acked-by: Julien Grall 

commit d595e126c96976d056d7877e4319b3f2f3d246b1
Author: Jan Beulich 
Date:   Mon May 8 13:36:28 2017 +0200

VMX: constrain vmx_intr_assist() debugging code to debug builds

This is because that code, added by commit 997382b771 ("y86/vmx: dump
PIR and vIRR before ASSERT()"), was meant to be removed by the time we
finalize 4.9, but the root cause of the ASSERT() wrongly(?) triggering
still wasn't found.

Take the opportunity and also correct the format specifiers, which I
had got wrong when editing said change while committing.

Signed-off-by: Jan Beulich 
Acked-by: Kevin Tian 
Release-Acked-by: Julien Grall 
(qemu changes not included)

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

Re: [Xen-devel] [RFC PATCH 01/10] ARM: vGIC: remove rank lock from IRQ routing functions


Hi Andre,

On 08/05/17 10:15, Andre Przywara wrote:

On 04/05/17 16:53, Julien Grall wrote:

Hi Andre,

On 04/05/17 16:31, Andre Przywara wrote:

gic_route_irq_to_guest() and gic_remove_irq_from_guest() take the rank
lock, however never actually access the rank structure.
Avoid taking the lock in those two functions and remove some more
unneeded code on the way.


The rank is here to protect p->desc when checking that the virtual
interrupt was not yet routed to another physical interrupt.


Really? To me that sounds quite surprising.
My understanding is that the VGIC VCPU lock protected the pending_irq
(and thus the desc pointer?) so far, and the desc itself has its own lock.
According to the comment in the struct irq_rank declaration the lock
protects the members of this struct only.

Looking briefly at users of pending_irq->desc (for instance
gicv[23]_update_lr() or gic_update_one_lr()) I can't see any hint that
they care about the lock.

So should that be fixed or at least documented?


My understanding is this rank lock is preventing race between two 
updates of p->desc. This can happen if gic_route_irq_to_guest(...) is 
called concurrently with the same vIRQ but different pIRQ.


If you drop this lock, nothing will protect that anymore.




Without this locking, you can have two concurrent call of
gic_route_irq_to_guest that will update the same virtual interrupt but
with different physical interrupts.

You would have to replace the rank lock by the per-pending_irq lock to
keep the code safe.


That indeed sounds reasonable.


As you mentioned IRL, the current code may lead to a deadlock due to 
locking order.


Indeed routing an IRQ (route_irq_to_guest) will take:
1) desc lock (in route_irq_to_guest)
2) rank lock (in gic_route_irq_to_guest)

Whilst the MMIO emulation of ISENABLER_* will take:
1) rank lock
2) desc lock (in vgic_enable_irqs)

Using the per-pending_irq lock will not solve the deadlock. I though a 
bit more to the code. I believe the routing of SPIs/PPIs after domain 
creation time is a call to mistake and locking nightmare. Similarly an 
interrupt should stay routed for the duration of the domain life.


So I would forbid IRQ routing after domain creation (see 
d->creation_finished) and remove IRQ whilst routing (I think with 
d->is_dying). This would have an race between the routing and the rest 
of the vGIC code.


However, this would not prevent the routing function to race against 
itself. For that I would take the vgic domain lock, it is fine because 
routing is not something we expect to happen often.


Any opinions?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] Security response; public holidays [and 1 more messages]

Ian Jackson writes ("Security response; public holidays"):
> It would perhaps be possible to find a holiday calendar listing all
> known public holidays throughout the world.  But we are interested in
> public holidays affecting Xen Project community members.
> 
> I was wondering if we should start a wiki page and invite community
> members to add their own local holidays.

I have now created this page:
  https://wiki.xenproject.org/wiki/HolidayCalendar

I populated it with holidays from my own jurisdiction, the UK, to
provide a skeleton.

Does that look about right ?  If so then we should send out a call on
xen-announce for community members to add their own dates.

The policy doesn't explicitly ask us to avoid holidays.  It talks
about a "usual starting point" and "reasons to diverge".  We have
usually treated public holidays (at least, ones which are widely
observed) as reasons to diverge and no-one seems to object to this
practice.

If anyone things this needs to be made more explicit in the policy, I
wouldn't oppose that, but personally I think what we have is OK.

> I suggest we set a reminder to send out a call for updates in around
> October of the previous year.

I changed this to September and put it in the bottom of the page.

Ian.

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

Re: [Xen-devel] [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

2017-05-08 Thread Eric Blake

On 05/08/2017 12:26 AM, David Gibson wrote:
> On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
>> Time to wire up all the call sites that request a shutdown or
>> reset to use the enum added in the previous patch.
>>
>> It would have been less churn to keep the common case with no
>> arguments as meaning guest-triggered, and only modified the
>> host-triggered code paths, via a wrapper function, but then we'd
>> still have to audit that I didn't miss any host-triggered spots;
>> changing the signature forces us to double-check that I correctly
>> categorized all callers.
>>
>> Since command line options can change whether a guest reset request
>> causes an actual reset vs. a shutdown, it's easy to also add the
>> information to reset requests.
>>
>> Replay adds a FIXME to preserve the cause across the replay stream,
>> that will be tackled in the next patch.
>>
>> Signed-off-by: Eric Blake 
>> Acked-by: David Gibson  [ppc parts]
>> Reviewed-by: Mark Cave-Ayland  [SPARC part]
> 
> [snip]
> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 9f18f75..2735fe9 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1166,7 +1166,7 @@ static target_ulong 
>> h_client_architecture_support(PowerPCCPU *cpu,
>>  spapr_ovec_cleanup(ov5_updates);
>>
>>  if (spapr->cas_reboot) {
>> -qemu_system_reset_request();
>> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> 
> I'm not 100% sure about this one, since I'm not sure 100% of how the
> different enum values are defined.  This one is tripped when feature
> negotiation between firmware and guest can't be satisfied without
> rebooting (next time round the firmware will use some different
> options).

Patch 2/5 introduced the enum.  The biggest part of the patch (for now)
is that anything SHUTDOWN_CAUSE_HOST_ will be exposed to the QMP client
as host-triggered, anything SHUTDOWN_CAUSE_GUEST_ will be exposed as
guest-triggered.  I basically used SHUTDOWN_CAUSE_GUEST_RESET for any
call to qemu_system_reset_requst() underneath the hw/ tree, because the
hw/ tree is emulating guest behavior and therefore it is presumably a
reset caused by a guest request.

> 
> So it's essentially a firmware/hypervisor triggered reset, but one
> that should only ever be tripped during early guest boot.  Is
> CAUSE_GUEST_RESET correct for that?

Of course, I'm not an export on SPAPR, so I'll happily change it to
anything else if you think that is more appropriate. But the rule of
thumb I went by is whether this is qemu emulating a bare-metal
reset/shutdown, vs. qemu killing the guest without waiting for guest
instructions to reach some magic
memory/register/ACPI/who-knows-what-else request.  While it may happen
only early during guest boot, it is still the guest firmware that is
requesting it, and not qemu causing a unilateral death.

> 
> Apart from this, ppc changes
> 
> Acked-by: David Gibson 

Thanks.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

Re: [Xen-devel] [RFC PATCH v2 06/25] x86: NUMA: Add accessors for nodes[] and node_memblk_range[] structs


Hi Vijay,

On 28/03/17 16:53, vijay.kil...@gmail.com wrote:

From: Vijaya Kumar K 

Add accessor for nodes[] and other static variables and


s/accessor/accessors/


used those accessors.


Also, I am not sure to understand the usefulness of those accessors over 
a global variable.



Signed-off-by: Vijaya Kumar K 
---
 xen/arch/x86/srat.c | 108 +++-
 1 file changed, 82 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index ccacbcd..983e1d8 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -41,7 +41,45 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
 static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
 static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);

-static inline bool node_found(unsigned idx, unsigned pxm)
+static struct node *get_numa_node(int id)


unsigned int.


+{
+   return &nodes[id];
+}
+
+static nodeid_t get_memblk_nodeid(int id)


unsigned int.


+{
+   return memblk_nodeid[id];
+}
+
+static nodeid_t *get_memblk_nodeid_map(void)
+{
+   return &memblk_nodeid[0];
+}
+
+static struct node *get_node_memblk_range(int memblk)


unsigned int.


+{
+   return &node_memblk_range[memblk];
+}
+
+static int get_num_node_memblks(void)
+{
+   return num_node_memblks;
+}
+
+static int __init numa_add_memblk(nodeid_t nodeid, paddr_t start, uint64_t 
size)
+{
+   if (nodeid >= NR_NODE_MEMBLKS)
+   return -EINVAL;
+
+   node_memblk_range[num_node_memblks].start = start;
+   node_memblk_range[num_node_memblks].end = start + size;
+   memblk_nodeid[num_node_memblks] = nodeid;
+   num_node_memblks++;
+
+   return 0;
+}
+
+static inline bool node_found(unsigned int idx, unsigned int pxm)


Please don't make unrelated change in the same patch. In this case I 
don't see why you switch from "unsigned" to "unsigned int".



 {
return ((pxm2node[idx].pxm == pxm) &&
(pxm2node[idx].node != NUMA_NO_NODE));
@@ -107,11 +145,11 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t 
node)
 {
int i;

-   for (i = 0; i < num_node_memblks; i++) {
-   struct node *nd = &node_memblk_range[i];
+   for (i = 0; i < get_num_node_memblks(); i++) {
+   struct node *nd = get_node_memblk_range(i);

if (nd->start <= start && nd->end > end &&
-   memblk_nodeid[i] == node )
+   get_memblk_nodeid(i) == node)


Why the indentation changed here?


return 1;
}

@@ -122,8 +160,8 @@ static int __init conflicting_memblks(paddr_t start, 
paddr_t end)
 {
int i;

-   for (i = 0; i < num_node_memblks; i++) {
-   struct node *nd = &node_memblk_range[i];
+   for (i = 0; i < get_num_node_memblks(); i++) {
+   struct node *nd = get_node_memblk_range(i);
if (nd->start == nd->end)
continue;
if (nd->end > start && nd->start < end)
@@ -136,7 +174,8 @@ static int __init conflicting_memblks(paddr_t start, 
paddr_t end)

 static void __init cutoff_node(int i, paddr_t start, paddr_t end)
 {
-   struct node *nd = &nodes[i];
+   struct node *nd = get_numa_node(i);
+
if (nd->start < start) {
nd->start = start;
if (nd->end < nd->start)
@@ -278,6 +317,7 @@ acpi_numa_memory_affinity_init(const struct 
acpi_srat_mem_affinity *ma)
unsigned pxm;
nodeid_t node;
int i;
+   struct node *memblk;

if (srat_disabled())
return;
@@ -288,7 +328,7 @@ acpi_numa_memory_affinity_init(const struct 
acpi_srat_mem_affinity *ma)
if (!(ma->flags & ACPI_SRAT_MEM_ENABLED))
return;

-   if (num_node_memblks >= NR_NODE_MEMBLKS)
+   if (get_num_node_memblks() >= NR_NODE_MEMBLKS)
{
dprintk(XENLOG_WARNING,
 "Too many numa entry, try bigger NR_NODE_MEMBLKS \n");
@@ -310,27 +350,31 @@ acpi_numa_memory_affinity_init(const struct 
acpi_srat_mem_affinity *ma)
i = conflicting_memblks(start, end);
if (i < 0)
/* everything fine */;
-   else if (memblk_nodeid[i] == node) {
+   else if (get_memblk_nodeid(i) == node) {
bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
!test_bit(i, memblk_hotplug);

+   memblk = get_node_memblk_range(i);
+
printk("%sSRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with itself 
(%"PRIx64"-%"PRIx64")\n",
   mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
-  node_memblk_range[i].start, node_memblk_range[i].end);
+  memblk->start, memblk->end);
if (mismatch) {
bad_srat();
return;
}
} else {
+   memblk = get_

Re: [Xen-devel] Security response; public holidays [and 1 more messages]


> On 8 May 2017, at 15:29, Ian Jackson  wrote:
> 
> Ian Jackson writes ("Security response; public holidays"):
>> It would perhaps be possible to find a holiday calendar listing all
>> known public holidays throughout the world.  But we are interested in
>> public holidays affecting Xen Project community members.
>> 
>> I was wondering if we should start a wiki page and invite community
>> members to add their own local holidays.
> 
> I have now created this page:
>  https://wiki.xenproject.org/wiki/HolidayCalendar
> 
> I populated it with holidays from my own jurisdiction, the UK, to
> provide a skeleton.
> 
> Does that look about right ?  If so then we should send out a call on
> xen-announce for community members to add their own dates.
> 
> The policy doesn't explicitly ask us to avoid holidays.  It talks
> about a "usual starting point" and "reasons to diverge".  We have
> usually treated public holidays (at least, ones which are widely
> observed) as reasons to diverge and no-one seems to object to this
> practice.
> 
> If anyone things this needs to be made more explicit in the policy, I
> wouldn't oppose that, but personally I think what we have is OK.
> 
>> I suggest we set a reminder to send out a call for updates in around
>> October of the previous year.
> 
> I changed this to September and put it in the bottom of the page.

I think this makes sense. It is probably also worth looking at 
http://www.officeholidays.com/ (or something similar)

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

Re: [Xen-devel] [Qemu-devel] [PATCH v5 06/10] qobject: Use simpler QDict/QList scalar insertion macros

2017-05-08 Thread Alberto Garcia

On Fri 28 Apr 2017 10:33:36 AM CEST, Markus Armbruster wrote:
>> v5: rebase to master (Coccinelle found a couple new spots), squash 3
>> patches into 1, adjust R-b to only list Markus (while there were other
>> reviews on the pre-squashed patches, Markus was the only one on all 3)
>
> The block: part had
>
> Acked-by: Richard W.M. Jones 
> Reviewed-by: Stefan Hajnoczi 
> Reviewed-by: Alberto Garcia 
>
> The tests and qobject parts had
>
> Reviewed-by: Philippe Mathieu-Daudé 
>
> Richard, Stefan, Alberto, Philippe, let me know if you'd like me to
> convert your R-by of parts to an Acked-by of the combined patch.  Feel
> free to review the combined patch, of course.

You can keep my R-by, I just reviewed the combined patch.

Berto

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

Re: [Xen-devel] Security response; public holidays [and 1 more messages]

On 08/05/17 15:29, Ian Jackson wrote:
> Ian Jackson writes ("Security response; public holidays"):
>> It would perhaps be possible to find a holiday calendar listing all
>> known public holidays throughout the world.  But we are interested in
>> public holidays affecting Xen Project community members.
>>
>> I was wondering if we should start a wiki page and invite community
>> members to add their own local holidays.
> 
> I have now created this page:
>   https://wiki.xenproject.org/wiki/HolidayCalendar
> 
> I populated it with holidays from my own jurisdiction, the UK, to
> provide a skeleton.
> 
> Does that look about right ?  If so then we should send out a call on
> xen-announce for community members to add their own dates.

Looks decent -- I've added some US holidays.

 -George


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

Re: [Xen-devel] Security response; public holidays [and 1 more messages] [and 1 more messages]

Lars Kurth writes ("Re: Security response; public holidays [and 1 more 
messages]"):
> I think this makes sense. It is probably also worth looking at 
> http://www.officeholidays.com/ (or something similar)

I deliberately decided to avoid one of those portmanteau sites because
the set of holidays listed there may not be the set of ones that the
Xen Project Community cares about.  There is also an awkardness in
referring to a random third-party site with uncertain editorial
practices.

Ie I think the right approach is for community members who care about
this to record the holidays relevant to them in a Xen Project wiki
page.

Ian.

George Dunlap writes ("Re: Security response; public holidays [and 1 more 
messages]"):
> On 08/05/17 15:29, Ian Jackson wrote:
> > Does that look about right ?  If so then we should send out a call on
> > xen-announce for community members to add their own dates.
> 
> Looks decent -- I've added some US holidays.

Thanks.  I'll send out a formal call for contributions in a day or
two.

Ian.

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

Re: [Xen-devel] [PATCH] x86/mm: add temporary debugging code to get_page_from_gfn_p2m()

On 04/05/17 10:09, Jan Beulich wrote:
> See the code comment.
> 
> Signed-off-by: Jan Beulich 

Acked-by: George Dunlap 

> 
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -480,6 +480,22 @@ struct page_info *get_page_from_gfn_p2m(
>  p2m_access_t _a;
>  p2m_type_t _t;
>  mfn_t mfn;
> +#ifndef NDEBUG
> +/*
> + * Temporary debugging code, added in the hope of finding the origin
> + * of calls to get_page(..., dom_cow) as observed during osstest
> + * migration failures (see
> + * lists.xenproject.org/archives/html/xen-devel/2017-04/msg03331.html).
> + */
> +static unsigned long cnt, thr;
> +
> +if ( d->is_dying && ++cnt > thr )
> +{
> +thr |= cnt;
> +printk("%pv: d%d dying (looking up %lx)\n", current, d->domain_id, 
> gfn);
> +dump_execution_state();
> +}
> +#endif
>  
>  /* Allow t or a to be NULL */
>  t = t ?: &_t;
> 
> 
> 


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

Re: [Xen-devel] Security response; public holidays [and 1 more messages] [and 1 more messages]

On 08/05/17 15:50, Ian Jackson wrote:
> Lars Kurth writes ("Re: Security response; public holidays [and 1 more 
> messages]"):
>> I think this makes sense. It is probably also worth looking at 
>> http://www.officeholidays.com/ (or something similar)
> 
> I deliberately decided to avoid one of those portmanteau sites because
> the set of holidays listed there may not be the set of ones that the
> Xen Project Community cares about.  There is also an awkardness in
> referring to a random third-party site with uncertain editorial
> practices.
> 
> Ie I think the right approach is for community members who care about
> this to record the holidays relevant to them in a Xen Project wiki
> page.

+1 to this.  Those might be good prompts for people to remember their
own national holidays which are important; but we're not really in a
position to tell which holidays people actually affect people's
availability.

 -George


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

Re: [Xen-devel] Security response; public holidays [and 1 more messages] [and 1 more messages]


> On 8 May 2017, at 16:07, George Dunlap  wrote:
> 
> On 08/05/17 15:50, Ian Jackson wrote:
>> Lars Kurth writes ("Re: Security response; public holidays [and 1 more 
>> messages]"):
>>> I think this makes sense. It is probably also worth looking at 
>>> http://www.officeholidays.com/ (or something similar)
>> 
>> I deliberately decided to avoid one of those portmanteau sites because
>> the set of holidays listed there may not be the set of ones that the
>> Xen Project Community cares about.  There is also an awkardness in
>> referring to a random third-party site with uncertain editorial
>> practices.
>> 
>> Ie I think the right approach is for community members who care about
>> this to record the holidays relevant to them in a Xen Project wiki
>> page.

Agreed: this was more about checking what people add to the wiki page.

> +1 to this.  Those might be good prompts for people to remember their
> own national holidays which are important; but we're not really in a
> position to tell which holidays people actually affect people's
> availability.

+1

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

[Xen-devel] [ovmf baseline-only test] 71264: tolerable trouble: blocked/broken/fail/pass

2017-05-08 Thread Platform Team regression test user

This run is configured for baseline tests only.

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-xsm   3 host-install(3)   broken baseline untested
 build-amd64-pvops 3 host-install(3)   broken baseline untested
 build-amd64   3 host-install(3)   broken baseline untested
 build-i386-libvirt5 libvirt-build   fail baseline untested

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

version targeted for testing:
 ovmf 3654c4623c9b37ddc64426995fa399d8626b7ced
baseline version:
 ovmf d7b96017ccf5922b798f496fbcdcac4067d04c6d

Last test of basis71262  2017-05-08 03:16:38 Z0 days
Testing same since71264  2017-05-08 08:17:59 Z0 days1 attempts


People who touched revisions under test:
  Jeff Fan 
  Ruiyu Ni 

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



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

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

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

broken-step build-amd64-xsm host-install(3)
broken-step build-amd64-pvops host-install(3)
broken-step build-amd64 host-install(3)

Push not applicable.


commit 3654c4623c9b37ddc64426995fa399d8626b7ced
Author: Ruiyu Ni 
Date:   Fri May 5 10:10:00 2017 +0800

UefiCpuPkg/MtrrLib: Don't report OutOfResource when MTRR is enough

The MTRR calculation algorithm contains a bug that when left
subtraction cannot produce better MTRR solution, it forgets
to restore the BaseAddress/Length so that MtrrLibGetMtrrNumber()
returns bigger value of actual required MTRR numbers.
As a result, the MtrrLib reports OutOfResource but actually the
MTRR is enough.

MEMORY_RANGE mC[] = {
  0, 0x10, CacheUncacheable,
  0x10, 0x89F0, CacheWriteBack,
  0x8A00, 0x7500, CacheUncacheable,
  0xFF00, 0x0100, CacheWriteProtected,
  0x1, 0x7F, CacheUncacheable,
  0xFC24, 0x2000, CacheWriteCombining // <-- trigger the error
};

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Reviewed-by: Jeff Fan 

commit 007b51e180d2202637dfc6d2afbe8a34704c423f
Author: Jeff Fan 
Date:   Mon May 8 13:31:00 2017 +0800

UefiCpuPkg: Update package version to 0.80

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

commit 93041972617af5f538fac8291e7a57b7766c6e10
Author: Jeff Fan 
Date:   Mon May 8 11:31:50 2017 +0800

UefiCpuPkg: Update package version to 0.80

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

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

Re: [Xen-devel] [PATCH] x86/ioreq server: Fix DomU couldn't reboot when using p2m_ioreq_server p2m_type

>>> On 08.05.17 at 15:55,  wrote:
> On 08/05/17 14:26, George Dunlap wrote:
>> On 05/05/17 15:40, Jan Beulich wrote:
>> On 05.05.17 at 05:52,  wrote:
 'commit 1679e0df3df6 ("x86/ioreq server: asynchronously reset
 outstanding p2m_ioreq_server entries")' will call
 p2m_change_entry_type_global() which set entry.recalc=1. Then
 the following get_entry(p2m_ioreq_server) will return
 p2m_ram_rw type.
 But 'commit 6d774a951696 ("x86/ioreq server: synchronously reset
 outstanding p2m_ioreq_server entries when an ioreq server unmaps")'
 assume get_entry(p2m_ioreq_server) will return p2m_ioreq_server
 type, then reset p2m_ioreq_server entries. The fact is the assumption
 isn't true, and sysnchronously reset function couldn't work. Then
 ioreq.entry_count is larger than zero after an ioreq server unmaps,
 finally this results DomU couldn't reboot.
>>>
>>> I've had trouble understanding this part already on v1 (btw, why is
>>> this one not tagged v2?), and since I still can't figure it I have to ask:
>>> Why is it that guest reboot is being impacted here? From what I recall
>>> a non-zero count should only prevent migration.
>>>
 This patch add a P2M_PRE_RECALC flag to p2m_query_t, then
 get_entry(P2M_PRE_RECALC) will return p2m_ioreq_server type
 for p2m_ioreq_server pfn, and finally change mem type through set_entry.
>>>
>>> This looks to be a relatively little impact change, but nevertheless
>>> I'm wondering whether someone else (George?) may be able to
>>> think of some more elegant solution (I have to admit that, having
>>> suggested the one here, I can't).
>> 
>> So the basic problem is that the implementation-specific get_entry()
>> will return the theoretical new type without changing it, and
>> finish_type_change() is in the generic code which calls these
>> implementation-specific p2m functions.
>> 
>> I think the possible solutions are:
>> 
>> 1. Add a flag to the get_entry() call to return the real entry (i.e.,
>> the solution in this patch)
>> 
>> 2. Have get_entry() update entries as it reads them
>> 
>> 3. Implement an implementation-specific finish_type_change()
> 
> Actually -- there are already implementation-specific versions for
> individual gpas: p2m-pt.c:do_recalc() and p2m-ept.c:resolve_misconfig().
>  They even already have the same function signature
> 
> If we made a new p2m hook, p2m->recalc(), we could simply loop around
> calling that instead of doing the get_entry() / set_entry() cycle.

Ah, that's a neat idea indeed.

Jan


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

[Xen-devel] [PATCH 1/4] x86/pv: Drop int80_bounce from struct pv_vcpu

The int80_bounce field of struct pv_vcpu is a bit of an odd special case,
because it is a simple derivation of trap_ctxt[0x80], which is also stored.

It is also the only use of {compat_,}create_bounce_frame() which isn't
referencing the plain trap_bounce field of struct pv_vcpu.  (And altering this
property the purpose of this patch.)

Remove the int80_bounce field entirely, along with init_int80_direct_trap(),
which in turn requires that the int80_direct_trap() path gain logic previously
contained in init_int80_direct_trap().

This does admittedly make the int80 fastpath slightly longer, but these few
instructions are in the noise compared to the architectural context switch
overhead, and it now matches the syscall/sysenter paths (which have far less
architectural overhead already).

No behavioural change from the guests point of view.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/domain.c  |  2 --
 xen/arch/x86/traps.c   |  4 
 xen/arch/x86/x86_64/asm-offsets.c  |  1 -
 xen/arch/x86/x86_64/compat/traps.c |  4 
 xen/arch/x86/x86_64/entry.S| 32 
 xen/arch/x86/x86_64/traps.c| 14 --
 xen/include/asm-x86/domain.h   |  1 -
 xen/include/asm-x86/processor.h|  2 --
 8 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 2ef1c9f..7b301e3 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -833,8 +833,6 @@ int arch_set_info_guest(
 goto out;
 }
 
-init_int80_direct_trap(v);
-
 /* IOPL privileges are virtualised. */
 v->arch.pv_vcpu.iopl = v->arch.user_regs.eflags & X86_EFLAGS_IOPL;
 v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 27fdf12..ece2c13 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -4013,7 +4013,6 @@ long 
do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
 if ( guest_handle_is_null(traps) )
 {
 memset(dst, 0, NR_VECTORS * sizeof(*dst));
-init_int80_direct_trap(curr);
 return 0;
 }
 
@@ -4035,9 +4034,6 @@ long 
do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
 
 memcpy(&dst[cur.vector], &cur, sizeof(cur));
 
-if ( cur.vector == 0x80 )
-init_int80_direct_trap(curr);
-
 guest_handle_add_offset(traps, 1);
 
 if ( hypercall_preempt_check() )
diff --git a/xen/arch/x86/x86_64/asm-offsets.c 
b/xen/arch/x86/x86_64/asm-offsets.c
index e136af6..adf2749 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -61,7 +61,6 @@ void __dummy__(void)
 OFFSET(VCPU_domain, struct vcpu, domain);
 OFFSET(VCPU_vcpu_info, struct vcpu, vcpu_info);
 OFFSET(VCPU_trap_bounce, struct vcpu, arch.pv_vcpu.trap_bounce);
-OFFSET(VCPU_int80_bounce, struct vcpu, arch.pv_vcpu.int80_bounce);
 OFFSET(VCPU_thread_flags, struct vcpu, arch.flags);
 OFFSET(VCPU_event_addr, struct vcpu, arch.pv_vcpu.event_callback_eip);
 OFFSET(VCPU_event_sel, struct vcpu, arch.pv_vcpu.event_callback_cs);
diff --git a/xen/arch/x86/x86_64/compat/traps.c 
b/xen/arch/x86/x86_64/compat/traps.c
index 1751ec6..0ab2c31 100644
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -339,7 +339,6 @@ int 
compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
 if ( guest_handle_is_null(traps) )
 {
 memset(dst, 0, NR_VECTORS * sizeof(*dst));
-init_int80_direct_trap(current);
 return 0;
 }
 
@@ -358,9 +357,6 @@ int 
compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
 
 XLAT_trap_info(dst + cur.vector, &cur);
 
-if ( cur.vector == 0x80 )
-init_int80_direct_trap(current);
-
 guest_handle_add_offset(traps, 1);
 
 if ( hypercall_preempt_check() )
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 65c771f..57952d0 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -233,12 +233,36 @@ UNLIKELY_END(msi_check)
 
 GET_CURRENT(bx)
 
-/* Check that the callback is non-null. */
-leaq  VCPU_int80_bounce(%rbx),%rdx
-cmpb  $0,TRAPBOUNCE_flags(%rdx)
+mov   VCPU_trap_ctxt(%rbx), %rsi
+mov   VCPU_domain(%rbx), %rax
+
+/*
+ * if ( null_trap_bounce(v, &v->arch.pv_vcpu.trap_ctxt[0x80]) )
+ *goto int80_slow_path;
+ */
+mov0x80 * TRAPINFO_sizeof + TRAPINFO_eip(%rsi), %rdi
+movzwl 0x80 * TRAPINFO_sizeof + TRAPINFO_cs (%rsi), %ecx
+
+mov   %ecx, %edx
+and   $~3, %edx
+
+testb $1, DOMAIN_is_32bit_pv(%rax)
+cmove %rdi, %rdx
+
+test  %rdx, %rdx
 jzint80_slow_path
 
-movq  VCPU_domain(%rbx),%rax
+/* Construct trap_bounce from trap_ctxt[0x80]. */
+lea   VCPU_trap_bounce(%

[Xen-devel] [PATCH RFC for-next 0/4] Reimpliement {compat_, }create_bounce_frame() in C

Bloat-o-meter reports:
  add/remove: 1/1 grow/shrink: 0/1 up/down: 1211/-2587 (-1376)
  function old new   delta
  pv_create_exception_frame  -1211   +1211
  asm_domain_crash_synchronous  65   - -65
  context_switch  35651043   -2522

but as the assembly symbols aren't tagged with type/size information, it can't
spot the removal of {compat_,}create_bounce_frame() at all.

This series is based on top of the "[PATCH for-4.9 0/2] x86/pv: Misc fixes"
series, which were discovered while developing this series.

Andrew Cooper (4):
  x86/pv: Drop int80_bounce from struct pv_vcpu
  x86/pv: Introduce pv_create_exception_frame()
  x86/pv: Drop {compat_,}create_bounce_frame() and use the C version instead
  x86/pv: Implement the failsafe callback using the general path

 xen/arch/x86/domain.c  | 102 ++
 xen/arch/x86/pv/traps.c| 152 
 xen/arch/x86/traps.c   |  27 --
 xen/arch/x86/x86_64/asm-offsets.c  |   1 -
 xen/arch/x86/x86_64/compat/entry.S | 116 ++---
 xen/arch/x86/x86_64/compat/traps.c |   4 -
 xen/arch/x86/x86_64/entry.S| 173 +++--
 xen/arch/x86/x86_64/traps.c|  14 ---
 xen/include/asm-x86/domain.h   |   1 -
 xen/include/asm-x86/processor.h|   3 +-
 xen/include/xen/sched.h|   7 --
 11 files changed, 198 insertions(+), 402 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH 3/4] x86/pv: Drop {compat_, }create_bounce_frame() and use the C version instead

The clobbering of TRAPBOUNCE_flags in .L{compat_}bounce_exception is subsumed
by the logic at the end of pv_create_bounce_frame().

This cleanup removes all callers of asm_domain_crash_synchronous(), which is
therefore dropped as well.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/traps.c   |  23 --
 xen/arch/x86/x86_64/compat/entry.S | 116 ++
 xen/arch/x86/x86_64/entry.S| 141 ++---
 xen/include/xen/sched.h|   7 --
 4 files changed, 10 insertions(+), 277 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index ece2c13..73a9c7c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -4195,29 +4195,6 @@ unsigned long do_get_debugreg(int reg)
 return -EINVAL;
 }
 
-void asm_domain_crash_synchronous(unsigned long addr)
-{
-/*
- * We need clear AC bit here because in entry.S AC is set
- * by ASM_STAC to temporarily allow accesses to user pages
- * which is prevented by SMAP by default.
- *
- * For some code paths, where this function is called, clac()
- * is not needed, but adding clac() here instead of each place
- * asm_domain_crash_synchronous() is called can reduce the code
- * redundancy, and it is harmless as well.
- */
-clac();
-
-if ( addr == 0 )
-addr = this_cpu(last_extable_addr);
-
-printk("domain_crash_sync called from entry.S: fault at %p %pS\n",
-   _p(addr), _p(addr));
-
-__domain_crash_synchronous();
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86/x86_64/compat/entry.S
index 90bda09..1cd4672 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -51,7 +51,7 @@ compat_test_guest_events:
 movl  VCPU_event_sel(%rbx),%eax
 movw  %ax,TRAPBOUNCE_cs(%rdx)
 movb  $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-call  compat_create_bounce_frame
+call  pv_create_exception_frame
 jmp   compat_test_all_events
 
 ALIGN
@@ -95,7 +95,7 @@ compat_process_nmi:
 /* FALLTHROUGH */
 compat_process_trap:
 leaq  VCPU_trap_bounce(%rbx),%rdx
-call  compat_create_bounce_frame
+call  pv_create_exception_frame
 jmp   compat_test_all_events
 
 /* %rbx: struct vcpu, interrupts disabled */
@@ -181,8 +181,7 @@ ENTRY(compat_post_handle_exception)
 testb $TBF_EXCEPTION,TRAPBOUNCE_flags(%rdx)
 jzcompat_test_all_events
 .Lcompat_bounce_exception:
-call  compat_create_bounce_frame
-movb  $0,TRAPBOUNCE_flags(%rdx)
+call  pv_create_exception_frame
 jmp   compat_test_all_events
 
 /* See lstar_enter for entry register state. */
@@ -234,115 +233,10 @@ ENTRY(compat_sysenter)
 movl  $FLAT_COMPAT_USER_SS,UREGS_ss(%rsp)
 cmovzl %ecx,%eax
 movw  %ax,TRAPBOUNCE_cs(%rdx)
-call  compat_create_bounce_frame
+call  pv_create_exception_frame
 jmp   compat_test_all_events
 
 ENTRY(compat_int80_direct_trap)
 CR4_PV32_RESTORE
-call  compat_create_bounce_frame
+call  pv_create_exception_frame
 jmp   compat_test_all_events
-
-/* CREATE A BASIC EXCEPTION FRAME ON GUEST OS (RING-1) STACK:*/
-/*   {[ERRCODE,] EIP, CS, EFLAGS, [ESP, SS]} */
-/* %rdx: trap_bounce, %rbx: struct vcpu  */
-/* On return only %rbx and %rdx are guaranteed non-clobbered.*/
-compat_create_bounce_frame:
-ASSERT_INTERRUPTS_ENABLED
-mov   %fs,%edi
-ASM_STAC
-testb $2,UREGS_cs+8(%rsp)
-jz1f
-/* Push new frame at registered guest-OS stack base. */
-movl  VCPU_kernel_sp(%rbx),%esi
-.Lft1:  mov   VCPU_kernel_ss(%rbx),%fs
-subl  $2*4,%esi
-movl  UREGS_rsp+8(%rsp),%eax
-.Lft2:  movl  %eax,%fs:(%rsi)
-movl  UREGS_ss+8(%rsp),%eax
-.Lft3:  movl  %eax,%fs:4(%rsi)
-jmp   2f
-1:  /* In kernel context already: push new frame at existing %rsp. */
-movl  UREGS_rsp+8(%rsp),%esi
-.Lft4:  mov   UREGS_ss+8(%rsp),%fs
-2:
-movq  VCPU_domain(%rbx),%r8
-subl  $3*4,%esi
-movq  VCPU_vcpu_info(%rbx),%rax
-pushq COMPAT_VCPUINFO_upcall_mask(%rax)
-testb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-setnz %ch   # TBF_INTERRUPT -> set upcall mask
-orb   %ch,COMPAT_VCPUINFO_upcall_mask(%rax)
-popq  %rax
-shll  $16,%eax  # Bits 16-23: saved_upcall_mask
-movw  UREGS_cs+8(%rsp),%ax  # Bits  0-15: CS
-.Lft5:  movl  %eax,%fs:4(%rsi)  # CS / saved_upcall_mask
-shrl  $16,%eax
-testb %al,%al   # Bits 0-7: saved_upcall_mask
-setz  %ch   # %ch == !saved_upcall_mask
-movl  UREGS_eflags+8(%rsp),%eax
-andl  $~(X86_EFLAGS_I

[Xen-devel] [PATCH 4/4] x86/pv: Implement the failsafe callback using the general path

Reintroduce TBF_FAILSAFE and update pv_create_exception_frame() to cope with
the additional data segment registers.

load_segments() now fills in trap_bounce, and lets the general return-to-guest
path inject the exception.

Bloat-o-meter reports:
  add/remove: 0/0 grow/shrink: 1/1 up/down: 123/-2522 (-2399)
  function old new   delta
  pv_create_exception_frame   10881211+123
  context_switch  35651043   -2522

which I suspect is largely due to the quantity of code hidden behind
put_user().

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/domain.c   | 100 +++-
 xen/arch/x86/pv/traps.c |  31 ++---
 xen/include/asm-x86/processor.h |   1 +
 3 files changed, 33 insertions(+), 99 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7b301e3..c533e05 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1275,100 +1275,14 @@ static void load_segments(struct vcpu *n)
 
 if ( unlikely(!all_segs_okay) )
 {
-struct pv_vcpu *pv = &n->arch.pv_vcpu;
-struct cpu_user_regs *regs = guest_cpu_user_regs();
-unsigned long *rsp =
-(unsigned long *)(((n->arch.flags & TF_kernel_mode)
-   ? regs->rsp : pv->kernel_sp) & ~0xf);
-unsigned long cs_and_mask, rflags;
-
-/* Fold upcall mask and architectural IOPL into RFLAGS.IF. */
-rflags  = regs->rflags & ~(X86_EFLAGS_IF|X86_EFLAGS_IOPL);
-rflags |= !vcpu_info(n, evtchn_upcall_mask) << 9;
-if ( VM_ASSIST(n->domain, architectural_iopl) )
-rflags |= n->arch.pv_vcpu.iopl;
-
-if ( is_pv_32bit_vcpu(n) )
-{
-unsigned int *esp = ring_1(regs) ?
-(unsigned int *)regs->rsp :
-(unsigned int *)pv->kernel_sp;
-int ret = 0;
-
-/* CS longword also contains full evtchn_upcall_mask. */
-cs_and_mask = (unsigned short)regs->cs |
-((unsigned int)vcpu_info(n, evtchn_upcall_mask) << 16);
-
-if ( !ring_1(regs) )
-{
-ret  = put_user(regs->ss,   esp-1);
-ret |= put_user(regs->esp,  esp-2);
-esp -= 2;
-}
-
-if ( ret |
- put_user(rflags,  esp-1) |
- put_user(cs_and_mask, esp-2) |
- put_user(regs->eip,   esp-3) |
- put_user(uregs->gs,   esp-4) |
- put_user(uregs->fs,   esp-5) |
- put_user(uregs->es,   esp-6) |
- put_user(uregs->ds,   esp-7) )
-{
-gprintk(XENLOG_ERR,
-"error while creating compat failsafe callback 
frame\n");
-domain_crash(n->domain);
-}
+bool disable = n->arch.vgc_flags & VGCF_failsafe_disables_events;
 
-if ( n->arch.vgc_flags & VGCF_failsafe_disables_events )
-vcpu_info(n, evtchn_upcall_mask) = 1;
-
-regs->entry_vector |= TRAP_syscall;
-regs->eflags   &= ~(X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
-X86_EFLAGS_IOPL|X86_EFLAGS_TF);
-regs->ss= FLAT_COMPAT_KERNEL_SS;
-regs->esp   = (unsigned long)(esp-7);
-regs->cs= FLAT_COMPAT_KERNEL_CS;
-regs->eip   = pv->failsafe_callback_eip;
-return;
-}
-
-if ( !(n->arch.flags & TF_kernel_mode) )
-toggle_guest_mode(n);
-else
-regs->cs &= ~3;
-
-/* CS longword also contains full evtchn_upcall_mask. */
-cs_and_mask = (unsigned long)regs->cs |
-((unsigned long)vcpu_info(n, evtchn_upcall_mask) << 32);
-
-if ( put_user(regs->ss,rsp- 1) |
- put_user(regs->rsp,   rsp- 2) |
- put_user(rflags,  rsp- 3) |
- put_user(cs_and_mask, rsp- 4) |
- put_user(regs->rip,   rsp- 5) |
- put_user(uregs->gs,   rsp- 6) |
- put_user(uregs->fs,   rsp- 7) |
- put_user(uregs->es,   rsp- 8) |
- put_user(uregs->ds,   rsp- 9) |
- put_user(regs->r11,   rsp-10) |
- put_user(regs->rcx,   rsp-11) )
-{
-gprintk(XENLOG_ERR,
-"error while creating failsafe callback frame\n");
-domain_crash(n->domain);
-}
-
-if ( n->arch.vgc_flags & VGCF_failsafe_disables_events )
-vcpu_info(n, evtchn_upcall_mask) = 1;
-
-regs->entry_vector |= TRAP_syscall;
-regs->rflags   &= ~(X86_EFLAGS_AC|X86_EFLAGS_V

[Xen-devel] [PATCH 2/4] x86/pv: Introduce pv_create_exception_frame()

This is a C implementation of {compat_,}create_bounce_frame(), based loosely
on the existing failsafe implementation in load_segments().  It picks up all
injection information from the trap_bounce structure.

One minor improvement is that at not point is regs->cs left with an rpl of 0
on the root stack frame.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/pv/traps.c | 133 
 1 file changed, 133 insertions(+)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 51125a8..8973b23 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -32,6 +32,139 @@ void do_entry_int82(struct cpu_user_regs *regs)
 }
 
 /*
+ * This function emulates the behaviour of hardware when Xen needs to inject
+ * an event into into a guest.
+ *
+ * It may switch from user mode to kernel mode, will write an appropriate
+ * hardware exception frame (including Xen-specific extras), and alter the
+ * root stack frame to invoke the guest kernels correct entry point on exit
+ * from the hypervisor.
+ */
+void pv_create_exception_frame(void)
+{
+struct vcpu *curr = current;
+struct trap_bounce *tb = &curr->arch.pv_vcpu.trap_bounce;
+struct cpu_user_regs *regs = guest_cpu_user_regs();
+const bool user_mode_frame = !guest_kernel_mode(curr, regs);
+uint8_t *evt_mask = &vcpu_info(curr, evtchn_upcall_mask);
+unsigned long rflags;
+unsigned int bytes, missing;
+
+ASSERT_NOT_IN_ATOMIC();
+
+if ( unlikely(null_trap_bounce(curr, tb)) )
+{
+gprintk(XENLOG_ERR, "Fatal: Attempting to inject null trap bounce\n");
+__domain_crash_synchronous();
+}
+
+/* Fold the upcall mask and architectural IOPL into the guests rflags. */
+rflags  = regs->rflags & ~(X86_EFLAGS_IF | X86_EFLAGS_IOPL);
+rflags |= ((*evt_mask ? 0 : X86_EFLAGS_IF) |
+   (VM_ASSIST(curr->domain, architectural_iopl)
+? curr->arch.pv_vcpu.iopl : 0));
+
+if ( is_pv_32bit_vcpu(curr) )
+{
+/* { [ERRCODE,] EIP, CS/MASK , EFLAGS, [ESP, SS] } */
+unsigned int frame[6], *ptr = frame, ksp =
+(user_mode_frame ? curr->arch.pv_vcpu.kernel_sp : regs->esp);
+
+if ( tb->flags & TBF_EXCEPTION_ERRCODE )
+*ptr++ = tb->error_code;
+
+*ptr++ = regs->eip;
+*ptr++ = regs->cs | (((unsigned int)*evt_mask) << 16);
+*ptr++ = rflags;
+
+if ( user_mode_frame )
+{
+*ptr++ = regs->esp;
+*ptr++ = regs->ss;
+}
+
+/* Copy the constructed frame to the guest kernel stack. */
+bytes = _p(ptr) - _p(frame);
+ksp -= bytes;
+
+if ( unlikely((missing = __copy_to_user(_p(ksp), frame, bytes)) != 0) )
+{
+gprintk(XENLOG_ERR, "Fatal: Fault while writing exception 
frame\n");
+show_page_walk(ksp + missing);
+__domain_crash_synchronous();
+}
+
+/* Rewrite our stack frame. */
+regs->rip   = (uint32_t)tb->eip;
+regs->cs= tb->cs;
+regs->eflags   &= ~(X86_EFLAGS_VM | X86_EFLAGS_RF |
+X86_EFLAGS_NT | X86_EFLAGS_TF);
+regs->rsp   = ksp;
+if ( user_mode_frame )
+regs->ss = curr->arch.pv_vcpu.kernel_ss;
+}
+else
+{
+/* { RCX, R11, [ERRCODE,] RIP, CS/MASK, RFLAGS, RSP, SS } */
+unsigned long frame[7], *ptr = frame, ksp =
+(user_mode_frame ? curr->arch.pv_vcpu.kernel_sp : regs->rsp) & 
~0xf;
+
+if ( user_mode_frame )
+toggle_guest_mode(curr);
+
+*ptr++ = regs->rcx;
+*ptr++ = regs->r11;
+
+if ( tb->flags & TBF_EXCEPTION_ERRCODE )
+*ptr++ = tb->error_code;
+
+*ptr++ = regs->rip;
+*ptr++ = (user_mode_frame ? regs->cs : regs->cs & ~3) |
+((unsigned long)(*evt_mask) << 32);
+*ptr++ = rflags;
+*ptr++ = regs->rsp;
+*ptr++ = regs->ss;
+
+/* Copy the constructed frame to the guest kernel stack. */
+bytes = _p(ptr) - _p(frame);
+ksp -= bytes;
+
+if ( unlikely(!__addr_ok(ksp)) )
+{
+gprintk(XENLOG_ERR, "Fatal: Bad guest kernel stack %p\n", _p(ksp));
+__domain_crash_synchronous();
+}
+else if ( unlikely((missing =
+__copy_to_user(_p(ksp), frame, bytes)) != 0) )
+{
+gprintk(XENLOG_ERR, "Fatal: Fault while writing exception 
frame\n");
+show_page_walk(ksp + missing);
+__domain_crash_synchronous();
+}
+
+/* Rewrite our stack frame. */
+regs->entry_vector |= TRAP_syscall;
+regs->rip   = tb->eip;
+regs->cs= FLAT_KERNEL_CS;
+regs->rflags   &= ~(X86_EFLAGS_AC | X86_EFLAGS_VM | X86_EFLAGS_RF |
+X86_EFLAGS_NT | X86_EFLAGS_TF);
+regs->rsp

Re: [Xen-devel] [PATCH v3 9/9] mm: Make sure pages are scrubbed

2017-05-08 Thread Konrad Rzeszutek Wilk

On Fri, May 05, 2017 at 09:05:54AM -0600, Jan Beulich wrote:
> >>> On 14.04.17 at 17:37,  wrote:
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -114,6 +114,13 @@ config DEVICE_TREE_DEBUG
> >   logged in the Xen ring buffer.
> >   If unsure, say N here.
> >  
> > +config SCRUB_DEBUG
> > +bool "Page scrubbing test"
> > +default DEBUG
> > +---help---
> > +  Verify that pages that need to be scrubbed before being allocated to
> > +  a guest are indeed scrubbed.
> 
> Indentation.
> 
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -694,6 +694,31 @@ static void page_list_add_scrub(struct page_info *pg, 
> > unsigned int node,
> >  page_list_add(pg, &heap(node, zone, order));
> >  }
> >  
> > +#define SCRUB_BYTE_PATTERN 0xc2c2c2c2c2c2c2c2
> 
> This likely needs a ULL suffix at least for the ARM32 build.

See 3ac7ca1

So 0xe7f000f0 for ARM32 and AARCH64_BREAK_FAULT for ARM64.

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

Re: [Xen-devel] [RFC PATCH v2 09/25] ARM: NUMA: Add existing ARM numa code under CONFIG_NUMA


Hi Vijay,

On 28/03/17 16:53, vijay.kil...@gmail.com wrote:

From: Vijaya Kumar K 

Right now CONFIG_NUMA is not enabled for ARM and
existing code in asm-arm/numa.h is for !CONFIG_NUMA.
Hence put this code under #ifndef CONFIG_NUMA.

This help to make this changes work when CONFIG_NUMA
is not enabled.


But you always turn NUMA on by default (see patch #24) and there is no 
possibility to turn off NUMA.




Also define NODES_SHIFT macro for ARM to value 2.
This limits number of NUMA nodes supported to 4.
There is not hard restrictions on this value set to 2.


Again, why only 2 when x86 is supporting 6?

Furthermore, this is not related to this patch itself and should be part 
of separate patch.


Lastly, why don't you move that to a Kconfig allowing the user to 
configure the number of Nodes?




Signed-off-by: Vijaya Kumar K 
---
 xen/include/asm-arm/numa.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index 53f99af..924bfc0 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -3,6 +3,10 @@

 typedef uint8_t nodeid_t;

+/* Limit number of NUMA nodes supported to 4 */
+#define NODES_SHIFT 2


Why this is not covered by CONFIG_NUMA?


+
+#ifndef CONFIG_NUMA
 /* Fake one node for now. See also node_online_map. */
 #define cpu_to_node(cpu) 0
 #define node_to_cpumask(node)   (cpu_online_map)
@@ -16,6 +20,7 @@ static inline __attribute__((pure)) nodeid_t 
phys_to_nid(paddr_t addr)
 #define node_spanned_pages(nid) (total_pages)
 #define node_start_pfn(nid) (pdx_to_pfn(frametable_base_pdx))
 #define __node_distance(a, b) (20)
+#endif /* CONFIG_NUMA */

 static inline unsigned int arch_get_dma_bitsize(void)
 {



Cheers,

--
Julien Grall

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

Re: [Xen-devel] Security support scope (apropos of Xen and CNA)

Tim Deegan writes ("Re: Security support scope (apropos of Xen and CNA)"):
> Ah, so it is.  So there is information on the wiki page that's not in
> MAINTAINERS.  Could that be moved into MAINTAINERS?  There are
> a few things that don't map well to maintainership of specific
> files, e.g. "vMCE" or nested virtualization.  But on the whole I
> think that adding clauses for them would be OK.

I think this is quite awkward, really.  MAINTAINERS is about files,
and implementations.  The security support status is about parts of
interfaces, which don't map at all well.

We could add no-files stanzas, but how would you tell what they
referred to ?

Ian.

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

Re: [Xen-devel] Security support scope (apropos of Xen and CNA)

On 08/05/17 17:04, Ian Jackson wrote:
> Tim Deegan writes ("Re: Security support scope (apropos of Xen and CNA)"):
>> Ah, so it is.  So there is information on the wiki page that's not in
>> MAINTAINERS.  Could that be moved into MAINTAINERS?  There are
>> a few things that don't map well to maintainership of specific
>> files, e.g. "vMCE" or nested virtualization.  But on the whole I
>> think that adding clauses for them would be OK.
> I think this is quite awkward, really.  MAINTAINERS is about files,
> and implementations.  The security support status is about parts of
> interfaces, which don't map at all well.
>
> We could add no-files stanzas, but how would you tell what they
> referred to ?

This is the principle behind introducing docs/features/* which, as part
of the required metadata, contains a support statement.

This way, there is an authoritative statement of support on a
per-feature basis which is easy to keep up to date.

Lars: Any update on your project level clarifications of support statuses?

~Andrew

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

[Xen-devel] [PATCH] x86/efi: Reserve EFI properties table

2017-05-08 Thread Ross Lagerwall

Some EFI firmware implementations may place the EFI properties table in
RAM marked as BootServicesData, which Xen does not consider as reserved.
When dom0 tries to access the EFI properties table (which Linux >= 4.4
does), it crashes with a page fault.  Fix this by unconditionally
marking the EFI properties table as reserved in the E820, much like is
done with the dmi regions.

Signed-off-by: Ross Lagerwall 
---
 xen/arch/x86/efi/efi-boot.h |  8 
 xen/common/efi/boot.c   | 20 
 xen/common/efi/efi.h|  4 
 3 files changed, 32 insertions(+)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 34537d4..36ae464 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -209,6 +209,14 @@ static void __init 
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 }
 }
 
+if ( efi_properties_tbl_addr && efi_properties_tbl_size )
+{
+++e;
+e->addr = efi_properties_tbl_addr;
+e->size = efi_properties_tbl_size;
+e->type = E820_RESERVED;
+++e820_raw.nr_map;
+}
 }
 
 static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index daf0c80..1fb396b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -40,6 +40,8 @@
   { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 
0x23} }
 #define APPLE_PROPERTIES_PROTOCOL_GUID \
   { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 
0xe0} }
+#define EFI_PROPERTIES_TABLE_GUID \
+  { 0x880aaca3, 0x4adc, 0x4a04, { 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 
0xe5} }
 
 typedef EFI_STATUS
 (/* _not_ EFIAPI */ *EFI_SHIM_LOCK_VERIFY) (
@@ -171,6 +173,15 @@ static char __section(".bss.page_aligned") 
__aligned(PAGE_SIZE)
 ebmalloc_mem[EBMALLOC_SIZE];
 static unsigned long __initdata ebmalloc_allocated;
 
+struct efi_properties_table {
+u32 version;
+u32 length;
+u64 memory_protection_attribute;
+};
+
+u64 __initdata efi_properties_tbl_addr;
+u32 __initdata efi_properties_tbl_size;
+
 /* EFI boot allocator. */
 static void __init __maybe_unused *ebmalloc(size_t size)
 {
@@ -809,6 +820,7 @@ static void __init efi_tables(void)
 static EFI_GUID __initdata mps_guid = MPS_TABLE_GUID;
 static EFI_GUID __initdata smbios_guid = SMBIOS_TABLE_GUID;
 static EFI_GUID __initdata smbios3_guid = SMBIOS3_TABLE_GUID;
+static EFI_GUID __initdata properties_guid = EFI_PROPERTIES_TABLE_GUID;
 
 if ( match_guid(&acpi2_guid, &efi_ct[i].VendorGuid) )
   efi.acpi20 = (long)efi_ct[i].VendorTable;
@@ -820,6 +832,14 @@ static void __init efi_tables(void)
   efi.smbios = (long)efi_ct[i].VendorTable;
 if ( match_guid(&smbios3_guid, &efi_ct[i].VendorGuid) )
   efi.smbios3 = (long)efi_ct[i].VendorTable;
+if ( match_guid(&properties_guid, &efi_ct[i].VendorGuid) )
+{
+struct efi_properties_table *properties;
+
+efi_properties_tbl_addr = (long)efi_ct[i].VendorTable;
+properties = (struct efi_properties_table 
*)efi_properties_tbl_addr;
+efi_properties_tbl_size = properties->length;
+}
 }
 
 #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index 6b9c56e..e509111 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -39,3 +40,6 @@ extern UINT64 efi_boot_max_var_store_size, 
efi_boot_remain_var_store_size,
 
 extern UINT64 efi_apple_properties_addr;
 extern UINTN efi_apple_properties_len;
+
+extern u64 __initdata efi_properties_tbl_addr;
+extern u32 __initdata efi_properties_tbl_size;
-- 
2.7.4


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

Re: [Xen-devel] [PATCH v3 9/9] mm: Make sure pages are scrubbed

2017-05-08 Thread Boris Ostrovsky


>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -694,6 +694,31 @@ static void page_list_add_scrub(struct page_info *pg, 
>>> unsigned int node,
>>>  page_list_add(pg, &heap(node, zone, order));
>>>  }
>>>  
>>> +#define SCRUB_BYTE_PATTERN 0xc2c2c2c2c2c2c2c2
>> This likely needs a ULL suffix at least for the ARM32 build.
> See 3ac7ca1
>
> So 0xe7f000f0 for ARM32 and AARCH64_BREAK_FAULT for ARM64.


That's slightly different though --- unlike free_init_memory() this
pattern is only used when !NDEBUG and it is byte-sized (passed into
memset()).


-boris

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

Re: [Xen-devel] Security support scope (apropos of Xen and CNA)

> On 8 May 2017, at 17:11, Andrew Cooper  wrote:
> 
> On 08/05/17 17:04, Ian Jackson wrote:
>> Tim Deegan writes ("Re: Security support scope (apropos of Xen and CNA)"):
>>> Ah, so it is.  So there is information on the wiki page that's not in
>>> MAINTAINERS.  Could that be moved into MAINTAINERS?  There are
>>> a few things that don't map well to maintainership of specific
>>> files, e.g. "vMCE" or nested virtualization.  But on the whole I
>>> think that adding clauses for them would be OK.
>> I think this is quite awkward, really.  MAINTAINERS is about files,
>> and implementations.  The security support status is about parts of
>> interfaces, which don't map at all well.
>> 
>> We could add no-files stanzas, but how would you tell what they
>> referred to ?
> 
> This is the principle behind introducing docs/features/* which, as part
> of the required metadata, contains a support statement.
> 
> This way, there is an authoritative statement of support on a
> per-feature basis which is easy to keep up to date.
> 
> Lars: Any update on your project level clarifications of support statuses?

No, there is no vendor agreement yet on the testing side (aka what testing they 
commit to and what would happen if 3rd party testing outside of OSSTEST does 
not take place). That and what to do with "security supported" components which 
have poor testing for historical reasons are the only outstanding issue. These 
are really big issues, where there is no real consensus. I will get a Design 
Session in place for this at the summit: maybe we can make some progress.

However, IMHO, we can start off agreeing a format: in nutshell what we need is 
a list that shows feature=status - although we may want to add a few more 
fields, aka feature=(support status=..., maintainer status=, tested=OSSTEST|XTF|3rd Party|..., ...). Someone needs to make a sensible 
proposal.

For example, we could just map "status" on what we have now: preview, 
experimental, supported (which includes security support) and extend/change the 
meaning of the status field as this gets resolved. I don't think the two things 
are strongly linked.

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

Re: [Xen-devel] [PATCH] x86/efi: Reserve EFI properties table

On 08/05/17 17:17, Ross Lagerwall wrote:
> Some EFI firmware implementations may place the EFI properties table in
> RAM marked as BootServicesData, which Xen does not consider as reserved.
> When dom0 tries to access the EFI properties table (which Linux >= 4.4
> does), it crashes with a page fault.

The pagefault is just a side effect of Linux blindly assuming that the
ioremap() request succeeded.

From Xen's point of view, Dom0 tries to map a page which doesn't belong
to dom_xen, resulting a permission failure.

>   Fix this by unconditionally
> marking the EFI properties table as reserved in the E820, much like is
> done with the dmi regions.
>
> Signed-off-by: Ross Lagerwall 

Reviewed-by: Andrew Cooper 

This is probably also 4.9 material.

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

[Xen-devel] [PATCH for-next] x86/tlb: Don't use locked operations in tlbflush_filter()