Re: [Xen-devel] [PATCH 11/13] x86/paravirt: Add paravirt alternatives infrastructure

[Josh Poimboeuf]

On Mon, Oct 16, 2017 at 02:18:48PM -0400, Boris Ostrovsky wrote:
> On 10/12/2017 03:53 PM, Boris Ostrovsky wrote:
> > On 10/12/2017 03:27 PM, Andrew Cooper wrote:
> >> On 12/10/17 20:11, Boris Ostrovsky wrote:
> >>> There is also another problem:
> >>>
> >>> [1.312425] general protection fault:  [#1] SMP
> >>> [1.312901] Modules linked in:
> >>> [1.313389] CPU: 0 PID: 1 Comm: init Not tainted 4.14.0-rc4+ #6
> >>> [1.313878] task: 88003e2c task.stack: c938c000
> >>> [1.314360] RIP: 1e030:entry_SYSCALL_64_fastpath+0x1/0xa5
> >>> [1.314854] RSP: e02b:c938ff50 EFLAGS: 00010046
> >>> [1.315336] RAX: 000c RBX: 55f550168040 RCX:
> >>> 7fcfc959f59a
> >>> [1.315827] RDX:  RSI:  RDI:
> >>> 
> >>> [1.316315] RBP: 000a R08: 037f R09:
> >>> 0064
> >>> [1.316805] R10: 1f89cbf5 R11: 88003e2c R12:
> >>> 7fcfc958ad60
> >>> [1.317300] R13:  R14: 55f550185954 R15:
> >>> 1000
> >>> [1.317801] FS:  () GS:88003f80()
> >>> knlGS:
> >>> [1.318267] CS:  e033 DS:  ES:  CR0: 80050033
> >>> [1.318750] CR2: 7fcfc97ab218 CR3: 3c88e000 CR4:
> >>> 00042660
> >>> [1.319235] Call Trace:
> >>> [1.319700] Code: 51 50 57 56 52 51 6a da 41 50 41 51 41 52 41 53 48
> >>> 83 ec 30 65 4c 8b 1c 25 c0 d2 00 00 41 f7 03 df 39 08 90 0f 85 a5 00 00
> >>> 00 50  15 9c 95 d0 ff 58 48 3d 4c 01 00 00 77 0f 4c 89 d1 ff 14 c5
> >>> [1.321161] RIP: entry_SYSCALL_64_fastpath+0x1/0xa5 RSP: 
> >>> c938ff50
> >>> [1.344255] ---[ end trace d7cb8cd6cd7c294c ]---
> >>> [1.345009] Kernel panic - not syncing: Attempted to kill init!
> >>> exitcode=0x000b
> >>>
> >>>
> >>> All code
> >>> 
> >>>0:51   push   %rcx
> >>>1:50   push   %rax
> >>>2:57   push   %rdi
> >>>3:56   push   %rsi
> >>>4:52   push   %rdx
> >>>5:51   push   %rcx
> >>>6:6a dapushq  $0xffda
> >>>8:41 50push   %r8
> >>>a:41 51push   %r9
> >>>c:41 52push   %r10
> >>>e:41 53push   %r11
> >>>   10:48 83 ec 30  sub$0x30,%rsp
> >>>   14:65 4c 8b 1c 25 c0 d2 mov%gs:0xd2c0,%r11
> >>>   1b:00 00
> >>>   1d:41 f7 03 df 39 08 90 testl  $0x900839df,(%r11)
> >>>   24:0f 85 a5 00 00 00jne0xcf
> >>>   2a:50   push   %rax
> >>>   2b:*ff 15 9c 95 d0 ffcallq  *-0x2f6a64(%rip)#
> >>> 0xffd095cd<-- trapping instruction
> >>>   31:58   pop%rax
> >>>   32:48 3d 4c 01 00 00cmp$0x14c,%rax
> >>>   38:77 0fja 0x49
> >>>   3a:4c 89 d1 mov%r10,%rcx
> >>>   3d:ff   .byte 0xff
> >>>   3e:14 c5adc$0xc5,%al
> >>>
> >>>
> >>> so the original 'cli' was replaced with the pv call but to me the offset
> >>> looks a bit off, no? Shouldn't it always be positive?
> >> callq takes a 32bit signed displacement, so jumping back by up to 2G is
> >> perfectly legitimate.
> > Yes, but
> >
> > ostr@workbase> nm vmlinux | grep entry_SYSCALL_64_fastpath
> > 817365dd t entry_SYSCALL_64_fastpath
> > ostr@workbase> nm vmlinux | grep " pv_irq_ops"
> > 81c2dbc0 D pv_irq_ops
> > ostr@workbase>
> >
> > so pv_irq_ops.irq_disable is about 5MB ahead of where we are now. (I
> > didn't mean that x86 instruction set doesn't allow negative
> > displacement, I was trying to say that pv_irq_ops always live further down)
> 
> I believe the problem is this:
> 
> #define PV_INDIRECT(addr)   *addr(%rip)
> 
> The displacement that the linker computes will be relative to the where
> this instruction is placed at the time of linking, which is in
> .pv_altinstructions (and not .text). So when we copy it into .text the
> displacement becomes bogus.

apply_alternatives() is supposed to adjust that displacement based on
the new IP, though it could be messing that up somehow.  (See patch
10/13.)

-- 
Josh

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

[Xen-devel] [xen-4.8-testing baseline-only test] 72248: regressions - trouble: blocked/broken/fail/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 72248 xen-4.8-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/72248/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 13 xen-install/l1 fail REGR. vs. 7
 test-amd64-amd64-xl-qemut-win10-i386 16 guest-localmigrate/x10 fail REGR. vs. 
7

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   2 hosts-allocate   broken never pass
 build-arm64-pvops 2 hosts-allocate   broken never pass
 build-arm64-xsm   2 hosts-allocate   broken never pass
 build-arm64   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 build-arm64-xsm   3 capture-logs broken never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail like 7
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail like 7
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 7
 build-amd64-prev  7 xen-build/dist-test  fail   never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-pvh-intel 12 guest-start  fail  never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 build-i386-prev   7 xen-build/dist-test  fail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win10-i386 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 17 guest-stop  fail never pass

version targeted for testing:
 xen  bdc2ae68e2ecba1c3f55ad953189fe33362d1c51
baseline version:
 xen  667f70e658c4c382672056ebaf1505b4c5cdb0aa

Last test of basis7  

Re: [Xen-devel] [PATCH v9 08/16] x86: implement set value flow for MBA

[Yi Sun]

I forgot to add 'Reviewed-by' provided by Jan. So please ignore this but
refer v9.1. Thanks!

On 17-10-17 09:04:18, Yi Sun wrote:
> This patch implements set value flow for MBA including its callback
> function and domctl interface.
> 
> Signed-off-by: Yi Sun 

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

[Xen-devel] [PATCH v9.1 08/16] x86: implement set value flow for MBA

[Yi Sun]

This patch implements set value flow for MBA including its callback
function and domctl interface.

Signed-off-by: Yi Sun 
Reviewed-by: Jan Beulich 
---
CC: Jan Beulich 
CC: Andrew Cooper 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: Chao Peng 

v9:
- adjust codes in 'mba_sanitize_thrtl'.
  (suggested by Jan Beulich)
v8:
- restore some old codes in 'cat_check_cbm'.
  (suggested by Jan Beulich)
- use 'fls()' but not 'flsl()'.
  (suggested by Jan Beulich)
- use plain '=' to assign value for thrtl in 'mba_sanitize_thrtl'.
  (suggested by Jan Beulich)
v7:
- change name of 'check_val' to 'sanitize'.
  (suggested by Jan Beulich)
- fix comments.
  (suggested by Jan Beulich)
- add parentheses and change '== 0' to '!'.
  (suggested by Jan Beulich)
- remove unnecessary check of 'mba.thrtl_max'.
  (suggested by Jan Beulich)
- remove unnecessary intermediate variable 'mod'.
  (suggested by Jan Beulich)
- refine an assignement sentence to use '&='.
  (suggested by Jan Beulich)
- change type of last parameter of 'sanitize' to 'uint32_t' and
  apply same change to 'cat_check_cbm'.
  (suggested by Jan Beulich)
v6:
- split co-exist features' values setting flow to a new patch.
  (suggested by Jan Beulich)
- restore codes related to 'mba_check_thrtl' and 'check_value'.
  (suggested by Jan Beulich)
v5:
- adjust position of 'cat_check_cbm' to not to make changes so big.
  (suggested by Roger Pau Monné)
- remove 'props' from 'struct cos_write_info'.
  (suggested by Roger Pau Monné)
- make a single return statement in 'mba_check_thrtl'.
  (suggested by Jan Beulich)
v4:
- remove 'ALLOC_' from macro names.
  (suggested by Roger Pau Monné)
- join two checks into a single if.
  (suggested by Roger Pau Monné)
- remove redundant local variable 'array_len'.
  (suggested by Roger Pau Monné)
v3:
- modify commit message to make it clear.
  (suggested by Roger Pau Monné)
- modify functionality of 'check_val' to make it simple to only check value.
  Change the last parameter type from 'unsigned long *' to 'unsigned long'.
  (suggested by Roger Pau Monné)
- call rdmsrl to get value just written into MSR for MBA. Because HW can
  automatically change input value to what it wants.
  (suggested by Roger Pau Monné)
- change type of 'write_msr' to 'uint32_t' to return the value actually
  written into MSR. Then, change 'do_write_psr_msrs' to set the returned
  value into 'cos_reg_val[]'
- move the declaration of 'j' into loop in 'do_write_psr_msrs'.
  (suggested by Roger Pau Monné)
- change 'mba_info' to 'mba'.
  (suggested by Roger Pau Monné)
- change 'cat_info' to 'cat'.
  (suggested by Roger Pau Monné)
- rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
  from name.
  (suggested by Roger Pau Monné)
- change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
  (suggested by Roger Pau Monné)
v2:
- remove linear mode 'thrtl_max' check in 'mba_check_thrtl' because it has
  been checked in 'mba_init_feature'.
  (suggested by Chao Peng)
- for non-linear mode, check if '*thrtl' is not 0 in 'mba_check_thrtl'. If
  it is 0, we do not need to change it.
  (suggested by Chao Peng)
- move comments to explain changes of 'cos_write_info' from psr.c to commit
  message.
  (suggested by Chao Peng)
---
 xen/arch/x86/domctl.c   |  6 ++
 xen/arch/x86/psr.c  | 46 +
 xen/include/public/domctl.h |  1 +
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index b726eae..4bca15d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1465,6 +1465,12 @@ long arch_do_domctl(
   PSR_TYPE_L2_CBM);
 break;
 
+case XEN_DOMCTL_PSR_SET_MBA_THRTL:
+ret = psr_set_val(d, domctl->u.psr_alloc.target,
+  domctl->u.psr_alloc.data,
+  PSR_TYPE_MBA_THRTL);
+break;
+
 #define domctl_psr_get_val(d, domctl, type, copyback) ({\
 uint32_t v_;\
 int r_ = psr_get_val((d), (domctl)->u.psr_alloc.target, \
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 549f21b..e8f6dd8 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -138,6 +138,12 @@ static const struct feat_props {
 
 /* write_msr is used to write out feature MSR register. */
 void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
+
+/*
+ * sanitize is used to check if input val fulfills SDM requirement.
+ * And change it to valid 

[Xen-devel] [PATCH v9 08/16] x86: implement set value flow for MBA

[Yi Sun]

This patch implements set value flow for MBA including its callback
function and domctl interface.

Signed-off-by: Yi Sun 
---
CC: Jan Beulich 
CC: Andrew Cooper 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: Chao Peng 

v9:
- adjust codes in 'mba_sanitize_thrtl'.
  (suggested by Jan Beulich)
v8:
- restore some old codes in 'cat_check_cbm'.
  (suggested by Jan Beulich)
- use 'fls()' but not 'flsl()'.
  (suggested by Jan Beulich)
- use plain '=' to assign value for thrtl in 'mba_sanitize_thrtl'.
  (suggested by Jan Beulich)
v7:
- change name of 'check_val' to 'sanitize'.
  (suggested by Jan Beulich)
- fix comments.
  (suggested by Jan Beulich)
- add parentheses and change '== 0' to '!'.
  (suggested by Jan Beulich)
- remove unnecessary check of 'mba.thrtl_max'.
  (suggested by Jan Beulich)
- remove unnecessary intermediate variable 'mod'.
  (suggested by Jan Beulich)
- refine an assignement sentence to use '&='.
  (suggested by Jan Beulich)
- change type of last parameter of 'sanitize' to 'uint32_t' and
  apply same change to 'cat_check_cbm'.
  (suggested by Jan Beulich)
v6:
- split co-exist features' values setting flow to a new patch.
  (suggested by Jan Beulich)
- restore codes related to 'mba_check_thrtl' and 'check_value'.
  (suggested by Jan Beulich)
v5:
- adjust position of 'cat_check_cbm' to not to make changes so big.
  (suggested by Roger Pau Monné)
- remove 'props' from 'struct cos_write_info'.
  (suggested by Roger Pau Monné)
- make a single return statement in 'mba_check_thrtl'.
  (suggested by Jan Beulich)
v4:
- remove 'ALLOC_' from macro names.
  (suggested by Roger Pau Monné)
- join two checks into a single if.
  (suggested by Roger Pau Monné)
- remove redundant local variable 'array_len'.
  (suggested by Roger Pau Monné)
v3:
- modify commit message to make it clear.
  (suggested by Roger Pau Monné)
- modify functionality of 'check_val' to make it simple to only check value.
  Change the last parameter type from 'unsigned long *' to 'unsigned long'.
  (suggested by Roger Pau Monné)
- call rdmsrl to get value just written into MSR for MBA. Because HW can
  automatically change input value to what it wants.
  (suggested by Roger Pau Monné)
- change type of 'write_msr' to 'uint32_t' to return the value actually
  written into MSR. Then, change 'do_write_psr_msrs' to set the returned
  value into 'cos_reg_val[]'
- move the declaration of 'j' into loop in 'do_write_psr_msrs'.
  (suggested by Roger Pau Monné)
- change 'mba_info' to 'mba'.
  (suggested by Roger Pau Monné)
- change 'cat_info' to 'cat'.
  (suggested by Roger Pau Monné)
- rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
  from name.
  (suggested by Roger Pau Monné)
- change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
  (suggested by Roger Pau Monné)
v2:
- remove linear mode 'thrtl_max' check in 'mba_check_thrtl' because it has
  been checked in 'mba_init_feature'.
  (suggested by Chao Peng)
- for non-linear mode, check if '*thrtl' is not 0 in 'mba_check_thrtl'. If
  it is 0, we do not need to change it.
  (suggested by Chao Peng)
- move comments to explain changes of 'cos_write_info' from psr.c to commit
  message.
  (suggested by Chao Peng)
---
 xen/arch/x86/domctl.c   |  6 ++
 xen/arch/x86/psr.c  | 46 +
 xen/include/public/domctl.h |  1 +
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index b726eae..4bca15d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1465,6 +1465,12 @@ long arch_do_domctl(
   PSR_TYPE_L2_CBM);
 break;
 
+case XEN_DOMCTL_PSR_SET_MBA_THRTL:
+ret = psr_set_val(d, domctl->u.psr_alloc.target,
+  domctl->u.psr_alloc.data,
+  PSR_TYPE_MBA_THRTL);
+break;
+
 #define domctl_psr_get_val(d, domctl, type, copyback) ({\
 uint32_t v_;\
 int r_ = psr_get_val((d), (domctl)->u.psr_alloc.target, \
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 549f21b..e8f6dd8 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -138,6 +138,12 @@ static const struct feat_props {
 
 /* write_msr is used to write out feature MSR register. */
 void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
+
+/*
+ * sanitize is used to check if input val fulfills SDM requirement.
+ * And change it to valid value if SDM allows.
+ */
+bool 

Re: [Xen-devel] [PATCH v8 08/16] x86: implement set value flow for MBA

[Yi Sun]

On 17-10-16 06:49:49, Jan Beulich wrote:
> >>> On 16.10.17 at 05:04,  wrote:
> > This patch implements set value flow for MBA including its callback
> > function and domctl interface.
> > 
> > Signed-off-by: Yi Sun 
> 
> Reviewed-by: Jan Beulich 
> 
> > v8:
> > - restore some unnecessary changes in 'cat_check_cbm'.
> >   (suggested by Jan Beulich)
> 
> This reads as if you did exactly the opposite thing of what you
> actually did.
> 
> > +static bool mba_sanitize_thrtl(const struct feat_node *feat, uint32_t 
> > *thrtl)
> > +{
> > +if ( *thrtl > feat->mba.thrtl_max )
> > +return false;
> 
> Wouldn't it be better to do this check after ...
> 
> > +/*
> > + * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> > + * 1. Linear mode: In the linear mode the input precision is defined
> > + *as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> > + *input precision is 10%. Values not an even multiple of the
> > + *precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> > + *applied).
> > + * 2. Non-linear mode: Input delay values are powers-of-two from zero
> > + *to the MBA_MAX value from CPUID. In this case any values not a
> > + *power of two will be rounded down the next nearest power of two.
> > + */
> > +if ( feat->mba.linear )
> > +*thrtl -= *thrtl % (100 - feat->mba.thrtl_max);
> > +else
> > +{
> > +/* Not power of 2. */
> > +if ( *thrtl & (*thrtl - 1) )
> > +*thrtl = 1 << (fls(*thrtl) - 1);
> > +}
> 
> ... these adjustments? That way someone specifying e.g. a linear
> value of 95% would get 90% just like for 85% (s)he would get
> 80%.
> 
> > +return true;
> 
> If so, the return statement could simply become
> 
> return *thrtl <= feat->mba.thrtl_max;
> 
> My R-b also applies if you decide to make this change.
> 
Thank you for the suggestion! It is not worthy to send whole patch set out.
I will just update this patch.

> Jan

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

[Xen-devel] [PATCH 54/58] net/xen-netback: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Wei Liu 
Cc: Paul Durrant 
Cc: xen-de...@lists.xenproject.org
Cc: net...@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/net/xen-netback/common.h| 2 +-
 drivers/net/xen-netback/interface.c | 2 +-
 drivers/net/xen-netback/netback.c   | 6 ++
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 5b1d2e8402d9..a46a1e94505d 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -307,7 +307,7 @@ static inline struct xenbus_device 
*xenvif_to_xenbus_device(struct xenvif *vif)
return to_xenbus_device(vif->dev->dev.parent);
 }
 
-void xenvif_tx_credit_callback(unsigned long data);
+void xenvif_tx_credit_callback(struct timer_list *t);
 
 struct xenvif *xenvif_alloc(struct device *parent,
domid_t domid,
diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index dcfcb153918c..5cbe0ae55a07 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -520,7 +520,7 @@ int xenvif_init_queue(struct xenvif_queue *queue)
 
queue->credit_bytes = queue->remaining_credit = ~0UL;
queue->credit_usec  = 0UL;
-   setup_timer(>credit_timeout, xenvif_tx_credit_callback, 0UL);
+   timer_setup(>credit_timeout, xenvif_tx_credit_callback, 0);
queue->credit_window_start = get_jiffies_64();
 
queue->rx_queue_max = XENVIF_RX_QUEUE_BYTES;
diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 5042ff8d449a..a27daa23c9dc 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -183,9 +183,9 @@ static void tx_add_credit(struct xenvif_queue *queue)
queue->rate_limited = false;
 }
 
-void xenvif_tx_credit_callback(unsigned long data)
+void xenvif_tx_credit_callback(struct timer_list *t)
 {
-   struct xenvif_queue *queue = (struct xenvif_queue *)data;
+   struct xenvif_queue *queue = from_timer(queue, t, credit_timeout);
tx_add_credit(queue);
xenvif_napi_schedule_or_enable_events(queue);
 }
@@ -700,8 +700,6 @@ static bool tx_credit_exceeded(struct xenvif_queue *queue, 
unsigned size)
 
/* Still too big to send right now? Set a callback. */
if (size > queue->remaining_credit) {
-   queue->credit_timeout.data =
-   (unsigned long)queue;
mod_timer(>credit_timeout,
  next_credit);
queue->credit_window_start = next_credit;
-- 
2.7.4


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

[Xen-devel] [xen-4.9-testing test] 114543: tolerable FAIL - PUSHED

[osstest service owner]

flight 114543 xen-4.9-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/114543/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 14 guest-localmigrate fail in 114533 pass 
in 114543
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail pass in 114533

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-win7-amd64 18 guest-start/win.repeat fail blocked in 
114118
 test-armhf-armhf-xl-rtds 17 guest-start.2   fail blocked in 114118
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 114533 
like 114091
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 114533 like 
114091
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail in 114533 like 
114118
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-install fail in 114533 like 
114118
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore fail in 114533 like 
114118
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore fail in 114533 like 
114118
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114091
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 114118
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 114118
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  de38e28cc2cc62e6e9e4741403e4a8f6c07d8cfd
baseline version:
 xen  9cde7a833db53c9c3a88b767af8c7cb07053a6fd

Last test of basis   114118  2017-10-08 03:30:54 Z8 days
Failing since114312  2017-10-11 00:44:07 Z5 days6 attempts
Testing same since   114448  2017-10-13 03:26:47 Z3 days4 attempts


People who touched revisions under test:
  Andrew Cooper 

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumprun   8 xen-buildfail REGR. vs. 114204
 build-i386-rumprun8 xen-buildfail REGR. vs. 114204

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2  16 guest-start/debian.repeat  fail pass in 114525

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail in 114525 like 
114204
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-install fail in 114525 like 
114204
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore fail in 114525 like 
114204
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore fail in 114525 like 
114204
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 114204
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 114204
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114204
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114204
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114204
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 114204
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass

version targeted for testing:
 xen  46aaf41ee099a048d7a554c03ae01bcdaa07f776
baseline version:
 xen  572a78190403e5f2acbd01fa72c35fafe9700169

Last test of basis   114204  2017-10-09 19:19:08 Z7 days
Failing since114288  2017-10-10 17:02:59 Z6 days6 attempts
Testing same since   114498  2017-10-14 08:22:08 Z2 days3 attempts


People who touched revisions under test:
  Alexandru Isaila 
  Andre Przywara 
  Andrew Cooper 
  Boris Ostrovsky 

Re: [Xen-devel] [PATCH] xen-netfront, xen-netback: Use correct minimum MTU values

[David Miller]

From: Boris Ostrovsky 
Date: Mon, 16 Oct 2017 16:19:07 -0400

> On 10/16/2017 11:05 AM, Wei Liu wrote:
>> On Mon, Oct 16, 2017 at 03:20:32PM +0200, Mohammed Gamal wrote:
>>> RFC791 specifies the minimum MTU to be 68, while xen-net{front|back}
>>> drivers use a minimum value of 0.
>>>
>>> When set MTU to 0~67 with xen_net{front|back} driver, the network
>>> will become unreachable immediately, the guest can no longer be pinged.
>>>
>>> xen_net{front|back} should not allow the user to set this value which causes
>>> network problems.
>>>
>>> Reported-by: Chen Shi 
>>> Signed-off-by: Mohammed Gamal 
>> Acked-by: Wei Liu 
>>
>> CC netfront maintainers
> 
> 
> Reviewed-by: Boris Ostrovsky 
> 
> I can take it via Xen tree unless there are objections.

No problem on my end.

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

Re: [Xen-devel] [PATCH] xen-netfront, xen-netback: Use correct minimum MTU values

[Boris Ostrovsky]

On 10/16/2017 11:05 AM, Wei Liu wrote:
> On Mon, Oct 16, 2017 at 03:20:32PM +0200, Mohammed Gamal wrote:
>> RFC791 specifies the minimum MTU to be 68, while xen-net{front|back}
>> drivers use a minimum value of 0.
>>
>> When set MTU to 0~67 with xen_net{front|back} driver, the network
>> will become unreachable immediately, the guest can no longer be pinged.
>>
>> xen_net{front|back} should not allow the user to set this value which causes
>> network problems.
>>
>> Reported-by: Chen Shi 
>> Signed-off-by: Mohammed Gamal 
> Acked-by: Wei Liu 
>
> CC netfront maintainers


Reviewed-by: Boris Ostrovsky 

I can take it via Xen tree unless there are objections.

-boris

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

Re: [Xen-devel] [PATCH 11/13] x86/paravirt: Add paravirt alternatives infrastructure

[Boris Ostrovsky]

On 10/12/2017 03:53 PM, Boris Ostrovsky wrote:
> On 10/12/2017 03:27 PM, Andrew Cooper wrote:
>> On 12/10/17 20:11, Boris Ostrovsky wrote:
>>> There is also another problem:
>>>
>>> [1.312425] general protection fault:  [#1] SMP
>>> [1.312901] Modules linked in:
>>> [1.313389] CPU: 0 PID: 1 Comm: init Not tainted 4.14.0-rc4+ #6
>>> [1.313878] task: 88003e2c task.stack: c938c000
>>> [1.314360] RIP: 1e030:entry_SYSCALL_64_fastpath+0x1/0xa5
>>> [1.314854] RSP: e02b:c938ff50 EFLAGS: 00010046
>>> [1.315336] RAX: 000c RBX: 55f550168040 RCX:
>>> 7fcfc959f59a
>>> [1.315827] RDX:  RSI:  RDI:
>>> 
>>> [1.316315] RBP: 000a R08: 037f R09:
>>> 0064
>>> [1.316805] R10: 1f89cbf5 R11: 88003e2c R12:
>>> 7fcfc958ad60
>>> [1.317300] R13:  R14: 55f550185954 R15:
>>> 1000
>>> [1.317801] FS:  () GS:88003f80()
>>> knlGS:
>>> [1.318267] CS:  e033 DS:  ES:  CR0: 80050033
>>> [1.318750] CR2: 7fcfc97ab218 CR3: 3c88e000 CR4:
>>> 00042660
>>> [1.319235] Call Trace:
>>> [1.319700] Code: 51 50 57 56 52 51 6a da 41 50 41 51 41 52 41 53 48
>>> 83 ec 30 65 4c 8b 1c 25 c0 d2 00 00 41 f7 03 df 39 08 90 0f 85 a5 00 00
>>> 00 50  15 9c 95 d0 ff 58 48 3d 4c 01 00 00 77 0f 4c 89 d1 ff 14 c5
>>> [1.321161] RIP: entry_SYSCALL_64_fastpath+0x1/0xa5 RSP: c938ff50
>>> [1.344255] ---[ end trace d7cb8cd6cd7c294c ]---
>>> [1.345009] Kernel panic - not syncing: Attempted to kill init!
>>> exitcode=0x000b
>>>
>>>
>>> All code
>>> 
>>>0:51   push   %rcx
>>>1:50   push   %rax
>>>2:57   push   %rdi
>>>3:56   push   %rsi
>>>4:52   push   %rdx
>>>5:51   push   %rcx
>>>6:6a dapushq  $0xffda
>>>8:41 50push   %r8
>>>a:41 51push   %r9
>>>c:41 52push   %r10
>>>e:41 53push   %r11
>>>   10:48 83 ec 30  sub$0x30,%rsp
>>>   14:65 4c 8b 1c 25 c0 d2 mov%gs:0xd2c0,%r11
>>>   1b:00 00
>>>   1d:41 f7 03 df 39 08 90 testl  $0x900839df,(%r11)
>>>   24:0f 85 a5 00 00 00jne0xcf
>>>   2a:50   push   %rax
>>>   2b:*ff 15 9c 95 d0 ffcallq  *-0x2f6a64(%rip)#
>>> 0xffd095cd<-- trapping instruction
>>>   31:58   pop%rax
>>>   32:48 3d 4c 01 00 00cmp$0x14c,%rax
>>>   38:77 0fja 0x49
>>>   3a:4c 89 d1 mov%r10,%rcx
>>>   3d:ff   .byte 0xff
>>>   3e:14 c5adc$0xc5,%al
>>>
>>>
>>> so the original 'cli' was replaced with the pv call but to me the offset
>>> looks a bit off, no? Shouldn't it always be positive?
>> callq takes a 32bit signed displacement, so jumping back by up to 2G is
>> perfectly legitimate.
> Yes, but
>
> ostr@workbase> nm vmlinux | grep entry_SYSCALL_64_fastpath
> 817365dd t entry_SYSCALL_64_fastpath
> ostr@workbase> nm vmlinux | grep " pv_irq_ops"
> 81c2dbc0 D pv_irq_ops
> ostr@workbase>
>
> so pv_irq_ops.irq_disable is about 5MB ahead of where we are now. (I
> didn't mean that x86 instruction set doesn't allow negative
> displacement, I was trying to say that pv_irq_ops always live further down)

I believe the problem is this:

#define PV_INDIRECT(addr)   *addr(%rip)

The displacement that the linker computes will be relative to the where
this instruction is placed at the time of linking, which is in
.pv_altinstructions (and not .text). So when we copy it into .text the
displacement becomes bogus.

Replacing the macro with

#define PV_INDIRECT(addr)   *addr  // well, it's not so much
indirect anymore

makes things work. Or maybe it can be adjusted top be kept truly indirect.

-boris



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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Stefano Stabellini]

On Mon, 16 Oct 2017, Andrew Cooper wrote:
>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>all state is actually set up.  As it currently stands, d0v0 is eligible for
>scheduling before its registers have been set.  This is latent as we also
>hold a systemcontroller pause reference at the time which prevents d0 from
>being scheduled.
> 
>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>being able to call VCPUOP_initialise and modify state under the feet of the
>running vcpu.  This is latent as PVH dom0 construction don't yet function.
> 
> Signed-off-by: Andrew Cooper 

ARM bits:

Reviewed-by: Stefano Stabellini 


> ---
> CC: Jan Beulich 
> CC: Stefano Stabellini 
> CC: Julien Grall 
> CC: Wei Liu 
> CC: Roger Pau Monné 
> ---
>  xen/arch/arm/domain_build.c   |  6 +++---
>  xen/arch/x86/dom0_build.c | 13 +++--
>  xen/arch/x86/hvm/dom0_build.c |  1 +
>  xen/arch/x86/pv/dom0_build.c  |  6 +++---
>  4 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4636b17..bf29299 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2197,9 +2197,6 @@ int construct_dom0(struct domain *d)
>  
>  discard_initial_modules();
>  
> -v->is_initialised = 1;
> -clear_bit(_VPF_down, >pause_flags);
> -
>  memset(regs, 0, sizeof(*regs));
>  
>  regs->pc = (register_t)kinfo.entry;
> @@ -2247,6 +2244,9 @@ int construct_dom0(struct domain *d)
>  vcpu_switch_to_aarch64_mode(d->vcpu[i]);
>  }
>  
> +v->is_initialised = 1;
> +clear_bit(_VPF_down, >pause_flags);
> +
>  return 0;
>  }
>  
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index e4bffd5..bf992fe 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -466,6 +466,8 @@ int __init construct_dom0(struct domain *d, const 
> module_t *image,
>void *(*bootstrap_map)(const module_t *),
>char *cmdline)
>  {
> +int rc;
> +
>  /* Sanity! */
>  BUG_ON(d->domain_id != 0);
>  BUG_ON(d->vcpu[0] == NULL);
> @@ -481,8 +483,15 @@ int __init construct_dom0(struct domain *d, const 
> module_t *image,
>  }
>  #endif
>  
> -return (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
> -   (d, image, image_headroom, initrd,bootstrap_map, cmdline);
> +rc = (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
> + (d, image, image_headroom, initrd, bootstrap_map, cmdline);
> +if ( rc )
> +return rc;
> +
> +/* Sanity! */
> +BUG_ON(!d->vcpu[0]->is_initialised);
> +
> +return 0;
>  }
>  
>  /*
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index e8f746c..a67071c 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
> paddr_t entry,
>  
>  update_domain_wallclock_time(d);
>  
> +v->is_initialised = 1;
>  clear_bit(_VPF_down, >pause_flags);
>  
>  return 0;
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index dcbee43..8ad7e3d 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -847,9 +847,6 @@ int __init dom0_construct_pv(struct domain *d,
>  
>  update_domain_wallclock_time(d);
>  
> -v->is_initialised = 1;
> -clear_bit(_VPF_down, >pause_flags);
> -
>  /*
>   * Initial register values:
>   *  DS,ES,FS,GS = FLAT_KERNEL_DS
> @@ -883,6 +880,9 @@ int __init dom0_construct_pv(struct domain *d,
>  if ( d->domain_id == hardware_domid )
>  iommu_hwdom_init(d);
>  
> +v->is_initialised = 1;
> +clear_bit(_VPF_down, >pause_flags);
> +
>  return 0;
>  
>  out:
> -- 
> 2.1.4
> ___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v7] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Petre Pircalabu]

From: Razvan Cojocaru 

For the default EPT view we have xc_set_mem_access_multi(), which
is able to set an array of pages to an array of access rights with
a single hypercall. However, this functionality was lacking for the
altp2m subsystem, which could only set page restrictions for one
page at a time. This patch addresses the gap.

HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a
DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart (and
hence with the original altp2m design, where domains are allowed - with the
proper altp2m access rights - to alter these settings), in the absence of an
official position on the issue from the original altp2m designers.

Signed-off-by: Razvan Cojocaru 
Signed-off-by: Petre Pircalabu 

---

Changed since v2:
* Added support for compat arguments translation

Changed since v3:
* Replaced  __copy_to_guest with __copy_field_to_guest
* Removed the un-needed parentheses.
* Fixed xlat.lst ordering
* Added comment to patch description explaining why the
functionality was added as an HVMOP.
* Guard using XEN_GENERATING_COMPAT_HEADERS the hvmmem_type_t definition.
This will prevent suplicate definitions to be generated for the
compat equivalent.
* Added comment describing the manual translation of
xen_hvm_altp2m_op_t generic fields from compat_hvm_altp2m_op_t.

Changed since v4:
* Changed the mask parameter to 0x3F.
* Split long lines.
* Added "improperly named HVMMEM_(*)" to the comment explaining the
XEN_GENERATING_COMPAT_HEADERS guard.
* Removed typedef and XEN_GUEST_HANDLE for 
xen_hvm_altp2m_set_mem_access_multi.
* Copied the "opaque" field to guest in compat_altp2m_op.
* Added build time test to check if the size of
xen_hvm_altp2m_set_mem_access_multi at least equal to the size of
compat_hvm_altp2m_set_mem_access_multi.

Changed since v5:
* Changed the domid parameter type to uint32_t to match 5b42c82f.
* Added comment to explain why the 0x3F value was chosen.
* Fixed switch indentation in compat_altp2m_op.
* Changed the condition used to check if the opaque field has to
be copied to the guest.
* Added CHECK_hvm_altp2m_op and CHECK_hvm_altp2m_set_mem_access_multi.

Changed since v6:
* Removed trailing semicolon from the definitions of CHECK_hvm_altp2m_op
and CHECK_hvm_altp2m_set_mem_access_multi.
* Removed BUILD_BUG_ON check.
* Added comment describing the reason for manually defining the CHECK_
macros.
* Added ASSERT_UNREACHABLE as the default switch label action in
compat_altp2m_op.
* Added ASSERT(rc == __HYPERVISOR_hvm_op) to make sure the return
code was actually sey by hypercall_create_continuation.
---
 tools/libxc/include/xenctrl.h   |   3 +
 tools/libxc/xc_altp2m.c |  40 
 xen/arch/x86/hvm/hvm.c  | 138 +++-
 xen/include/Makefile|   1 +
 xen/include/public/hvm/hvm_op.h |  36 +--
 xen/include/xlat.lst|   1 +
 6 files changed, 213 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 666db0b..f171668 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1974,6 +1974,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle, 
uint32_t domid,
 int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid,
  uint16_t view_id, xen_pfn_t old_gfn,
  xen_pfn_t new_gfn);
+int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid,
+   uint16_t view_id, uint8_t *access,
+   uint64_t *pages, uint32_t nr);
 
 /** 
  * Mem paging operations.
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index 07fcd18..e170fe3 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -213,3 +213,43 @@ int xc_altp2m_change_gfn(xc_interface *handle, uint32_t 
domid,
 return rc;
 }
 
+int xc_altp2m_set_mem_access_multi(xc_interface *xch, uint32_t domid,
+   uint16_t view_id, uint8_t *access,
+   uint64_t *pages, uint32_t nr)
+{
+int rc;
+
+DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
+DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t),
+ XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
+if ( arg == NULL )
+return -1;
+
+arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
+arg->cmd = HVMOP_altp2m_set_mem_access_multi;
+arg->domain = domid;
+arg->u.set_mem_access_multi.view = view_id;
+arg->u.set_mem_access_multi.nr = nr;
+
+if ( 

Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be nonnull in libxl__enum_from_string

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be 
nonnull in libxl__enum_from_string"):
> On 16/10/17 15:56, Ian Jackson wrote:
> > We are very soon going to want "NN2" and maybe "NN_1_2".
> 
> The hypervisor uses
> 
> #define __nonnull(...) __attribute__((__nonnull__(__VA_ARGS__)))
> 
> I suggest you use the same in libxl to avoid a combinatorial  explosion
> of NN variants.

We have NN already so in our code that should be NN(1) etc. I guess.

Ian.

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

Re: [Xen-devel] [PATCH for-next] xen/pv: Construct d0v0's GDT properly

[Andrew Cooper]

On 16/10/17 16:58, Jan Beulich wrote:
 On 16.10.17 at 16:38,  wrote:
>> c/s cf6d39f8199 "x86/PV: properly populate descriptor tables" changed the GDT
>> to reference zero_page for intermediate frames between the guest and Xen
>> frames.
>>
>> Because dom0_construct_pv() doesn't call arch_set_info_guest(), some bits of
>> initialisation are missed, including the pv_destroy_gdt() which initially
>> fills the references to zero_page.
>>
>> In practice, this means there is a window between starting and the first call
>> to HYPERCALL_set_gdt() were lar/lsl/verr/verw suffer non-architectural
>> behaviour.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 
>
>> This probably wants backporting to Xen 4.7 and later.
> Could you remind me once the patch has gone in (as that'll be only
> in quite a few weeks)?

I will stick this in my x86-next branch, and leave myself a note.  (I
guess we will see how well this scheme works.)

~Andrew

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

Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [PATCH] libxc: don't fail domain 
creation when unpacking initrd fails"):
> IMO, the toolstack should not be making assumptions about the initrd,
> and shouldn't be touching it.  It is the users responsibility to provide
> an initrd which its kernel can read.
> 
> Furthermore, leaving the decompression to the kernel reduces the dom0
> attack surface.

If we expect that only very old or very odd kernels can't do the
decompression themselves, then perhaps we could have an option to
enable initrd decompression and have it off by default.

Your point about the attack surface is well-made.

Ian.

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

Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be nonnull in libxl__enum_from_string

[Andrew Cooper]

On 16/10/17 15:56, Ian Jackson wrote:
> Wei Liu writes ("[PATCH for-4.10] libxl: annotate s to be nonnull in 
> libxl__enum_from_string"):
>> Hope this can placate coverity.
> Acked-by: Ian Jackson 
>
> We are very soon going to want "NN2" and maybe "NN_1_2".

The hypervisor uses

#define __nonnull(...) __attribute__((__nonnull__(__VA_ARGS__)))

I suggest you use the same in libxl to avoid a combinatorial  explosion
of NN variants.

~Andrew

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

Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH] libxc: don't fail domain creation when 
unpacking initrd fails"):
> On 16.10.17 at 17:45,  wrote:
> > Is there no way to tell that a kernel supports gzipped initrds by
> > looking at the kernel ?
> 
> Well, Linux kernels have config options controlling their ability. So
> even a modern kernel _could_ be configured to require unzipping.
> I didn't check whether they announce this anywhere outside the
> (possibly) embedded .config, but even if they did this would be
> only Linux then. A solution here shouldn't really be OS-specific imo.

I guess I was hoping for an ELF note or some multiboot protocol
element or something.  If it doesn't exist then your proposed general
approach is probably best.

I'm afraid I still find the patch less clear than it could be.
The new semantics of xc_dom_ramdisk_check_size are awkward.  And
looking at it briefly, I think it might be possible to try the unzip
even if the size is too large.

I think a sensible implementation is might have to have a flag
variable to control "try doing it raw".  And it might be bdest to
replace xc_dom_ramdisk_check_size with either a function which does
not bomb out if the limit is exceeded.

What you are really trying to do here is to pursue two strategies in
parallel.  And ideally they would not be entangled.  Maybe there would
have to be a comment.  Each of the strategies must rely only on
functions which don't bomb out, to achieve that.

Ian.

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

Re: [Xen-devel] [xen-4.8-testing test] 114505: regressions - FAIL

[Andrew Cooper]

On 16/10/17 16:16, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [xen-4.8-testing test] 114505: 
> regressions - FAIL"):
>> On 15/10/17 20:45, osstest service owner wrote:
>>> flight 114505 xen-4.8-testing real [real]
>>> http://logs.test-lab.xenproject.org/osstest/logs/114505/
>>>
>>> Regressions :-(
>>>
>>> Tests which did not succeed and are blocking,
>>> including tests which could not be run:
>>>  test-xtf-amd64-amd64-2 48 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
>>> 114173
> ...
>> Ian: These tests exercise something very machine specific, and the XTF
>> tests really do need tying to specific hardware when making regression
>> considerations.
> Is this test new enough that it might have never run on that
> hardware ?  If so then a force push might be justified.

andrewcoop@andrewcoop:/local/xen-test-framework.git$ git show --format=fuller 
36d926fe
commit 36d926fe0e9b7db39965f430cdb4c5f1daf4eef3
Author: Andrew Cooper 
AuthorDate: Wed Oct 12 18:23:42 2016
Commit: Andrew Cooper 
CommitDate: Tue Apr 25 13:55:42 2017

LBR/TSX VMentry failure test

Signed-off-by: Andrew Cooper 

It has been running in OSSTest for a fair while now.

The test will only fail on versions of Xen before the fixes went in
(Currently Xen 4.9), on Haswell and Broadwell hardware.

Its also possible

> It is difficult to tie the tests to specific hardware without
> insisting that every run uses every host.

How hard would it be to tag each flight with which host it ran on, and
filter for host == current when determining whether a regression has
occurred?

~Andrew

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

Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Andrew Cooper]

On 16/10/17 17:19, Jan Beulich wrote:
 On 16.10.17 at 17:45,  wrote:
>> Jan Beulich writes ("[PATCH] libxc: don't fail domain creation when 
>> unpacking 
>> initrd fails"):
>>> At least Linux kernels have been able to work with gzip-ed initrd for
>>> quite some time; initrd compressed with other methods aren't even being
>>> attempted to unpack. Furthermore the unzip-ing routine used here isn't
>>> capable of dealing with various forms of concatenated files, each of
>>> which was gzip-ed separately (it is this particular case which has been
>>> the source of observed VM creation failures).
>> I'm not sure I really like this approach of attempting to ungzip it
>> and then falling back.  (And the size-checking logic is not
>> particularly easy to follow.)
>>
>> Is there no way to tell that a kernel supports gzipped initrds by
>> looking at the kernel ?
> Well, Linux kernels have config options controlling their ability. So
> even a modern kernel _could_ be configured to require unzipping.
> I didn't check whether they announce this anywhere outside the
> (possibly) embedded .config, but even if they did this would be
> only Linux then. A solution here shouldn't really be OS-specific imo.
>
>>  A heuristic would probably do: it's OK if we
>> sometimes insist on decompression ourselves, for a subset of old
>> kernels where it's not needed.
> Well, I specifically wanted to avoid any guesswork. But if I
> simply had reported this as a problem that needs dealing with,
> things likely would have gone like for the Python version issue
> (which I still haven't got around to), asking me to look into
> addressing it. So I thought I'd present a possible solution right
> away. To be honest, if you want this to be done some
> meaningfully different way which I'm not convinced of, I'm not
> sure I'm the one to carry this out, yet I'd still request the
> issue to be addressed.

I've been bitten by this issue several times before, and a fix would be
nice.

IMO, the toolstack should not be making assumptions about the initrd,
and shouldn't be touching it.  It is the users responsibility to provide
an initrd which its kernel can read.

Furthermore, leaving the decompression to the kernel reduces the dom0
attack surface.

~Andrew

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  24fb44e971a62b345c7b6ca3c03b454a1e150abe
baseline version:
 xen  765c2035a765c426c130c1f2cc009af60a99b1bd

Last test of basis   114544  2017-10-16 12:01:50 Z0 days
Testing same since   114547  2017-10-16 15:03:08 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=24fb44e971a62b345c7b6ca3c03b454a1e150abe
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.
 PERLLIB=.:.
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
24fb44e971a62b345c7b6ca3c03b454a1e150abe
+ branch=xen-unstable-smoke
+ revision=24fb44e971a62b345c7b6ca3c03b454a1e150abe
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.:.
 PERLLIB=.:.:.
+++ 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
+++ export PERLLIB=.:.:.:.
+++ PERLLIB=.:.:.:.
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.9-testing
+ '[' x24fb44e971a62b345c7b6ca3c03b454a1e150abe = 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
++ : 

Re: [Xen-devel] [xen-4.8-testing test] 114505: regressions - FAIL

[Andrew Cooper]

On 16/10/17 17:12, Jan Beulich wrote:
 On 16.10.17 at 11:14,  wrote:
>> On 15/10/17 20:45, osstest service owner wrote:
>>> flight 114505 xen-4.8-testing real [real]
>>> http://logs.test-lab.xenproject.org/osstest/logs/114505/ 
>>>
>>> Regressions :-(
>>>
>>> Tests which did not succeed and are blocking,
>>> including tests which could not be run:
>>>  test-xtf-amd64-amd64-2 48 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
>> 114173
>>> Tests which are failing intermittently (not blocking):
>>>  test-xtf-amd64-amd64-5 48 xtf/test-hvm64-lbr-tsx-vmentry fail in 114454 
>> pass in 114505
>>
>> Ian: These tests exercise something very machine specific, and the XTF
>> tests really do need tying to specific hardware when making regression
>> considerations.
>>
>> Jan: This highlights that TSX/VMEntry failure fixes probably want
>> backporting to before Xen 4.9.  IIRC, the 6 patches needed are:
> So I'm mildly confused by this request:
>
>> e3eb84e33c36 (only as a functional prerequisite)
>> 9b93c6b3695b: x86/vmx: introduce vmx_find_msr()
>> 7f11aa4b2b1f: x86/vmx: optimize vmx_read/write_guest_msr()
>> d6e9f8d4f35d: x86/vmx: fix vmentry failure with TSX bits in LBR
>> f97838bbd980: x86: Move microcode loading earlier
> Up to here, everything is in 4.9 already afaict. Considering the
> context here is a 4.8 test report, did you perhaps mean to ask
> for this on 4.8 (and possibly also 4.7)?

Well - I did ask for "backporting to before Xen 4.9".

> If so, I'm not really sure -
> these changes taken together look a little large for the gain
> they provide.

We have had several xen-devel reports of this problem, starting against
Xen 4.6 iirc.  If you really thing its more risk than its worth then fine.

>
>> 20f1976b4419: x86/vmx: Fix vmentry failure because of invalid LER on 
>> Broadwell
> I'll see to pull this one in for 4.9.1.

Oops - I'd not spotted that that change was missing in Xen 4.9.  Yes -
please backport that one.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Wei Liu]

On Mon, Oct 16, 2017 at 04:07:32PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() 
> to not pass a structure by value"):
> > On Mon, Oct 16, 2017 at 02:51:54PM +0100, Andrew Cooper wrote:
> ...
> > > With Joshua's patch in place, the implementer of this callback is the
> > > code generated by libxl_save_msgs_gen.pl, which is the aformentioned
> > > extra process.  Passing by pointer or value has nothing to do with the
> > > fact that the automatically generated code needs to know how to
> > > serialise/deserialise the data to feed it back to the main process.
> > 
> > Right. I agree with you here after going back to the old thread.
> 
> ISTM that the callback being a struct rather than a pointer does make
> the code in libxl_save_msgs_gen.pl simpler, since it can simply memcpy
> the struct.

Yes, that's the case.

> 
> I certainly dislike your 1/2 patch with the current commit message.
> 
> Andrew Cooper writes ("[PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() 
> to not pass a structure by value"):
> > c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
> > justify passing precopy_stats by value.
> > 
> > Under no circumstances can the precopy callback ever be executing in a
> > separate address space.
> 
> This statement is true only if you think "the precopy callback" refers
> to the stub generated by libxl_save_msgs_gen.  But a more natural
> reading is that "the precopy callback" refers to the actual code which
> implements whatever logic is required.
> 
> In a system using libxl, that code definitely _is_ executing in a
> separate address space.  And passing the stats by value rather than
> reference does make it marginally easier.
> 
> > Switch the callback to passing by pointer which is far more efficient, and
> > drop the typedef (because none of the other callback have this oddity).
> 
> I would like you to expand on this efficiency argument.
> 
> Certainly, with libxl (which is the primary upstream-supported
> toolstack) there is no discernable efficiency gain here.  The data
> must be copied back and forth between processes.
> 

This is true.

> If you are talking about out-of-tree consumers then you should say
> so.  And you should also give a realistic explanation of the size of
> the supposed performance benefit.
> 
> (FAOD: Nacked-by: Ian Jackson )
> 
> Sorry,
> Ian.

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

Re: [Xen-devel] [PATCH v2] x86/hvm: Add MSR old value

[Wei Liu]

On Fri, Oct 13, 2017 at 03:50:57PM +0300, Alexandru Isaila wrote:
> This patch adds the old value param and the onchangeonly option
> to the VM_EVENT_REASON_MOV_TO_MSR event.
> 
> The param was added to the vm_event_mov_to_msr struct and to the
> hvm_monitor_msr function. Finally I've changed the bool_t param
> to a bool for the hvm_msr_write_intercept function.
> 
> Signed-off-by: Alexandru Isaila 
> Acked-by: Tamas K Lengyel 
> 
> ---
> Changes since V1:
>   - Removed Stray blanks inside the inner parentheses
>   - Added space after the if statement
>   - Added * 8 to the set/clear/test_bit statements
>   - Removed the blank line after monitored_msr.
> ---
>  tools/libxc/include/xenctrl.h |  2 +-
>  tools/libxc/xc_monitor.c  |  3 ++-

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Andrew Cooper]

On 16/10/17 16:51, Roger Pau Monné wrote:
> On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
>>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>>all state is actually set up.  As it currently stands, d0v0 is eligible 
>> for
>>scheduling before its registers have been set.  This is latent as we also
>>hold a systemcontroller pause reference at the time which prevents d0 from
>>being scheduled.
>>
>>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>>being able to call VCPUOP_initialise and modify state under the feet of 
>> the
>>running vcpu.  This is latent as PVH dom0 construction don't yet function.
>>
>> Signed-off-by: Andrew Cooper 
> LGTM, just one question.
>
>> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
>> index e8f746c..a67071c 100644
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
>> paddr_t entry,
>>  
>>  update_domain_wallclock_time(d);
>>  
>> +v->is_initialised = 1;
>>  clear_bit(_VPF_down, >pause_flags);
> Don't you want to move this to the end of dom0_construct_pvh? In any
> case, at this point the vCPU state is already setup correctly.

I had considered that, but a) As you say, it only matters when the vcpu
state is set up, and b) it would look odd being anywhere later.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Jan Beulich]

>>> On 16.10.17 at 18:07,  wrote:
> On 16/10/17 16:41, Jan Beulich wrote:
>>  >>> On 16.10.17 at 16:38,  wrote:
>>> --- a/xen/arch/x86/hvm/dom0_build.c
>>> +++ b/xen/arch/x86/hvm/dom0_build.c
>>> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
>>> paddr_t entry,
>>>  
>>>  update_domain_wallclock_time(d);
>>>  
>>> +v->is_initialised = 1;
>>>  clear_bit(_VPF_down, >pause_flags);
>> How come this has no counterpart of code being deleted?
> 
> Because the bug is that it was never being set before.

Oh, I see - I had read that part of the commit message in slightly
a wrong way.

Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Jan Beulich]

>>> On 16.10.17 at 17:45,  wrote:
> Jan Beulich writes ("[PATCH] libxc: don't fail domain creation when unpacking 
> initrd fails"):
>> At least Linux kernels have been able to work with gzip-ed initrd for
>> quite some time; initrd compressed with other methods aren't even being
>> attempted to unpack. Furthermore the unzip-ing routine used here isn't
>> capable of dealing with various forms of concatenated files, each of
>> which was gzip-ed separately (it is this particular case which has been
>> the source of observed VM creation failures).
> 
> I'm not sure I really like this approach of attempting to ungzip it
> and then falling back.  (And the size-checking logic is not
> particularly easy to follow.)
> 
> Is there no way to tell that a kernel supports gzipped initrds by
> looking at the kernel ?

Well, Linux kernels have config options controlling their ability. So
even a modern kernel _could_ be configured to require unzipping.
I didn't check whether they announce this anywhere outside the
(possibly) embedded .config, but even if they did this would be
only Linux then. A solution here shouldn't really be OS-specific imo.

>  A heuristic would probably do: it's OK if we
> sometimes insist on decompression ourselves, for a subset of old
> kernels where it's not needed.

Well, I specifically wanted to avoid any guesswork. But if I
simply had reported this as a problem that needs dealing with,
things likely would have gone like for the Python version issue
(which I still haven't got around to), asking me to look into
addressing it. So I thought I'd present a possible solution right
away. To be honest, if you want this to be done some
meaningfully different way which I'm not convinced of, I'm not
sure I'm the one to carry this out, yet I'd still request the
issue to be addressed.

Jan


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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Andrew Cooper]

On 16/10/17 16:39, Jan Beulich wrote:
 On 16.10.17 at 16:49,  wrote:
>> On 16/10/17 15:44, Wei Liu wrote:
>>> On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
all state is actually set up.  As it currently stands, d0v0 is eligible 
>> for
scheduling before its registers have been set.  This is latent as we 
 also
hold a systemcontroller pause reference at the time which prevents d0 
>> from
being scheduled.

  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another 
 vcpu
being able to call VCPUOP_initialise and modify state under the feet of 
>> the
running vcpu.  This is latent as PVH dom0 construction don't yet 
>> function.
>>> While I think this patch is a good idea, the above paragraph confuses
>>> me: I did boot PVH Dom0 at one point so it did function; I also never
>>> triggered a bug like the one described here.
>> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
>> path.
>>
>> The bottom of dom0_construct_pvh() currently has:
>>
>> ...
>> panic("Building a PVHv2 Dom0 is not yet supported.");
>> return 0;
>> }
>>
>> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
>> issue, because it wouldn't call VCPUOP_initialise against a running
>> vcpu.  Nevertheless, it is relevant to Xen's security that such an
>> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
>> under a running vcpu.
> I don't understand this reply of yours: The changes you make
> are for vCPU 0 only, and even there only for its initial state.

Correct.

> Even if Dom0 decided to bring down and back up that vCPU, it
> would go through a different path.

Correct as well, but unrelated to the bug.

The bug is that, at the moment, d0v1 can make a blind VCPUOP_initialise
call targeting d0v0, while d0v0 is running, and it will go and rewrite
state.  The problem is that d0v0 starts running in a way that
VCPUOP_initialise believes it to be eligible for initialisation.

All other mechanisms of bringing a vcpu down and up cause there to be
proper isolation between playing with a vcpus state, and it being
unscheduled.

~Andrew

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

Re: [Xen-devel] [xen-4.8-testing test] 114505: regressions - FAIL

[Jan Beulich]

>>> On 16.10.17 at 11:14,  wrote:
> On 15/10/17 20:45, osstest service owner wrote:
>> flight 114505 xen-4.8-testing real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/114505/ 
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>  test-xtf-amd64-amd64-2 48 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
> 114173
>>
>> Tests which are failing intermittently (not blocking):
>>  test-xtf-amd64-amd64-5 48 xtf/test-hvm64-lbr-tsx-vmentry fail in 114454 
> pass in 114505
> 
> Ian: These tests exercise something very machine specific, and the XTF
> tests really do need tying to specific hardware when making regression
> considerations.
> 
> Jan: This highlights that TSX/VMEntry failure fixes probably want
> backporting to before Xen 4.9.  IIRC, the 6 patches needed are:

So I'm mildly confused by this request:

> e3eb84e33c36 (only as a functional prerequisite)
> 9b93c6b3695b: x86/vmx: introduce vmx_find_msr()
> 7f11aa4b2b1f: x86/vmx: optimize vmx_read/write_guest_msr()
> d6e9f8d4f35d: x86/vmx: fix vmentry failure with TSX bits in LBR
> f97838bbd980: x86: Move microcode loading earlier

Up to here, everything is in 4.9 already afaict. Considering the
context here is a 4.8 test report, did you perhaps mean to ask
for this on 4.8 (and possibly also 4.7)? If so, I'm not really sure -
these changes taken together look a little large for the gain
they provide.

> 20f1976b4419: x86/vmx: Fix vmentry failure because of invalid LER on 
> Broadwell

I'll see to pull this one in for 4.9.1.

Jan


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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Andrew Cooper]

On 16/10/17 16:41, Jan Beulich wrote:
>  >>> On 16.10.17 at 16:38,  wrote:
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
>> paddr_t entry,
>>  
>>  update_domain_wallclock_time(d);
>>  
>> +v->is_initialised = 1;
>>  clear_bit(_VPF_down, >pause_flags);
> How come this has no counterpart of code being deleted?

Because the bug is that it was never being set before.

~Andrew

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

Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Ian Jackson]

Jan Beulich writes ("[PATCH] libxc: don't fail domain creation when unpacking 
initrd fails"):
> At least Linux kernels have been able to work with gzip-ed initrd for
> quite some time; initrd compressed with other methods aren't even being
> attempted to unpack. Furthermore the unzip-ing routine used here isn't
> capable of dealing with various forms of concatenated files, each of
> which was gzip-ed separately (it is this particular case which has been
> the source of observed VM creation failures).

I'm not sure I really like this approach of attempting to ungzip it
and then falling back.  (And the size-checking logic is not
particularly easy to follow.)

Is there no way to tell that a kernel supports gzipped initrds by
looking at the kernel ?  A heuristic would probably do: it's OK if we
sometimes insist on decompression ourselves, for a subset of old
kernels where it's not needed.

Ian.

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

Re: [Xen-devel] [PATCH for-next 1/3] x86/smp: Rework cpu_smpboot_alloc() to cope with more than just -ENOMEM

[Wei Liu]

On Mon, Oct 02, 2017 at 05:13:47PM +0100, Andrew Cooper wrote:
> No functional change.
> 
> 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] [PATCH v1] x86/hvm: Add MSR old value

[Wei Liu]

On Thu, Oct 12, 2017 at 12:10:25PM +0300, Alexandru Isaila wrote:
> This patch adds the old value param and the onchangeonly option
> to the VM_EVENT_REASON_MOV_TO_MSR event.
> 
> The param was added to the vm_event_mov_to_msr struct and to the
> hvm_monitor_msr function. Finally I've changed the bool_t param
> to a bool for the hvm_msr_write_intercept function.
> 
> Signed-off-by: Alexandru Isaila 
> ---
>  tools/libxc/include/xenctrl.h |  2 +-
>  tools/libxc/xc_monitor.c  |  3 ++-

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH for-next] xen/pv: Construct d0v0's GDT properly

[Jan Beulich]

>>> On 16.10.17 at 16:38,  wrote:
> c/s cf6d39f8199 "x86/PV: properly populate descriptor tables" changed the GDT
> to reference zero_page for intermediate frames between the guest and Xen
> frames.
> 
> Because dom0_construct_pv() doesn't call arch_set_info_guest(), some bits of
> initialisation are missed, including the pv_destroy_gdt() which initially
> fills the references to zero_page.
> 
> In practice, this means there is a window between starting and the first call
> to HYPERCALL_set_gdt() were lar/lsl/verr/verw suffer non-architectural
> behaviour.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

> This probably wants backporting to Xen 4.7 and later.

Could you remind me once the patch has gone in (as that'll be only
in quite a few weeks)?

Thanks, Jan


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

Re: [Xen-devel] libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add

[Jan Beulich]

>>> On 16.10.17 at 17:47,  wrote:
> On 16 October 2017 at 15:53, Jan Beulich  wrote:
> On 16.10.17 at 11:02,  wrote:
>>>  static int console_create_ring(struct console *con)
>>>  {
>>> - int err, remote_port, ring_ref, rc;
>>> + int err, remote_port, rc;
>>> + xen_pfn_t ring_ref;
>>>   char *type, path[PATH_MAX];
>>>   struct domain *dom = con->d;
>>>
>>>   err = xs_gather(xs, con->xspath,
>>> - "ring-ref", "%u", _ref,
>>> + "ring-ref", "%i", _ref,
>>
>> How would you gather a 64-bit value using %i without any length
>> modifier? With just %i you're even going to use partially initialized
>> data, so unless somewhere else the upper 32 bits got clipped off
>> again the console wouldn't work anymore.
>>
> I should use "%lli" here to read it as a 64-bit value for all
> architectures. Correct?

No, that's break for a 32-bit tool stack on x86. You really need
an abstraction similar to PRIu_xen_pfn, just that I'd rather not
see SCNi_xen_pfn added to the public headers.

Jan


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

Re: [Xen-devel] [PATCH v11 06/11] x86/hvm/ioreq: add a new mappable resource type...

[Jan Beulich]

>>> On 16.10.17 at 16:17,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 16 October 2017 15:07
>> >>> On 12.10.17 at 18:25,  wrote:
>> > ... XENMEM_resource_ioreq_server
>> >
>> > This patch adds support for a new resource type that can be mapped using
>> > the XENMEM_acquire_resource memory op.
>> >
>> > If an emulator makes use of this resource type then, instead of mapping
>> > gfns, the IOREQ server will allocate pages from the heap. These pages
>> > will never be present in the P2M of the guest at any point and so are
>> > not vulnerable to any direct attack by the guest. They are only ever
>> > accessible by Xen and any domain that has mapping privilege over the
>> > guest (which may or may not be limited to the domain running the
>> emulator).
>> >
>> > NOTE: Use of the new resource type is not compatible with use of
>> >   XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns
>> flag is
>> >   set.
>> >
>> > Signed-off-by: Paul Durrant 
>> > Acked-by: George Dunlap 
>> > Reviewed-by: Wei Liu 
>> 
>> Can you have validly retained this?
> 
> I didn't think the structure of this particular patch had changed that 
> fundamentally.

The structure didn't change that much, yes, but the page type
ref acquiring which you now do alter behavior meaningfully.

>> > @@ -777,6 +886,51 @@ int hvm_get_ioreq_server_info(struct domain *d,
>> ioservid_t id,
>> >  return rc;
>> >  }
>> >
>> > +int hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
>> > +   unsigned long idx, mfn_t *mfn)
>> > +{
>> > +struct hvm_ioreq_server *s;
>> > +int rc;
>> > +
>> > +spin_lock_recursive(>arch.hvm_domain.ioreq_server.lock);
>> > +
>> > +if ( id == DEFAULT_IOSERVID )
>> > +return -EOPNOTSUPP;
>> > +
>> > +s = get_ioreq_server(d, id);
>> > +
>> > +ASSERT(!IS_DEFAULT(s));
>> > +
>> > +rc = hvm_ioreq_server_alloc_pages(s);
>> > +if ( rc )
>> > +goto out;
>> > +
>> > +switch ( idx )
>> > +{
>> > +case XENMEM_resource_ioreq_server_frame_bufioreq:
>> > +rc = -ENOENT;
>> > +if ( !HANDLE_BUFIOREQ(s) )
>> > +goto out;
>> > +
>> > +*mfn = _mfn(page_to_mfn(s->bufioreq.page));
>> > +rc = 0;
>> > +break;
>> 
>> How about
>> 
>> if ( HANDLE_BUFIOREQ(s) )
>> *mfn = _mfn(page_to_mfn(s->bufioreq.page));
>> else
>> rc = -ENOENT;
>> break;
>> 
> 
> Looking at the overall structure I prefer it as it is. If I could have got 
> rid of the out label by doing this then it might have been worth the change.

Okay, you're the maintainer. Just to clarify - what I find particularly
odd is the setting of rc to zero above, yet the other case block
relying on it already being zero when entering the switch().

>> > @@ -629,6 +634,10 @@ struct xen_mem_acquire_resource {
>> >   *  is optional if nr_frames is 0.
>> >   */
>> >  uint64_aligned_t frame;
>> > +
>> > +#define XENMEM_resource_ioreq_server_frame_bufioreq 0
>> > +#define XENMEM_resource_ioreq_server_frame_ioreq(n_) (1 + (n_))
>> 
>> I don't see what you need the trailing underscore for. This is
>> normally only needed on local variables defined in (gcc extended)
>> macros, which we generally can't use in a public header anyway.
> 
> I thought it was generally desirable to attempt to distinguish macro 
> arguments from variable to avoid name clashes. What do you prefer I should do 
> in a public header?

There are various cases to be considered here, but in the one
here there is no risk of name clash at all: Regardless of the
name of the parameter, any instance of it will be expanded
exactly once. Even if the expansion matches exactly the
parameter name, no issue will arise. There are certainly forms
of macros where some care is needed in how to name the
parameters. Trailing underscores to disambiguate names,
however, should - as said - rarely if ever be needed for other
than local variables inside the macro body (because _then_
there indeed can be name conflicts with outer scope variables).

Jan

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Roger Pau Monné]

On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>all state is actually set up.  As it currently stands, d0v0 is eligible for
>scheduling before its registers have been set.  This is latent as we also
>hold a systemcontroller pause reference at the time which prevents d0 from
>being scheduled.
> 
>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>being able to call VCPUOP_initialise and modify state under the feet of the
>running vcpu.  This is latent as PVH dom0 construction don't yet function.
> 
> Signed-off-by: Andrew Cooper 

LGTM, just one question.

> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index e8f746c..a67071c 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
> paddr_t entry,
>  
>  update_domain_wallclock_time(d);
>  
> +v->is_initialised = 1;
>  clear_bit(_VPF_down, >pause_flags);

Don't you want to move this to the end of dom0_construct_pvh? In any
case, at this point the vCPU state is already setup correctly.

Thanks, Roger.

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

Re: [Xen-devel] libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add

[Bhupinder Thakur]

On 16 October 2017 at 15:53, Jan Beulich  wrote:
 On 16.10.17 at 11:02,  wrote:
>>  static int console_create_ring(struct console *con)
>>  {
>> - int err, remote_port, ring_ref, rc;
>> + int err, remote_port, rc;
>> + xen_pfn_t ring_ref;
>>   char *type, path[PATH_MAX];
>>   struct domain *dom = con->d;
>>
>>   err = xs_gather(xs, con->xspath,
>> - "ring-ref", "%u", _ref,
>> + "ring-ref", "%i", _ref,
>
> How would you gather a 64-bit value using %i without any length
> modifier? With just %i you're even going to use partially initialized
> data, so unless somewhere else the upper 32 bits got clipped off
> again the console wouldn't work anymore.
>
I should use "%lli" here to read it as a 64-bit value for all
architectures. Correct?

Regards,
Bhupinder

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

Re: [Xen-devel] [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Jan Beulich]

>>> On 16.10.17 at 17:05,  wrote:
> On 10/16/2017 01:32 PM, Jan Beulich wrote:
>> Since the emulator acts on the live hardware registers, we need to
>> prevent the compiler from using them e.g. for inlined memcpy() /
>> memset() (as gcc7 does). 
> 
> Why doesn't this affect the rest of the hypervisor too, since we don't
> save and restore the *mm registers?

Because we build the hypervisor with -mno-sse.

>> We can't, however, set this from the command
>> line, as otherwise the 64-bit build would face issues with functions
>> returning floating point values and being declared in standard headers.
> 
> Sorry, just to clarify: You mean that there are standard headers which
> contain prototypes for functions which return floating point values; we
> include those headers but do not call the functions; and adding the
> #pragma to the command-line would cause the compiler to choke on the
> prototypes (even though the functions are never actually called)?

Yes (adding the command line option equivalent of the pragma,
that is).

Jan


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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Jan Beulich]

 >>> On 16.10.17 at 16:38,  wrote:
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
> paddr_t entry,
>  
>  update_domain_wallclock_time(d);
>  
> +v->is_initialised = 1;
>  clear_bit(_VPF_down, >pause_flags);

How come this has no counterpart of code being deleted?

Jan


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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Jan Beulich]

>>> On 16.10.17 at 16:49,  wrote:
> On 16/10/17 15:44, Wei Liu wrote:
>> On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
>>>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>>>all state is actually set up.  As it currently stands, d0v0 is eligible 
> for
>>>scheduling before its registers have been set.  This is latent as we also
>>>hold a systemcontroller pause reference at the time which prevents d0 
> from
>>>being scheduled.
>>>
>>>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another 
>>> vcpu
>>>being able to call VCPUOP_initialise and modify state under the feet of 
> the
>>>running vcpu.  This is latent as PVH dom0 construction don't yet 
> function.
>>>
>> While I think this patch is a good idea, the above paragraph confuses
>> me: I did boot PVH Dom0 at one point so it did function; I also never
>> triggered a bug like the one described here.
> 
> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
> path.
> 
> The bottom of dom0_construct_pvh() currently has:
> 
> ...
> panic("Building a PVHv2 Dom0 is not yet supported.");
> return 0;
> }
> 
> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
> issue, because it wouldn't call VCPUOP_initialise against a running
> vcpu.  Nevertheless, it is relevant to Xen's security that such an
> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
> under a running vcpu.

I don't understand this reply of yours: The changes you make
are for vCPU 0 only, and even there only for its initial state.
Even if Dom0 decided to bring down and back up that vCPU, it
would go through a different path.

Jan


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

[Xen-devel] Xen 4.10 RC1

[Julien Grall]

Hi all,

Xen 4.10 RC1 is tagged. You can check that out from xen.git:

  git://xenbits.xen.org/xen.git 4.10.0-rc1

For your convenience there is also a tarball at:
https://downloads.xenproject.org/release/xen/4.10.0-rc1/xen-4.10.0-rc1.tar.gz

And the signature is at:
https://downloads.xenproject.org/release/xen/4.10.0-rc1/xen-4.10.0-rc1.tar.gz.sig

Please send bug reports and test reports to
xen-de...@lists.xenproject.org. When sending bug reports, please CC
relevant maintainers and me (julien.gr...@linaro.org).

Thanks,

-- 
Julien Grall

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

Re: [Xen-devel] [xen-4.8-testing test] 114505: regressions - FAIL

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [xen-4.8-testing test] 114505: 
regressions - FAIL"):
> On 15/10/17 20:45, osstest service owner wrote:
> > flight 114505 xen-4.8-testing real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/114505/
> >
> > Regressions :-(
> >
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> >  test-xtf-amd64-amd64-2 48 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
> > 114173
...
> Ian: These tests exercise something very machine specific, and the XTF
> tests really do need tying to specific hardware when making regression
> considerations.

Is this test new enough that it might have never run on that
hardware ?  If so then a force push might be justified.

It is difficult to tie the tests to specific hardware without
insisting that every run uses every host.

Ian.

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

Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to 
not pass a structure by value"):
> On Mon, Oct 16, 2017 at 02:51:54PM +0100, Andrew Cooper wrote:
...
> > With Joshua's patch in place, the implementer of this callback is the
> > code generated by libxl_save_msgs_gen.pl, which is the aformentioned
> > extra process.  Passing by pointer or value has nothing to do with the
> > fact that the automatically generated code needs to know how to
> > serialise/deserialise the data to feed it back to the main process.
> 
> Right. I agree with you here after going back to the old thread.

ISTM that the callback being a struct rather than a pointer does make
the code in libxl_save_msgs_gen.pl simpler, since it can simply memcpy
the struct.

I certainly dislike your 1/2 patch with the current commit message.

Andrew Cooper writes ("[PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() 
to not pass a structure by value"):
> c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
> justify passing precopy_stats by value.
> 
> Under no circumstances can the precopy callback ever be executing in a
> separate address space.

This statement is true only if you think "the precopy callback" refers
to the stub generated by libxl_save_msgs_gen.  But a more natural
reading is that "the precopy callback" refers to the actual code which
implements whatever logic is required.

In a system using libxl, that code definitely _is_ executing in a
separate address space.  And passing the stats by value rather than
reference does make it marginally easier.

> Switch the callback to passing by pointer which is far more efficient, and
> drop the typedef (because none of the other callback have this oddity).

I would like you to expand on this efficiency argument.

Certainly, with libxl (which is the primary upstream-supported
toolstack) there is no discernable efficiency gain here.  The data
must be copied back and forth between processes.

If you are talking about out-of-tree consumers then you should say
so.  And you should also give a realistic explanation of the size of
the supposed performance benefit.

(FAOD: Nacked-by: Ian Jackson )

Sorry,
Ian.

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

[Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails

[Jan Beulich]

At least Linux kernels have been able to work with gzip-ed initrd for
quite some time; initrd compressed with other methods aren't even being
attempted to unpack. Furthermore the unzip-ing routine used here isn't
capable of dealing with various forms of concatenated files, each of
which was gzip-ed separately (it is this particular case which has been
the source of observed VM creation failures).

Hence, if unpacking fails, simply hand the the compressed blob to the
guest as is.

Signed-off-by: Jan Beulich 
---
I'm not intending to request this to go into 4.10, but I certainly
wouldn't mind. I would appreciate though if I could at least get some
initial feedback earlier than when 4.10 branches off, as we will want
to use a backport of this in our trees, which I'd prefer to be in
line with what is eventually going to go into master.

--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -291,7 +291,6 @@ int xc_dom_mem_init(struct xc_dom_image
 int xc_dom_kernel_check_size(struct xc_dom_image *dom, size_t sz);
 int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
 
-int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz);
 int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
 
 int xc_dom_devicetree_max_size(struct xc_dom_image *dom, size_t sz);
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -314,7 +314,8 @@ int xc_dom_kernel_check_size(struct xc_d
 return 0;
 }
 
-int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz)
+static int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz,
+ size_t raw)
 {
 /* No limit */
 if ( !dom->max_ramdisk_size )
@@ -322,8 +323,9 @@ int xc_dom_ramdisk_check_size(struct xc_
 
 if ( sz > dom->max_ramdisk_size )
 {
-xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
- "ramdisk image too large");
+if ( raw > dom->max_ramdisk_size )
+xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
+ "ramdisk image too large");
 return 1;
 }
 
@@ -999,13 +1001,13 @@ static int xc_dom_build_ramdisk(struct x
 {
 unziplen = xc_dom_check_gzip(dom->xch,
  dom->ramdisk_blob, dom->ramdisk_size);
-if ( xc_dom_ramdisk_check_size(dom, unziplen) != 0 )
+if ( xc_dom_ramdisk_check_size(dom, unziplen, dom->ramdisk_size) != 0 )
 unziplen = 0;
 }
 else
 unziplen = 0;
 
-ramdisklen = unziplen ? unziplen : dom->ramdisk_size;
+ramdisklen = max(unziplen, dom->ramdisk_size);
 
 if ( xc_dom_alloc_segment(dom, >ramdisk_seg, "ramdisk",
   dom->ramdisk_seg.vstart, ramdisklen) != 0 )
@@ -1017,14 +1019,15 @@ static int xc_dom_build_ramdisk(struct x
   __FUNCTION__);
 goto err;
 }
-if ( unziplen )
+if ( !unziplen ||
+ xc_dom_do_gunzip(dom->xch, dom->ramdisk_blob, dom->ramdisk_size,
+  ramdiskmap, unziplen) == -1 )
 {
-if ( xc_dom_do_gunzip(dom->xch, dom->ramdisk_blob, dom->ramdisk_size,
-  ramdiskmap, ramdisklen) == -1 )
-goto err;
-}
-else
 memcpy(ramdiskmap, dom->ramdisk_blob, dom->ramdisk_size);
+if ( unziplen > dom->ramdisk_size )
+memset(ramdiskmap + dom->ramdisk_size, 0,
+   unziplen - dom->ramdisk_size);
+}
 
 return 0;
 




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

Re: [Xen-devel] [PATCH for-4.10 2/2] tools/libxc: Fix various code smells in send_memory_live()

[Ian Jackson]

~Andrew Cooper writes ("[PATCH for-4.10 2/2] tools/libxc: Fix various code 
smells in send_memory_live()"):
>  * Don't zero ctx->save.stats; it is already zeroed
>  * No need for x as it duplicates ctx->save.stats.iteration
>  * Defer setting dirty_count until the bitmap has been filled to match the
>behaviour of XEN_DOMCTL_SHADOW_OP_CLEAN
>  * Drop spurious blank line

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[George Dunlap]

On 10/16/2017 01:32 PM, Jan Beulich wrote:
> Since the emulator acts on the live hardware registers, we need to
> prevent the compiler from using them e.g. for inlined memcpy() /
> memset() (as gcc7 does). 

Why doesn't this affect the rest of the hypervisor too, since we don't
save and restore the *mm registers?

> We can't, however, set this from the command
> line, as otherwise the 64-bit build would face issues with functions
> returning floating point values and being declared in standard headers.

Sorry, just to clarify: You mean that there are standard headers which
contain prototypes for functions which return floating point values; we
include those headers but do not call the functions; and adding the
#pragma to the command-line would cause the compiler to choke on the
prototypes (even though the functions are never actually called)?

 -George


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

Re: [Xen-devel] [PATCH] xen-netfront, xen-netback: Use correct minimum MTU values

[Wei Liu]

On Mon, Oct 16, 2017 at 03:20:32PM +0200, Mohammed Gamal wrote:
> RFC791 specifies the minimum MTU to be 68, while xen-net{front|back}
> drivers use a minimum value of 0.
> 
> When set MTU to 0~67 with xen_net{front|back} driver, the network
> will become unreachable immediately, the guest can no longer be pinged.
> 
> xen_net{front|back} should not allow the user to set this value which causes
> network problems.
> 
> Reported-by: Chen Shi 
> Signed-off-by: Mohammed Gamal 

Acked-by: Wei Liu 

CC netfront maintainers

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Wei Liu]

On Mon, Oct 16, 2017 at 03:49:54PM +0100, Andrew Cooper wrote:
> On 16/10/17 15:44, Wei Liu wrote:
> > On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
> >>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
> >>all state is actually set up.  As it currently stands, d0v0 is eligible 
> >> for
> >>scheduling before its registers have been set.  This is latent as we 
> >> also
> >>hold a systemcontroller pause reference at the time which prevents d0 
> >> from
> >>being scheduled.
> >>
> >>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another 
> >> vcpu
> >>being able to call VCPUOP_initialise and modify state under the feet of 
> >> the
> >>running vcpu.  This is latent as PVH dom0 construction don't yet 
> >> function.
> >>
> > While I think this patch is a good idea, the above paragraph confuses
> > me: I did boot PVH Dom0 at one point so it did function; I also never
> > triggered a bug like the one described here.
> 
> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
> path.
> 
> The bottom of dom0_construct_pvh() currently has:
> 
> ...
>     panic("Building a PVHv2 Dom0 is not yet supported.");
>     return 0;
> }
> 

Oh yes, I was using a development branch.

> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
> issue, because it wouldn't call VCPUOP_initialise against a running
> vcpu.  Nevertheless, it is relevant to Xen's security that such an
> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
> under a running vcpu.
> 

Right.

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

Re: [Xen-devel] [PATCH for-4.10] libxl: annotate s to be nonnull in libxl__enum_from_string

[Ian Jackson]

Wei Liu writes ("[PATCH for-4.10] libxl: annotate s to be nonnull in 
libxl__enum_from_string"):
> Hope this can placate coverity.

Acked-by: Ian Jackson 

We are very soon going to want "NN2" and maybe "NN_1_2".

Ian.

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

Re: [Xen-devel] [PATCH] xen-netfront, xen-netback: Use correct minimum MTU values

[Eduardo Otubo]

On Mon, Oct 16, 2017 at 03:20:32PM +0200, Mohammed Gamal wrote:
> RFC791 specifies the minimum MTU to be 68, while xen-net{front|back}
> drivers use a minimum value of 0.
> 
> When set MTU to 0~67 with xen_net{front|back} driver, the network
> will become unreachable immediately, the guest can no longer be pinged.
> 
> xen_net{front|back} should not allow the user to set this value which causes
> network problems.
> 
> Reported-by: Chen Shi 
> Signed-off-by: Mohammed Gamal 
> ---
>  drivers/net/xen-netback/interface.c | 2 +-
>  drivers/net/xen-netfront.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c 
> b/drivers/net/xen-netback/interface.c
> index ee8ed9da..4491ca5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -486,7 +486,7 @@ struct xenvif *xenvif_alloc(struct device *parent, 
> domid_t domid,
>  
>   dev->tx_queue_len = XENVIF_QUEUE_LENGTH;
>  
> - dev->min_mtu = 0;
> + dev->min_mtu = ETH_MIN_MTU;
>   dev->max_mtu = ETH_MAX_MTU - VLAN_ETH_HLEN;
>  
>   /*
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 523387e..8b8689c 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1316,7 +1316,7 @@ static struct net_device *xennet_create_dev(struct 
> xenbus_device *dev)
>   netdev->features |= netdev->hw_features;
>  
>   netdev->ethtool_ops = _ethtool_ops;
> - netdev->min_mtu = 0;
> + netdev->min_mtu = ETH_MIN_MTU;
>   netdev->max_mtu = XEN_NETIF_MAX_TX_SIZE;
>   SET_NETDEV_DEV(netdev, >dev);
>  
> -- 
> 1.8.3.1
> 

Acked-by: Eduardo Otubo 

-- 
Eduardo Otubo
Senior Software Engineer @ RedHat

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Wei Liu]

On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>all state is actually set up.  As it currently stands, d0v0 is eligible for
>scheduling before its registers have been set.  This is latent as we also
>hold a systemcontroller pause reference at the time which prevents d0 from
>being scheduled.
> 
>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>being able to call VCPUOP_initialise and modify state under the feet of the
>running vcpu.  This is latent as PVH dom0 construction don't yet function.
> 

While I think this patch is a good idea, the above paragraph confuses
me: I did boot PVH Dom0 at one point so it did function; I also never
triggered a bug like the one described here.

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

[Xen-devel] [PATCH for-next] xen/pv: Construct d0v0's GDT properly

[Andrew Cooper]

c/s cf6d39f8199 "x86/PV: properly populate descriptor tables" changed the GDT
to reference zero_page for intermediate frames between the guest and Xen
frames.

Because dom0_construct_pv() doesn't call arch_set_info_guest(), some bits of
initialisation are missed, including the pv_destroy_gdt() which initially
fills the references to zero_page.

In practice, this means there is a window between starting and the first call
to HYPERCALL_set_gdt() were lar/lsl/verr/verw suffer non-architectural
behaviour.

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

This probably wants backporting to Xen 4.7 and later.
---
 xen/arch/x86/pv/dom0_build.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 8ad7e3d..984a94a 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "mm.h"
@@ -867,6 +868,13 @@ int __init dom0_construct_pv(struct domain *d,
 regs->rsi = vstartinfo_start;
 regs->eflags = X86_EFLAGS_IF;
 
+/*
+ * We don't call arch_set_info_guest(), so some initialisation needs doing
+ * by hand:
+ *  - Reset the GDT to reference zero_page
+ */
+pv_destroy_gdt(v);
+
 if ( test_bit(XENFEAT_supervisor_mode_kernel, parms.f_required) )
 panic("Dom0 requires supervisor-mode execution");
 
-- 
2.1.4


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

[Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Andrew Cooper]

 * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
   all state is actually set up.  As it currently stands, d0v0 is eligible for
   scheduling before its registers have been set.  This is latent as we also
   hold a systemcontroller pause reference at the time which prevents d0 from
   being scheduled.

 * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
   being able to call VCPUOP_initialise and modify state under the feet of the
   running vcpu.  This is latent as PVH dom0 construction don't yet function.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Wei Liu 
CC: Roger Pau Monné 
---
 xen/arch/arm/domain_build.c   |  6 +++---
 xen/arch/x86/dom0_build.c | 13 +++--
 xen/arch/x86/hvm/dom0_build.c |  1 +
 xen/arch/x86/pv/dom0_build.c  |  6 +++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4636b17..bf29299 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2197,9 +2197,6 @@ int construct_dom0(struct domain *d)
 
 discard_initial_modules();
 
-v->is_initialised = 1;
-clear_bit(_VPF_down, >pause_flags);
-
 memset(regs, 0, sizeof(*regs));
 
 regs->pc = (register_t)kinfo.entry;
@@ -2247,6 +2244,9 @@ int construct_dom0(struct domain *d)
 vcpu_switch_to_aarch64_mode(d->vcpu[i]);
 }
 
+v->is_initialised = 1;
+clear_bit(_VPF_down, >pause_flags);
+
 return 0;
 }
 
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index e4bffd5..bf992fe 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -466,6 +466,8 @@ int __init construct_dom0(struct domain *d, const module_t 
*image,
   void *(*bootstrap_map)(const module_t *),
   char *cmdline)
 {
+int rc;
+
 /* Sanity! */
 BUG_ON(d->domain_id != 0);
 BUG_ON(d->vcpu[0] == NULL);
@@ -481,8 +483,15 @@ int __init construct_dom0(struct domain *d, const module_t 
*image,
 }
 #endif
 
-return (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
-   (d, image, image_headroom, initrd,bootstrap_map, cmdline);
+rc = (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
+ (d, image, image_headroom, initrd, bootstrap_map, cmdline);
+if ( rc )
+return rc;
+
+/* Sanity! */
+BUG_ON(!d->vcpu[0]->is_initialised);
+
+return 0;
 }
 
 /*
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index e8f746c..a67071c 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, paddr_t 
entry,
 
 update_domain_wallclock_time(d);
 
+v->is_initialised = 1;
 clear_bit(_VPF_down, >pause_flags);
 
 return 0;
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index dcbee43..8ad7e3d 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -847,9 +847,6 @@ int __init dom0_construct_pv(struct domain *d,
 
 update_domain_wallclock_time(d);
 
-v->is_initialised = 1;
-clear_bit(_VPF_down, >pause_flags);
-
 /*
  * Initial register values:
  *  DS,ES,FS,GS = FLAT_KERNEL_DS
@@ -883,6 +880,9 @@ int __init dom0_construct_pv(struct domain *d,
 if ( d->domain_id == hardware_domid )
 iommu_hwdom_init(d);
 
+v->is_initialised = 1;
+clear_bit(_VPF_down, >pause_flags);
+
 return 0;
 
 out:
-- 
2.1.4


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

Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Wei Liu]

On Mon, Oct 16, 2017 at 02:51:54PM +0100, Andrew Cooper wrote:
> On 16/10/17 14:40, Wei Liu wrote:
> > On Fri, Oct 13, 2017 at 06:32:18PM +0100, Andrew Cooper wrote:
> >> c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
> >> justify passing precopy_stats by value.
> >>
> >> Under no circumstances can the precopy callback ever be executing in a
> >> separate address space.
> >>
> > The callback is not executed in a separate address space. 
> >
> > Have you checked
> > <1506365735-133776-4-git-send-email-jennifer.herb...@citrix.com>?
> >
> > The open source toolstack spawns another process to save vm image. In
> > order to let libxl control the process (in the future) there is
> > information passed across process boundary.
> >
> > Your code might work for now because Joshua's patch is not yet applied.
> 
> I'm perfectly aware of that discussion, and it is factually incorrect. 
> Nothing, not even Joshua's patch, can cause the callback to be executed
> in a separate address space.
> 
> With Joshua's patch in place, the implementer of this callback is the
> code generated by libxl_save_msgs_gen.pl, which is the aformentioned
> extra process.  Passing by pointer or value has nothing to do with the
> fact that the automatically generated code needs to know how to
> serialise/deserialise the data to feed it back to the main process.
> 

Right. I agree with you here after going back to the old thread.

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

Re: [Xen-devel] [RFC 1/4] arm: add SMC wrapper that is compatible with SMCCC

[Julien Grall]

Hi Volodymyr,

On 11/10/17 20:01, Volodymyr Babchuk wrote:

Existing SMC wrapper call_smc() allows only 4 parameters and
returns only one value. This is enough for existing
use in PSCI code, but TEE mediator will need a call that is
fully compatible with ARM SMCCC.
This patch adds this call for both arm32 and arm64.

There was similar patch by Edgar E. Iglesias ([1]), but looks
like it is abandoned.

[1] https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html

CC: "Edgar E. Iglesias" 

Signed-off-by: Volodymyr Babchuk 
---
  xen/arch/arm/arm32/Makefile |  1 +
  xen/arch/arm/arm32/smc.S| 32 
  xen/arch/arm/arm64/Makefile |  1 +
  xen/arch/arm/arm64/smc.S| 29 +
  xen/include/asm-arm/processor.h |  4 
  5 files changed, 67 insertions(+)
  create mode 100644 xen/arch/arm/arm32/smc.S
  create mode 100644 xen/arch/arm/arm64/smc.S

diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 0ac254f..c69f35e 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
  obj-y += smpboot.o
  obj-y += traps.o
  obj-y += vfp.o
+obj-y += smc.o
  
diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S

new file mode 100644
index 000..1cc9528
--- /dev/null
+++ b/xen/arch/arm/arm32/smc.S
@@ -0,0 +1,32 @@
+/*
+ * xen/arch/arm/arm32/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ * register_t a3, register_t a4, register_t a5,
+ * register_t a6, register_t a7, register_t res[4])
+ */
+ENTRY(call_smccc_smc)
+mov r12, sp
+push{r4-r7}
+ldm r12, {r4-r7}
+smc #0
+pop {r4-r7}
+ldr r12, [sp, #(4 * 4)]


Can we get some comment in the code to explain the hardcoded values? And 
maybe introduce defines?



+stm r12, {r0-r3}
+bx  lr
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 718fe44..58a8ddd 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -8,6 +8,7 @@ obj-y += entry.o
  obj-y += insn.o
  obj-$(CONFIG_LIVEPATCH) += livepatch.o
  obj-y += smpboot.o
+obj-y += smc.o
  obj-y += traps.o
  obj-y += vfp.o
  obj-y += vsysreg.o
diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
new file mode 100644
index 000..aa44fba
--- /dev/null
+++ b/xen/arch/arm/arm64/smc.S
@@ -0,0 +1,29 @@
+/*
+ * xen/arch/arm/arm64/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ * register_t a3, register_t a4, register_t a5,
+ * register_t a6, register_t a7, register_t res[4])
+ */
+ENTRY(call_smccc_smc)
+smc #0
+ldr x4, [sp]
+stp x0, x1, [x4, 0]
+stp x2, x3, [x4, 16]


Ditto.


+ret
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index dc6ab62..71f3e60 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -787,6 +787,10 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
  int call_smc(register_t function_id, register_t arg0, register_t arg1,
   register_t arg2);
  
+void call_smccc_smc(register_t a0, register_t a1, register_t a2,

+register_t a3, register_t a4, register_t a5,
+register_t a6, register_t a7, register_t res[4]);


I would much prefer if you introduce a structure describing the result. 
This would make easier for the caller to understand what the result look 
like and avoid to hardcode some values in the code.


So how Linux does it.


+
  void do_trap_hyp_serror(struct cpu_user_regs *regs);
 

Re: [Xen-devel] [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Jan Beulich]

>>> On 16.10.17 at 15:46,  wrote:
> On Mon, Oct 16, 2017 at 08:26:09AM -0600, Jan Beulich wrote:
> On 16.10.17 at 15:13,  wrote:
>>> On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
>>> On 13.10.17 at 07:10,  wrote:
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned 
> int 
> gsi)
>  spin_unlock(>arch.hvm_domain.irq_lock);
>  }
>  
> -void hvm_isa_irq_assert(
> -struct domain *d, unsigned int isa_irq)
> +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
> +   int (*get_vector)(const struct domain *d,
> + unsigned int gsi))
>  {
>  struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>  unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
> +int vector = 0;

Why zero (which is valid aiui) instead of e.g. -1?
>>> 
>>> vector also serves as the return value. I want to return 0 if no
>>> callback is set.  And the callback, get_vector, can override the return
>>> value. Do you think it is reasonable?
>>
>>Why "also" - being the return value is the only purpose of "vector".
>>And as said - zero is a valid vector, and I wouldn't like to see the
>>function return a valid but meaningless vector number.
> 
> But if no callback is set, would it be a little weird to return -1 which
> always means failure?

To me, -1 doesn't mean "failure" here, but "no valid vector".

Jan


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

Re: [Xen-devel] [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Konrad Rzeszutek Wilk]

On Sun, Oct 15, 2017 at 03:31:15AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 13, 2017 at 03:46:39PM -0700, Stefano Stabellini wrote:
> > On Fri, 13 Oct 2017, Jan Beulich wrote:
> > > >>> On 13.10.17 at 13:13,  wrote:
> > > > To Jan, Andrew, Stefano and Anthony,
> > > > 
> > > > what do you think about allowing QEMU to build the entire guest ACPI
> > > > and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> > > > still there and just bypassed in this case.
> > > 
> > > Well, if that can be made work in a non-quirky way and without
> > > loss of functionality, I'd probably be fine. I do think, however,
> > > that there's a reason this is being handled in hvmloader right now.
> > 
> > And not to discourage you, just as a clarification, you'll also need to
> > consider backward compatibility: unless the tables are identical, I
> > imagine we'll have to keep using the old tables for already installed
> > virtual machines.
> 
> Maybe you can handle this using machine type versioning.

 And the type could be v2 if nvdimm was provided (which is
something that the toolstack would figure out).

The toolstack could also have a seperate 'v2' config flag if somebody
wanted to play with this _outside_ of having NVDIMM in the guest?


> Installed guests would use the old type.



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

Re: [Xen-devel] [PATCH v3 6/6] x86/msr: handle VMX MSRs with guest_rd/wrmsr()

[Sergey Dyasli]

On Fri, 2017-10-13 at 16:38 +0100, Andrew Cooper wrote:
> On 13/10/17 13:35, Sergey Dyasli wrote:
> > diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> > index a22e3dfaf2..2527fdd1d1 100644
> > --- a/xen/arch/x86/msr.c
> > +++ b/xen/arch/x86/msr.c
> > @@ -426,6 +426,13 @@ int init_vcpu_msr_policy(struct vcpu *v)
> >  return 0;
> >  }
> >  
> > +#define vmx_guest_rdmsr(dp, name, msr) \
> > +case name: \
> > +if ( !dp->msr.available )  \
> > +goto gp_fault; \
> > +*val = dp->msr.u.raw;  \
> > +break;
> 
> Eww :(
> 
> For blocks of MSRs, it would be far better to go with the same structure
> as the cpuid policy.  Something like:
> 
> struct {
>     union {
>         uint64_t raw[NR_VMX_MSRS];
>         struct {
>             struct {
>                 ...
>             } basic;
>             struct {
>                 ...
>             } pinbased_ctls;
>         };
>     };
> } vmx;
> 
> This way, the guest_rdmsr() will be far more efficient.
> 
> case MSR_IA32_VMX_BASIC ... xxx:
>     if ( !cpuid->basic.vmx )
>         goto gp_fault;
>     *val = dp->vmx.raw[msr - MSR_IA32_VMX_BASIC];
>     break;
> 
> It would probably be worth splitting into a couple of different blocks
> based on the different availability checks.

I can understand an argument about removing available flags and getting
smaller msr policy's struct, but I fail to see how a big number of case
statements will make guest_rdmsr() inefficient. I expect a switch
statement to have O(log(N)) complexity which means it doesn't really
matter how many case statements there are.

Do you have some other performance concerns?

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

Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Andrew Cooper]

On 16/10/17 15:44, Wei Liu wrote:
> On Mon, Oct 16, 2017 at 03:38:03PM +0100, Andrew Cooper wrote:
>>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>>all state is actually set up.  As it currently stands, d0v0 is eligible 
>> for
>>scheduling before its registers have been set.  This is latent as we also
>>hold a systemcontroller pause reference at the time which prevents d0 from
>>being scheduled.
>>
>>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>>being able to call VCPUOP_initialise and modify state under the feet of 
>> the
>>running vcpu.  This is latent as PVH dom0 construction don't yet function.
>>
> While I think this patch is a good idea, the above paragraph confuses
> me: I did boot PVH Dom0 at one point so it did function; I also never
> triggered a bug like the one described here.

Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
path.

The bottom of dom0_construct_pvh() currently has:

...
    panic("Building a PVHv2 Dom0 is not yet supported.");
    return 0;
}

As for the v->is_initialised, a well behaved dom0 wouldn't hit the
issue, because it wouldn't call VCPUOP_initialise against a running
vcpu.  Nevertheless, it is relevant to Xen's security that such an
attempt doesn't get to the point of actually trying to edit the VMC{S,B}
under a running vcpu.

~Andrew

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

Re: [Xen-devel] [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Chao Gao]

On Mon, Oct 16, 2017 at 08:26:09AM -0600, Jan Beulich wrote:
 On 16.10.17 at 15:13,  wrote:
>> On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
>> On 13.10.17 at 07:10,  wrote:
 --- a/xen/arch/x86/hvm/irq.c
 +++ b/xen/arch/x86/hvm/irq.c
 @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned int 
 gsi)
  spin_unlock(>arch.hvm_domain.irq_lock);
  }
  
 -void hvm_isa_irq_assert(
 -struct domain *d, unsigned int isa_irq)
 +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
 +   int (*get_vector)(const struct domain *d,
 + unsigned int gsi))
  {
  struct hvm_irq *hvm_irq = hvm_domain_irq(d);
  unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
 +int vector = 0;
>>>
>>>Why zero (which is valid aiui) instead of e.g. -1?
>> 
>> vector also serves as the return value. I want to return 0 if no
>> callback is set.  And the callback, get_vector, can override the return
>> value. Do you think it is reasonable?
>
>Why "also" - being the return value is the only purpose of "vector".
>And as said - zero is a valid vector, and I wouldn't like to see the
>function return a valid but meaningless vector number.

But if no callback is set, would it be a little weird to return -1 which
always means failure? Considering no caller would be confused by the
return value (since except the caller introduced by this patch, no one
would check the return value), I don't insist on this.

>
 --- a/xen/include/asm-x86/hvm/vmx/vmx.h
 +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
 @@ -109,6 +109,11 @@ static inline int pi_test_and_set_pir(int vector, 
 struct pi_desc *pi_desc)
  return test_and_set_bit(vector, pi_desc->pir);
  }
  
 +static inline int pi_test_pir(int vector, const struct pi_desc *pi_desc)
>>>
>>>This should not be a signed quantity - uint8_t or unsigned int
>>>please.
>> 
>> Yes.
>
>I.e. meaning you're fine with either variant, leaving it up to me
>which one to use?

Yes, both of them are ok to me.

Thanks
Chao

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

Re: [Xen-devel] [RFC 4/4] arm: tee: add basic OP-TEE mediator

[Julien Grall]

Hi Volodymyr,

On 11/10/17 20:01, Volodymyr Babchuk wrote:

Add basic OP-TEE mediator as an example how TEE mediator framework
works.

Currently it support only calls from Dom0. Calls from other guests
will be declined. It maps OP-TEE static shared memory region into
Dom0 address space, so Dom0 is the only domain which can work with
older versions of OP-TEE.

Also it alters SMC requests by\ adding domain id to request. OP-TEE
can use this information to track requesters.

Albeit being in early development stages, this mediator already can
be used on systems where only Dom0 interacts with OP-TEE.


A link to the spec would be useful here to be able to fully review this 
patch.




It was tested on RCAR Salvator-M3 board.


Is it with the stock op-tee? Or an updated version?



Signed-off-by: Volodymyr Babchuk 
---
  xen/arch/arm/tee/Kconfig  |   4 ++
  xen/arch/arm/tee/Makefile |   1 +
  xen/arch/arm/tee/optee.c  | 178 ++
  3 files changed, 183 insertions(+)
  create mode 100644 xen/arch/arm/tee/optee.c

diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
index e69de29..7c6b5c6 100644
--- a/xen/arch/arm/tee/Kconfig
+++ b/xen/arch/arm/tee/Kconfig
@@ -0,0 +1,4 @@
+config ARM_OPTEE
+   bool "Enable OP-TEE mediator"
+   default n
+   depends on ARM_TEE
diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
index c54d479..9d93b42 100644
--- a/xen/arch/arm/tee/Makefile
+++ b/xen/arch/arm/tee/Makefile
@@ -1 +1,2 @@
  obj-y += tee.o
+obj-$(CONFIG_ARM_OPTEE) += optee.o
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
new file mode 100644
index 000..0220691
--- /dev/null
+++ b/xen/arch/arm/tee/optee.c
@@ -0,0 +1,178 @@
+/*
+ * xen/arch/arm/tee/optee.c
+ *
+ * OP-TEE mediator
+ *
+ * Volodymyr Babchuk 
+ * Copyright (c) 2017 EPAM Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+
+#include "optee_msg.h"
+#include "optee_smc.h"
+
+/*
+ * OP-TEE violates SMCCC when it defines own UID. So we need
+ * to place bytes in correct order.


Can you please point the paragraph in the spec where it says that?


+ */
+#define OPTEE_UID  (xen_uuid_t){{  
 \
+(uint8_t)(OPTEE_MSG_UID_0 >>  0), (uint8_t)(OPTEE_MSG_UID_0 >>  8),
 \
+(uint8_t)(OPTEE_MSG_UID_0 >> 16), (uint8_t)(OPTEE_MSG_UID_0 >> 24),
 \
+(uint8_t)(OPTEE_MSG_UID_1 >>  0), (uint8_t)(OPTEE_MSG_UID_1 >>  8),
 \
+(uint8_t)(OPTEE_MSG_UID_1 >> 16), (uint8_t)(OPTEE_MSG_UID_1 >> 24),
 \
+(uint8_t)(OPTEE_MSG_UID_2 >>  0), (uint8_t)(OPTEE_MSG_UID_2 >>  8),
 \
+(uint8_t)(OPTEE_MSG_UID_2 >> 16), (uint8_t)(OPTEE_MSG_UID_2 >> 24),
 \
+(uint8_t)(OPTEE_MSG_UID_3 >>  0), (uint8_t)(OPTEE_MSG_UID_3 >>  8),
 \
+(uint8_t)(OPTEE_MSG_UID_3 >> 16), (uint8_t)(OPTEE_MSG_UID_3 >> 24),
 \
+}}
+
+static int optee_init(void)
+{
+printk("OP-TEE mediator init done\n");
+return 0;
+}
+
+static void optee_domain_create(struct domain *d)
+{
+/*
+ * Do nothing at this time.
+ * In the future this function will notify that new VM is started.


You already have a new client with the hardware domain. So don't you 
already need to notifity OP-TEE?



+ */
+}
+
+static void optee_domain_destroy(struct domain *d)
+{
+/*
+ * Do nothing at this time.
+ * In the future this function will notify that VM is being destroyed.
+ */


Same for the destruction?


+}
+
+static bool forward_call(struct cpu_user_regs *regs)
+{
+register_t resp[4];
+
+call_smccc_smc(get_user_reg(regs, 0),
+   get_user_reg(regs, 1),
+   get_user_reg(regs, 2),
+   get_user_reg(regs, 3),
+   get_user_reg(regs, 4),
+   get_user_reg(regs, 5),
+   get_user_reg(regs, 6),
+   /* VM id 0 is reserved for hypervisor itself */


s/VM/client/. Also, on your design document you mentioned that you did 
modify OP-TEE to support multiple client ID. So how do you know whether 
the TEE supports client ID?


Similarly, do you expect OP-TEE to support 16-bit of client identifier?


+   current->domain->domain_id +1,
+   resp);
+
+set_user_reg(regs, 0, resp[0]);
+set_user_reg(regs, 1, resp[1]);
+set_user_reg(regs, 2, resp[2]);
+set_user_reg(regs, 3, resp[3]);


Who will do the sanity check of the return values? 

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  765c2035a765c426c130c1f2cc009af60a99b1bd
baseline version:
 xen  46aaf41ee099a048d7a554c03ae01bcdaa07f776

Last test of basis   114460  2017-10-13 11:06:15 Z3 days
Testing same since   114544  2017-10-16 12:01:50 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=765c2035a765c426c130c1f2cc009af60a99b1bd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.
 PERLLIB=.:.
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
765c2035a765c426c130c1f2cc009af60a99b1bd
+ branch=xen-unstable-smoke
+ revision=765c2035a765c426c130c1f2cc009af60a99b1bd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.:.
 PERLLIB=.:.:.
+++ 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
+++ export PERLLIB=.:.:.:.
+++ PERLLIB=.:.:.:.
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.9-testing
+ '[' x765c2035a765c426c130c1f2cc009af60a99b1bd = 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
++ : 

Re: [Xen-devel] [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Jan Beulich]

>>> On 16.10.17 at 15:13,  wrote:
> On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
> On 13.10.17 at 07:10,  wrote:
>>> --- a/xen/arch/x86/hvm/irq.c
>>> +++ b/xen/arch/x86/hvm/irq.c
>>> @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned int 
>>> gsi)
>>>  spin_unlock(>arch.hvm_domain.irq_lock);
>>>  }
>>>  
>>> -void hvm_isa_irq_assert(
>>> -struct domain *d, unsigned int isa_irq)
>>> +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
>>> +   int (*get_vector)(const struct domain *d,
>>> + unsigned int gsi))
>>>  {
>>>  struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>>>  unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
>>> +int vector = 0;
>>
>>Why zero (which is valid aiui) instead of e.g. -1?
> 
> vector also serves as the return value. I want to return 0 if no
> callback is set.  And the callback, get_vector, can override the return
> value. Do you think it is reasonable?

Why "also" - being the return value is the only purpose of "vector".
And as said - zero is a valid vector, and I wouldn't like to see the
function return a valid but meaningless vector number.

>>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
>>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
>>> @@ -109,6 +109,11 @@ static inline int pi_test_and_set_pir(int vector, 
>>> struct pi_desc *pi_desc)
>>>  return test_and_set_bit(vector, pi_desc->pir);
>>>  }
>>>  
>>> +static inline int pi_test_pir(int vector, const struct pi_desc *pi_desc)
>>
>>This should not be a signed quantity - uint8_t or unsigned int
>>please.
> 
> Yes.

I.e. meaning you're fine with either variant, leaving it up to me
which one to use?

Jan


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

[Xen-devel] [seabios test] 114539: tolerable FAIL - PUSHED

[osstest service owner]

flight 114539 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/114539/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 114224
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 seabios  cd47172a673762a05a0c7bd27df6e3cc8febe8d6
baseline version:
 seabios  5c1a2c75951c4a59f1bf2d3c82ca7447244513ad

Last test of basis   114224  2017-10-10 01:21:52 Z6 days
Testing same since   114539  2017-10-15 22:17:10 Z0 days1 attempts


People who touched revisions under test:
  Filippo Sironi 

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-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 fail
 test-amd64-i386-xl-qemuu-ws16-amd64  fail
 test-amd64-amd64-xl-qemuu-win10-i386 fail
 test-amd64-i386-xl-qemuu-win10-i386  fail
 test-amd64-amd64-qemuu-nested-intel  pass
 test-amd64-i386-qemuu-rhel6hvm-intel 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=seabios
+ revision=cd47172a673762a05a0c7bd27df6e3cc8febe8d6
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.
 PERLLIB=.:.
+++ 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 seabios 
cd47172a673762a05a0c7bd27df6e3cc8febe8d6
+ branch=seabios
+ revision=cd47172a673762a05a0c7bd27df6e3cc8febe8d6
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
 export PERLLIB=.:.:.
 PERLLIB=.:.:.
+++ 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 

Re: [Xen-devel] [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Jan Beulich]

>>> On 16.10.17 at 16:07,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 16 October 2017 14:53
>> >>> On 12.10.17 at 18:25,  wrote:
>> > --- a/xen/common/memory.c
>> > +++ b/xen/common/memory.c
>> > @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain
>> *d, unsigned int space)
>> >  return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
>> >  }
>> >
>> > +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg)
>> > +{
>> > +struct domain *d, *currd = current->domain;
>> > +xen_mem_acquire_resource_t xmar;
>> > +unsigned long mfn_list[2];
>> > +int rc;
>> > +
>> > +if ( copy_from_guest(, arg, 1) )
>> > +return -EFAULT;
>> > +
>> > +if ( xmar.pad != 0 )
>> > +return -EINVAL;
>> > +
>> > +if ( xmar.nr_frames == 0 ||
>> > + xmar.nr_frames > ARRAY_SIZE(mfn_list) )
>> > +{
>> > +rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG;
>> 
>> Querying the implementation limit should be possible without
>> receiving an error. Hence my original suggestion to key this
>> off of the handle being a null one (in which case non-zero
>> nr_frames would indeed be -EINVAL), which afaics would also
>> simplify some of the compat handling.
> 
> Ok, FAOD, do you mean that passing in nr_frames and a NULL handle should not 
> yield an error but should pass back the implementation limit of nr_frames? 

If you mean "passing in zero nr_frames and a null handle", then
yes. Non-zero nr_frames and a null handle, as said, could certainly
be -EINVAL (you could as well try to read/write from/to that handle,
but I think Andrew wouldn't like that).

>> > +{
>> > +rc = set_foreign_p2m_entry(currd, gfn_list[i],
>> > +   _mfn(mfn_list[i]));
>> > +if ( rc )
>> > +{
>> > +while ( i-- != 0 )
>> > +{
>> > +int ignore;
>> > +
>> > +ignore = guest_physmap_remove_page(
>> > +currd, _gfn(gfn_list[i]), _mfn(mfn_list[i]), 0);
>> 
>> Why would an error here be plain ignored?
>> 
> 
> What could I usefully do with it? Should I just crash the domain at this 
> point, since I can't restore a consistent state?

Not being silent is the most important aspect. Crashing the domain
is one approach. Reporting the error (and documenting the possibly
resulting inconsistent state) is another (and a sub-option thereof is
to return the number of failed entries, perhaps allowing the caller
some way to recover). Plus note that if the error happens on the
first iteration, no inconsistency would result.

>> > @@ -1406,6 +1488,14 @@ long do_memory_op(unsigned long cmd,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>> >  }
>> >  #endif
>> >
>> > +case XENMEM_acquire_resource:
>> > +#ifdef CONFIG_X86
>> > +rc = acquire_resource(arg);
>> > +#else
>> > +rc = -EOPNOTSUPP;
>> > +#endif
>> 
>> I think this will cause an "unused static function" warning on ARM.
> 
> ...which is why I originally had the function wrapped in the #ifdef as well. 
> What do you want me to do?

As said before - I'd like to see the #ifdef placed inside the function
around the smallest possible range of code that still allows ARM to
build (with the alternative of introducing a dummy stub or two on
ARM to avoid #ifdef-s altogether).

>> > --- a/xen/include/public/memory.h
>> > +++ b/xen/include/public/memory.h
>> > @@ -599,6 +599,47 @@ struct xen_reserved_device_memory_map {
>> >  typedef struct xen_reserved_device_memory_map
>> xen_reserved_device_memory_map_t;
>> >  DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
>> >
>> > +/*
>> > + * Get the pages for a particular guest resource, so that they can be
>> > + * mapped directly by a tools domain.
>> > + */
>> > +#define XENMEM_acquire_resource 28
>> > +struct xen_mem_acquire_resource {
>> > +/* IN - the domain whose resource is to be mapped */
>> > +domid_t domid;
>> > +/* IN - the type of resource */
>> > +uint16_t type;
>> > +/*
>> > + * IN - a type-specific resource identifier, which must be zero
>> > + *  unless stated otherwise.
>> > + */
>> > +uint32_t id;
>> > +/* IN/OUT - As an IN parameter number of (4K) frames of the resource
>> 
>> Please don't say 4k here - this not being an x86-specific interface
>> other system page sizes ought to be permitted.
> 
> I was under the impression that resources such as grant tables were only 
> every mapped in 4k chunks. Perhaps the 4k should type-specific? It needs to 
> be 
> specified somewhere.

I don't think there's an inherent limit to granted pages being larger
than 4k; v2 sub-page grants limit things to less than 64k though.
There's no single mention of "4k" throughout the public grant_table.h
or the implementation in grant_table.c.

>> > + *  to be mapped. However, if the specified value is 0 then
>> 

Re: [Xen-devel] [PATCH v11 06/11] x86/hvm/ioreq: add a new mappable resource type...

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 16 October 2017 15:07
> To: Paul Durrant 
> Cc: Andrew Cooper ; Ian Jackson
> ; Stefano Stabellini ; xen-
> de...@lists.xenproject.org; Konrad Rzeszutek Wilk
> ; Tim (Xen.org) 
> Subject: Re: [Xen-devel] [PATCH v11 06/11] x86/hvm/ioreq: add a new
> mappable resource type...
> 
> >>> On 12.10.17 at 18:25,  wrote:
> > ... XENMEM_resource_ioreq_server
> >
> > This patch adds support for a new resource type that can be mapped using
> > the XENMEM_acquire_resource memory op.
> >
> > If an emulator makes use of this resource type then, instead of mapping
> > gfns, the IOREQ server will allocate pages from the heap. These pages
> > will never be present in the P2M of the guest at any point and so are
> > not vulnerable to any direct attack by the guest. They are only ever
> > accessible by Xen and any domain that has mapping privilege over the
> > guest (which may or may not be limited to the domain running the
> emulator).
> >
> > NOTE: Use of the new resource type is not compatible with use of
> >   XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns
> flag is
> >   set.
> >
> > Signed-off-by: Paul Durrant 
> > Acked-by: George Dunlap 
> > Reviewed-by: Wei Liu 
> 
> Can you have validly retained this?

I didn't think the structure of this particular patch had changed that 
fundamentally.

> 
> > --- a/xen/arch/x86/hvm/ioreq.c
> > +++ b/xen/arch/x86/hvm/ioreq.c
> > @@ -281,6 +294,69 @@ static int hvm_map_ioreq_gfn(struct
> hvm_ioreq_server *s, bool buf)
> >  return rc;
> >  }
> >
> > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > +{
> > +struct domain *currd = current->domain;
> > +struct hvm_ioreq_page *iorp = buf ? >bufioreq : >ioreq;
> > +
> > +if ( iorp->page )
> > +{
> > +/*
> > + * If a guest frame has already been mapped (which may happen
> > + * on demand if hvm_get_ioreq_server_info() is called), then
> > + * allocating a page is not permitted.
> > + */
> > +if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> > +return -EPERM;
> > +
> > +return 0;
> > +}
> > +
> > +/*
> > + * Allocated IOREQ server pages are assigned to the emulating
> > + * domain, not the target domain. This is because the emulator is
> > + * likely to be destroyed after the target domain has been torn
> > + * down, and we must use MEMF_no_refcount otherwise page
> allocation
> > + * could fail if the emulating domain has already reached its
> > + * maximum allocation.
> > + */
> > +iorp->page = alloc_domheap_page(currd, MEMF_no_refcount);
> > +if ( !iorp->page )
> > +return -ENOMEM;
> > +
> > +if ( !get_page_type(iorp->page, PGT_writable_page) )
> > +{
> 
> ASSERT_UNREACHABLE() ?

Ok.

> 
> > @@ -777,6 +886,51 @@ int hvm_get_ioreq_server_info(struct domain *d,
> ioservid_t id,
> >  return rc;
> >  }
> >
> > +int hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> > +   unsigned long idx, mfn_t *mfn)
> > +{
> > +struct hvm_ioreq_server *s;
> > +int rc;
> > +
> > +spin_lock_recursive(>arch.hvm_domain.ioreq_server.lock);
> > +
> > +if ( id == DEFAULT_IOSERVID )
> > +return -EOPNOTSUPP;
> > +
> > +s = get_ioreq_server(d, id);
> > +
> > +ASSERT(!IS_DEFAULT(s));
> > +
> > +rc = hvm_ioreq_server_alloc_pages(s);
> > +if ( rc )
> > +goto out;
> > +
> > +switch ( idx )
> > +{
> > +case XENMEM_resource_ioreq_server_frame_bufioreq:
> > +rc = -ENOENT;
> > +if ( !HANDLE_BUFIOREQ(s) )
> > +goto out;
> > +
> > +*mfn = _mfn(page_to_mfn(s->bufioreq.page));
> > +rc = 0;
> > +break;
> 
> How about
> 
> if ( HANDLE_BUFIOREQ(s) )
> *mfn = _mfn(page_to_mfn(s->bufioreq.page));
> else
> rc = -ENOENT;
> break;
> 

Looking at the overall structure I prefer it as it is. If I could have got rid 
of the out label by doing this then it might have been worth the change.

> ?
> 
> > +int xenmem_acquire_ioreq_server(struct domain *d, unsigned int id,
> > +unsigned long frame,
> > +unsigned long nr_frames,
> > +unsigned long mfn_list[])
> > +{
> > +unsigned int i;
> 
> This now doesn't match up with the upper bound's type.
> 

Ok.

> > @@ -629,6 +634,10 @@ struct xen_mem_acquire_resource {
> >   *  is optional if nr_frames is 0.
> >   */
> >  uint64_aligned_t frame;
> > +
> > +#define XENMEM_resource_ioreq_server_frame_bufioreq 0
> > +#define 

Re: [Xen-devel] [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Chao Gao]

On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
 On 13.10.17 at 07:10,  wrote:
>> --- a/xen/arch/x86/hvm/irq.c
>> +++ b/xen/arch/x86/hvm/irq.c
>> @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned int 
>> gsi)
>>  spin_unlock(>arch.hvm_domain.irq_lock);
>>  }
>>  
>> -void hvm_isa_irq_assert(
>> -struct domain *d, unsigned int isa_irq)
>> +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
>> +   int (*get_vector)(const struct domain *d,
>> + unsigned int gsi))
>>  {
>>  struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>>  unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
>> +int vector = 0;
>
>Why zero (which is valid aiui) instead of e.g. -1?

vector also serves as the return value. I want to return 0 if no
callback is set.  And the callback, get_vector, can override the return
value. Do you think it is reasonable?

>
>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
>> @@ -109,6 +109,11 @@ static inline int pi_test_and_set_pir(int vector, 
>> struct pi_desc *pi_desc)
>>  return test_and_set_bit(vector, pi_desc->pir);
>>  }
>>  
>> +static inline int pi_test_pir(int vector, const struct pi_desc *pi_desc)
>
>This should not be a signed quantity - uint8_t or unsigned int
>please.

Yes.

>
>I wouldn't mind making suitable adjustments while committing (and
>then adding my R-b), but that requires your feedback which way
>things should be.

Sure. I will appreciate it.
>
>Also please don't forget to Cc the release manager, unless you
>intend this fix only for after 4.10.

Hi, Julien.

This patch is to fix a possible cause of an assertion failure related to
periodic timer interrupt. OSSTEST reports regression occasionally when the bug
happens. I intend to merge this patch at first and then observe whether
the bug disappears or not. Since Jan said he could do some adjustments to the
patch when committing, Could you give acked-by on this patch?

Thanks
Chao.

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

Re: [Xen-devel] [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 16 October 2017 14:53
> To: Paul Durrant 
> Cc: Andrew Cooper ; Wei Liu
> ; George Dunlap ; Ian
> Jackson ; Stefano Stabellini
> ; xen-de...@lists.xenproject.org; Konrad Rzeszutek
> Wilk ; Tim (Xen.org) 
> Subject: Re: [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to
> acquire guest resources
> 
> >>> On 12.10.17 at 18:25,  wrote:
> > @@ -402,14 +469,56 @@ int compat_memory_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) compat)
> >  rc = do_memory_op(cmd, nat.hnd);
> >  if ( rc < 0 )
> >  {
> > -if ( rc == -ENOBUFS && op == XENMEM_get_vnumainfo )
> > +switch ( op)
> 
> Missing blank.

Oh yes.

> 
> >  {
> > -cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes;
> > -cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus;
> > -cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges;
> > -if ( __copy_to_guest(compat, , 1) )
> > -rc = -EFAULT;
> > +case XENMEM_get_vnumainfo:
> > +if ( rc == -ENOBUFS )
> > +{
> > +cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes;
> > +cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus;
> > +cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges;
> > +if ( __copy_to_guest(compat, , 1) )
> > +rc = -EFAULT;
> > +}
> > +
> > +break;
> > +
> > +case XENMEM_acquire_resource:
> > +{
> > +xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1);
> 
> const

Ok.

> 
> > +if ( rc == -EINVAL && xen_frame_list[0] != 0 )
> 
> I think this will go wrong if you get -EINVAL for other than the
> specific reason you consider here, in particular when caller
> passed in a valid array. You'd need to also check for
> cmp.mar.nr_frames being zero. But see also below.
> 
> > +{
> > +/*
> > + * The value of nr_frames passed to the implementation
> > + * was not the value passed by the caller, it was
> > + * overridden.
> > + * The value in xen_frame_list[0] is the maximum
> > + * number of frames that can be bounced so we need
> > + * to set cmp.nr_frames to the minimum of this and
> > + * the maximum number of frames allowed by the
> > + * implementation before passing back to the caller.
> > + */
> > +cmp.mar.nr_frames = min_t(unsigned int,
> > +  xen_frame_list[0],
> > +  nat.mar->nr_frames);
> > +rc = -E2BIG;
> > +}
> > +
> > +/* In either of these cases nr_frames is an OUT value */
> > +if ( rc == -EINVAL || rc == -E2BIG )
> > +{
> > +if ( copy_to_guest(compat, , 1) )
> > +rc = -EFAULT;
> 
> The two if()s should be combined. Also - __copy_field_to_guest()?

Yes, maybe that would neater.

> 
> > +}
> > +
> > +break;
> > +}
> > +default:
> > +break;
> 
> No real need for a default label. Yet if you want to keep it, please
> have a blank line ahead of it.
> 

Ok.

> > @@ -535,6 +644,30 @@ int compat_memory_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) compat)
> >  rc = -EFAULT;
> >  break;
> >
> > +case XENMEM_acquire_resource:
> > +{
> > +xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1);
> 
> const

Ok.

> 
> > +compat_ulong_t *compat_frame_list =
> > +(compat_ulong_t *)(nat.mar + 1);
> > +
> > +/* NOTE: the compat array overwrites the native array */
> 
> Perhaps "the smaller compat array ..."?

Ok.

> 
> > +for ( i = 0; i < cmp.mar.nr_frames; i++ )
> > +{
> > +compat_ulong_t frame = xen_frame_list[i];
> > +
> > +if ( frame != xen_frame_list[i] )
> > +return -ERANGE;
> > +
> > +compat_frame_list[i] = frame;
> > +}
> > +
> > +if ( __copy_to_compat_offset(cmp.mar.frame_list, 0,
> > + compat_frame_list,
> > + cmp.mar.nr_frames) )
> > +return -EFAULT;
> > +
> > +break;
> > +}
> >  default:
> 
> Again missing a blank 

Re: [Xen-devel] [PATCH v11 06/11] x86/hvm/ioreq: add a new mappable resource type...

[Jan Beulich]

>>> On 12.10.17 at 18:25,  wrote:
> ... XENMEM_resource_ioreq_server
> 
> This patch adds support for a new resource type that can be mapped using
> the XENMEM_acquire_resource memory op.
> 
> If an emulator makes use of this resource type then, instead of mapping
> gfns, the IOREQ server will allocate pages from the heap. These pages
> will never be present in the P2M of the guest at any point and so are
> not vulnerable to any direct attack by the guest. They are only ever
> accessible by Xen and any domain that has mapping privilege over the
> guest (which may or may not be limited to the domain running the emulator).
> 
> NOTE: Use of the new resource type is not compatible with use of
>   XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
>   set.
> 
> Signed-off-by: Paul Durrant 
> Acked-by: George Dunlap 
> Reviewed-by: Wei Liu 

Can you have validly retained this?

> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -281,6 +294,69 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, 
> bool buf)
>  return rc;
>  }
>  
> +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> +{
> +struct domain *currd = current->domain;
> +struct hvm_ioreq_page *iorp = buf ? >bufioreq : >ioreq;
> +
> +if ( iorp->page )
> +{
> +/*
> + * If a guest frame has already been mapped (which may happen
> + * on demand if hvm_get_ioreq_server_info() is called), then
> + * allocating a page is not permitted.
> + */
> +if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> +return -EPERM;
> +
> +return 0;
> +}
> +
> +/*
> + * Allocated IOREQ server pages are assigned to the emulating
> + * domain, not the target domain. This is because the emulator is
> + * likely to be destroyed after the target domain has been torn
> + * down, and we must use MEMF_no_refcount otherwise page allocation
> + * could fail if the emulating domain has already reached its
> + * maximum allocation.
> + */
> +iorp->page = alloc_domheap_page(currd, MEMF_no_refcount);
> +if ( !iorp->page )
> +return -ENOMEM;
> +
> +if ( !get_page_type(iorp->page, PGT_writable_page) )
> +{

ASSERT_UNREACHABLE() ?

> @@ -777,6 +886,51 @@ int hvm_get_ioreq_server_info(struct domain *d, 
> ioservid_t id,
>  return rc;
>  }
>  
> +int hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> +   unsigned long idx, mfn_t *mfn)
> +{
> +struct hvm_ioreq_server *s;
> +int rc;
> +
> +spin_lock_recursive(>arch.hvm_domain.ioreq_server.lock);
> +
> +if ( id == DEFAULT_IOSERVID )
> +return -EOPNOTSUPP;
> +
> +s = get_ioreq_server(d, id);
> +
> +ASSERT(!IS_DEFAULT(s));
> +
> +rc = hvm_ioreq_server_alloc_pages(s);
> +if ( rc )
> +goto out;
> +
> +switch ( idx )
> +{
> +case XENMEM_resource_ioreq_server_frame_bufioreq:
> +rc = -ENOENT;
> +if ( !HANDLE_BUFIOREQ(s) )
> +goto out;
> +
> +*mfn = _mfn(page_to_mfn(s->bufioreq.page));
> +rc = 0;
> +break;

How about

if ( HANDLE_BUFIOREQ(s) )
*mfn = _mfn(page_to_mfn(s->bufioreq.page));
else
rc = -ENOENT;
break;

?

> +int xenmem_acquire_ioreq_server(struct domain *d, unsigned int id,
> +unsigned long frame,
> +unsigned long nr_frames,
> +unsigned long mfn_list[])
> +{
> +unsigned int i;

This now doesn't match up with the upper bound's type.

> @@ -629,6 +634,10 @@ struct xen_mem_acquire_resource {
>   *  is optional if nr_frames is 0.
>   */
>  uint64_aligned_t frame;
> +
> +#define XENMEM_resource_ioreq_server_frame_bufioreq 0
> +#define XENMEM_resource_ioreq_server_frame_ioreq(n_) (1 + (n_))

I don't see what you need the trailing underscore for. This is
normally only needed on local variables defined in (gcc extended)
macros, which we generally can't use in a public header anyway.

Jan

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

[Xen-devel] [PATCH for-4.10] libxl: annotate s to be nonnull in libxl__enum_from_string

[Wei Liu]

Hope this can placate coverity.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Julien Grall 
---
 tools/libxl/libxl_internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 45e6df6c82..9fe472efe3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1711,7 +1711,7 @@ _hidden char *libxl__domid_to_name(libxl__gc *gc, 
uint32_t domid);
 _hidden char *libxl__cpupoolid_to_name(libxl__gc *gc, uint32_t poolid);
 
 _hidden int libxl__enum_from_string(const libxl_enum_string_table *t,
-const char *s, int *e);
+const char *s, int *e) 
__attribute__((nonnull(2)));
 
 _hidden yajl_gen_status libxl__yajl_gen_asciiz(yajl_gen hand, const char *str);
 
-- 
2.11.0


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

Re: [Xen-devel] [RFC 3/4] arm: tee: add OP-TEE header files

[Julien Grall]

Hi Volodymyr,

On 11/10/17 20:01, Volodymyr Babchuk wrote:

This header files describe protocol between OP-TEE and OP-TEE client
driver in Linux. They are needed for upcomient OP-TEE mediator, which
is added in the next patch.
Reason to add those headers in separate patch is to ease up review.
Those files were taken from linux tree (drivers/tee/optee/) and mangled
a bit to compile with XEN.

Signed-off-by: Volodymyr Babchuk 
---
  xen/arch/arm/tee/optee_msg.h | 444 +
  xen/arch/arm/tee/optee_smc.h | 457 +++


Headers should go in include/asm-arm/tee and not arch/arm.

Cheers,


  2 files changed, 901 insertions(+)
  create mode 100644 xen/arch/arm/tee/optee_msg.h
  create mode 100644 xen/arch/arm/tee/optee_smc.h

diff --git a/xen/arch/arm/tee/optee_msg.h b/xen/arch/arm/tee/optee_msg.h
new file mode 100644
index 000..10747b2
--- /dev/null
+++ b/xen/arch/arm/tee/optee_msg.h
@@ -0,0 +1,444 @@
+/*
+ * Copyright (c) 2015-2016, Linaro Limited
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _OPTEE_MSG_H
+#define _OPTEE_MSG_H
+
+#include 
+#include 
+
+/*
+ * This file defines the OP-TEE message protocol used to communicate
+ * with an instance of OP-TEE running in secure world.
+ *
+ * This file is divided into three sections.
+ * 1. Formatting of messages.
+ * 2. Requests from normal world
+ * 3. Requests from secure world, Remote Procedure Call (RPC), handled by
+ *tee-supplicant.
+ */
+
+/*
+ * Part 1 - formatting of messages
+ */
+
+#define OPTEE_MSG_ATTR_TYPE_NONE   0x0
+#define OPTEE_MSG_ATTR_TYPE_VALUE_INPUT0x1
+#define OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT   0x2
+#define OPTEE_MSG_ATTR_TYPE_VALUE_INOUT0x3
+#define OPTEE_MSG_ATTR_TYPE_RMEM_INPUT 0x5
+#define OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT0x6
+#define OPTEE_MSG_ATTR_TYPE_RMEM_INOUT 0x7
+#define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT 0x9
+#define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT0xa
+#define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT 0xb
+
+#define OPTEE_MSG_ATTR_TYPE_MASK   GENMASK(7, 0)
+
+/*
+ * Meta parameter to be absorbed by the Secure OS and not passed
+ * to the Trusted Application.
+ *
+ * Currently only used with OPTEE_MSG_CMD_OPEN_SESSION.
+ */
+#define OPTEE_MSG_ATTR_METABIT(8)
+
+/*
+ * Pointer to a list of pages used to register user-defined SHM buffer.
+ * Used with OPTEE_MSG_ATTR_TYPE_TMEM_*.
+ * buf_ptr should point to the beginning of the buffer. Buffer will contain
+ * list of page addresses. OP-TEE core can reconstruct contiguous buffer from
+ * that page addresses list. Page addresses are stored as 64 bit values.
+ * Last entry on a page should point to the next page of buffer.
+ * Every entry in buffer should point to a 4k page beginning (12 least
+ * significant bits must be equal to zero).
+ *
+ * 12 least significant bints of optee_msg_param.u.tmem.buf_ptr should hold 
page
+ * offset of the user buffer.
+ *
+ * So, entries should be placed like members of this structure:
+ *
+ * struct page_data {
+ *   uint64_t pages_array[OPTEE_MSG_NONCONTIG_PAGE_SIZE/sizeof(uint64_t) - 1];
+ *   uint64_t next_page_data;
+ * };
+ *
+ * Structure is designed to exactly fit into the page size
+ * OPTEE_MSG_NONCONTIG_PAGE_SIZE which is a standard 4KB page.
+ *
+ * The size of 4KB is chosen because this is the smallest page size for ARM
+ * architectures. If REE uses larger pages, it 

Re: [Xen-devel] [PATCH v3 2/6] x86/msr: add VMX MSRs into struct msr_domain_policy

[Andrew Cooper]

On 16/10/17 08:42, Sergey Dyasli wrote:
> On Fri, 2017-10-13 at 16:16 +0100, Andrew Cooper wrote:
>> On 13/10/17 13:35, Sergey Dyasli wrote:
>>> @@ -210,6 +375,255 @@ struct msr_domain_policy
>>>  bool available; /* This MSR is non-architectural */
>>>  bool cpuid_faulting;
>>>  } plaform_info;
>>> +
>>> +/* 0x0480  MSR_IA32_VMX_BASIC */
>>> +struct {
>>> +bool available;
>> We don't need available bits for any of these MSRs.  Their availability
>> is cpuid->basic.vmx, and we don't want (let alone need) to duplicate
>> information like this.
> Andrew,
>
> What do you think about the following way of checking the availability?

Preferably not.  You are duplicating a lot of information already
available in the guest_{rd,wr}msr(), and visually separating the
availability check from the data returned.  Worst however, is that you
risk having a mismatch between the MSR ranges which fall into this
check, and those which are calculated by it.

>
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 2527fdd1d1..828f1bb503 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -33,6 +33,43 @@ struct msr_domain_policy __read_mostly 
> raw_msr_domain_policy,
>  struct msr_vcpu_policy __read_mostly hvm_max_msr_vcpu_policy,
> __read_mostly  pv_max_msr_vcpu_policy;
>  
> +bool msr_vmx_available(const struct domain *d, uint32_t msr)
> +{
> +const struct msr_domain_policy *dp = d->arch.msr;
> +bool secondary_available;
> +
> +if ( !nestedhvm_enabled(d) || !d->arch.cpuid->basic.vmx )
> +return false;

For now, we do need to double up the d->arch.cpuid->basic.vmx with
nestedhvm_enabled(d), but rest assured that nestedhvm_enabled(d) will be
disappearing in due course.  (It exists only because we don't have fine
grain toolstack control of CPUID/MSR values yet).

> +
> +secondary_available =
> +dp->vmx_procbased_ctls.u.allowed_1.activate_secondary_controls;
> +
> +switch (msr)
> +{
> +case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMCS_ENUM:
> +return true;
> +
> +case MSR_IA32_VMX_PROCBASED_CTLS2:
> +return secondary_available;
> +
> +case MSR_IA32_VMX_EPT_VPID_CAP:
> +return ( secondary_available &&
> + (dp->vmx_procbased_ctls2.u.allowed_1.enable_ept ||
> +  dp->vmx_procbased_ctls2.u.allowed_1.enable_vpid) );

This check can be made more efficient in two ways.  First, use a bitwise
rather than logical or, which allows both _ept and _vpid to be tested
with a single instruction, rather than a conditional branch.

Secondly, the CPUID infrastructure has logic to flatten dependency
trees, so we don't need to encode logic paths like this.  In practice
however, you only read into the policy for details which match the
dependency tree, so you can drop the secondary_available check here, as
you know that if secondary_available is clear,
dp->vmx_procbased_ctls2.raw will be 0.

~Andrew

> +
> +case MSR_IA32_VMX_TRUE_PINBASED_CTLS ... MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> +return dp->vmx_basic.u.default1_zero;
> +
> +case MSR_IA32_VMX_VMFUNC:
> +return ( secondary_available &&
> + dp->vmx_procbased_ctls2.u.allowed_1.enable_vm_functions );
> +
> +default: break;
> +}
> +
> +return false;
> +}
> +
>  static void __init calculate_raw_vmx_policy(struct msr_domain_policy *dp)
>  {
>  if ( !cpu_has_vmx )
>


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

Re: [Xen-devel] [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Jan Beulich]

>>> On 12.10.17 at 18:25,  wrote:
> @@ -402,14 +469,56 @@ int compat_memory_op(unsigned int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) compat)
>  rc = do_memory_op(cmd, nat.hnd);
>  if ( rc < 0 )
>  {
> -if ( rc == -ENOBUFS && op == XENMEM_get_vnumainfo )
> +switch ( op)

Missing blank.

>  {
> -cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes;
> -cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus;
> -cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges;
> -if ( __copy_to_guest(compat, , 1) )
> -rc = -EFAULT;
> +case XENMEM_get_vnumainfo:
> +if ( rc == -ENOBUFS )
> +{
> +cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes;
> +cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus;
> +cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges;
> +if ( __copy_to_guest(compat, , 1) )
> +rc = -EFAULT;
> +}
> +
> +break;
> +
> +case XENMEM_acquire_resource:
> +{
> +xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1);

const

> +if ( rc == -EINVAL && xen_frame_list[0] != 0 )

I think this will go wrong if you get -EINVAL for other than the
specific reason you consider here, in particular when caller
passed in a valid array. You'd need to also check for
cmp.mar.nr_frames being zero. But see also below.

> +{
> +/*
> + * The value of nr_frames passed to the implementation
> + * was not the value passed by the caller, it was
> + * overridden.
> + * The value in xen_frame_list[0] is the maximum
> + * number of frames that can be bounced so we need
> + * to set cmp.nr_frames to the minimum of this and
> + * the maximum number of frames allowed by the
> + * implementation before passing back to the caller.
> + */
> +cmp.mar.nr_frames = min_t(unsigned int,
> +  xen_frame_list[0],
> +  nat.mar->nr_frames);
> +rc = -E2BIG;
> +}
> +
> +/* In either of these cases nr_frames is an OUT value */
> +if ( rc == -EINVAL || rc == -E2BIG )
> +{
> +if ( copy_to_guest(compat, , 1) )
> +rc = -EFAULT;

The two if()s should be combined. Also - __copy_field_to_guest()?

> +}
> +
> +break;
> +}
> +default:
> +break;

No real need for a default label. Yet if you want to keep it, please
have a blank line ahead of it.

> @@ -535,6 +644,30 @@ int compat_memory_op(unsigned int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) compat)
>  rc = -EFAULT;
>  break;
>  
> +case XENMEM_acquire_resource:
> +{
> +xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1);

const

> +compat_ulong_t *compat_frame_list =
> +(compat_ulong_t *)(nat.mar + 1);
> +
> +/* NOTE: the compat array overwrites the native array */

Perhaps "the smaller compat array ..."?

> +for ( i = 0; i < cmp.mar.nr_frames; i++ )
> +{
> +compat_ulong_t frame = xen_frame_list[i];
> +
> +if ( frame != xen_frame_list[i] )
> +return -ERANGE;
> +
> +compat_frame_list[i] = frame;
> +}
> +
> +if ( __copy_to_compat_offset(cmp.mar.frame_list, 0,
> + compat_frame_list,
> + cmp.mar.nr_frames) )
> +return -EFAULT;
> +
> +break;
> +}
>  default:

Again missing a blank line above here.

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain *d, 
> unsigned int space)
>  return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
>  }
>  
> +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +struct domain *d, *currd = current->domain;
> +xen_mem_acquire_resource_t xmar;
> +unsigned long mfn_list[2];
> +int rc;
> +
> +if ( copy_from_guest(, arg, 1) )
> +return -EFAULT;
> +
> +if ( xmar.pad != 0 )
> +return -EINVAL;
> +
> +if ( xmar.nr_frames == 0 ||
> + xmar.nr_frames > ARRAY_SIZE(mfn_list) )
> +{
> +rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG;

Querying the implementation limit should be possible without
receiving an error. Hence my original suggestion to 

Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Andrew Cooper]

On 16/10/17 14:40, Wei Liu wrote:
> On Fri, Oct 13, 2017 at 06:32:18PM +0100, Andrew Cooper wrote:
>> c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
>> justify passing precopy_stats by value.
>>
>> Under no circumstances can the precopy callback ever be executing in a
>> separate address space.
>>
> The callback is not executed in a separate address space. 
>
> Have you checked
> <1506365735-133776-4-git-send-email-jennifer.herb...@citrix.com>?
>
> The open source toolstack spawns another process to save vm image. In
> order to let libxl control the process (in the future) there is
> information passed across process boundary.
>
> Your code might work for now because Joshua's patch is not yet applied.

I'm perfectly aware of that discussion, and it is factually incorrect. 
Nothing, not even Joshua's patch, can cause the callback to be executed
in a separate address space.

With Joshua's patch in place, the implementer of this callback is the
code generated by libxl_save_msgs_gen.pl, which is the aformentioned
extra process.  Passing by pointer or value has nothing to do with the
fact that the automatically generated code needs to know how to
serialise/deserialise the data to feed it back to the main process.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Wei Liu]

On Fri, Oct 13, 2017 at 06:32:18PM +0100, Andrew Cooper wrote:
> c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
> justify passing precopy_stats by value.
> 
> Under no circumstances can the precopy callback ever be executing in a
> separate address space.
> 

The callback is not executed in a separate address space. 

Have you checked
<1506365735-133776-4-git-send-email-jennifer.herb...@citrix.com>?

The open source toolstack spawns another process to save vm image. In
order to let libxl control the process (in the future) there is
information passed across process boundary.

Your code might work for now because Joshua's patch is not yet applied.

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

[Xen-devel] [xen-4.8-testing test] 114538: tolerable FAIL - PUSHED

[osstest service owner]

flight 114538 xen-4.8-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/114538/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-xtf-amd64-amd64-2 48 xtf/test-hvm64-lbr-tsx-vmentry fail in 114505 pass 
in 114538
 test-armhf-armhf-xl-rtds 12 guest-start  fail in 114505 pass in 114538
 test-xtf-amd64-amd64-5   48 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 114505

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 114173
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 114173
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 114173
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 114173
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 114173
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 build-i386-prev   7 xen-build/dist-test  fail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 build-amd64-prev  7 xen-build/dist-test  fail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  bdc2ae68e2ecba1c3f55ad953189fe33362d1c51
baseline version:
 xen  667f70e658c4c382672056ebaf1505b4c5cdb0aa

Last test of basis   114173  2017-10-09 03:27:38 Z7 days
Failing since114313  2017-10-11 00:46:14 Z5 days5 attempts
Testing same since   114454  2017-10-13 06:48:53 Z3 days3 attempts


People who touched revisions under test:
  Andrew Cooper 
  George Dunlap 
  Jan Beulich 
  Julien Grall 
  Stefano Stabellini 
  Tim Deegan 
  Vitaly Kuznetsov 

jobs:
 build-amd64-xsm   

[Xen-devel] [PATCH] xen-netfront, xen-netback: Use correct minimum MTU values

[Mohammed Gamal]

RFC791 specifies the minimum MTU to be 68, while xen-net{front|back}
drivers use a minimum value of 0.

When set MTU to 0~67 with xen_net{front|back} driver, the network
will become unreachable immediately, the guest can no longer be pinged.

xen_net{front|back} should not allow the user to set this value which causes
network problems.

Reported-by: Chen Shi 
Signed-off-by: Mohammed Gamal 
---
 drivers/net/xen-netback/interface.c | 2 +-
 drivers/net/xen-netfront.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index ee8ed9da..4491ca5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -486,7 +486,7 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t 
domid,
 
dev->tx_queue_len = XENVIF_QUEUE_LENGTH;
 
-   dev->min_mtu = 0;
+   dev->min_mtu = ETH_MIN_MTU;
dev->max_mtu = ETH_MAX_MTU - VLAN_ETH_HLEN;
 
/*
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 523387e..8b8689c 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1316,7 +1316,7 @@ static struct net_device *xennet_create_dev(struct 
xenbus_device *dev)
netdev->features |= netdev->hw_features;
 
netdev->ethtool_ops = _ethtool_ops;
-   netdev->min_mtu = 0;
+   netdev->min_mtu = ETH_MIN_MTU;
netdev->max_mtu = XEN_NETIF_MAX_TX_SIZE;
SET_NETDEV_DEV(netdev, >dev);
 
-- 
1.8.3.1


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

Re: [Xen-devel] [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Jan Beulich]

>>> On 13.10.17 at 07:10,  wrote:
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned int 
> gsi)
>  spin_unlock(>arch.hvm_domain.irq_lock);
>  }
>  
> -void hvm_isa_irq_assert(
> -struct domain *d, unsigned int isa_irq)
> +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
> +   int (*get_vector)(const struct domain *d,
> + unsigned int gsi))
>  {
>  struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>  unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
> +int vector = 0;

Why zero (which is valid aiui) instead of e.g. -1?

> @@ -292,25 +292,38 @@ int pt_update_irq(struct vcpu *v)
>  
>  spin_unlock(>arch.hvm_vcpu.tm_lock);
>  
> +/*
> + * If periodic timer interrut is handled by lapic, its vector in
> + * IRR is returned and used to set eoi_exit_bitmap for virtual
> + * interrupt delivery case. Otherwise return -1 to do nothing.
> + */
>  if ( is_lapic )
> +{
>  vlapic_set_irq(vcpu_vlapic(v), irq, 0);
> +pt_vector = irq;
> +}
>  else
>  {
>  hvm_isa_irq_deassert(v->domain, irq);
> -hvm_isa_irq_assert(v->domain, irq);
> +if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
> + v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
> +{
> +hvm_isa_irq_assert(v->domain, irq, NULL);
> +pt_vector = -1;
> +}
> +else
> +{
> +pt_vector = hvm_isa_irq_assert(v->domain, irq, 
> vioapic_get_vector);

I like that you got away without introducing a new wrapper function
at all.

> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -109,6 +109,11 @@ static inline int pi_test_and_set_pir(int vector, struct 
> pi_desc *pi_desc)
>  return test_and_set_bit(vector, pi_desc->pir);
>  }
>  
> +static inline int pi_test_pir(int vector, const struct pi_desc *pi_desc)

This should not be a signed quantity - uint8_t or unsigned int
please.

I wouldn't mind making suitable adjustments while committing (and
then adding my R-b), but that requires your feedback which way
things should be.

Also please don't forget to Cc the release manager, unless you
intend this fix only for after 4.10.

Jan


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

Re: [Xen-devel] [RFC 2/4] arm: add generic TEE mediator framework

[Julien Grall]

Hi Volodymyr,

On 11/10/17 20:01, Volodymyr Babchuk wrote:

This patch adds basic framework for TEE mediators. Guests can't talk
to TEE directly, we need some entity that will intercept request
and decide what to do with them. "TEE mediaor" is a such entity.


s/mediaor/mediator/



This is how it works: user can build XEN with multiple TEE mediators
(see the next patches, where OP-TEE mediator is introduced).
TEE mediator register self with REGISTER_TEE_MEDIATOR() macro in the
same way, as device drivers use DT_DEVICE_START()/DT_DEVICE_END()
macros.
In runtime, during initialization, XEN issues standard SMC to read
TEE UID. Using this UID it selects and initializes one of built-in
mediators. Then generic vSMC handler will call selected mediator
when it intercept SMC that belongs to TEE OS or TEE application.


As you may remember the discussion on the SMCCC support for guests, 
there are currently no way to know the SMCCC is present on the platform.


I don't think you can rely on the platform support SMCC nor fully 
implementing it. This also bring the question of does every TEE are 
supporting SMCCC?




Also, there are hooks for domain construction and destruction, so
TEE mediator can inform TEE about VM lifecycle.

Signed-off-by: Volodymyr Babchuk 
---
  MAINTAINERS   |   5 ++
  xen/arch/arm/Kconfig  |  10 
  xen/arch/arm/Makefile |   1 +
  xen/arch/arm/domain.c |   7 +++
  xen/arch/arm/setup.c  |   4 ++
  xen/arch/arm/tee/Kconfig  |   0
  xen/arch/arm/tee/Makefile |   1 +
  xen/arch/arm/tee/tee.c| 134 ++
  xen/arch/arm/vsmc.c   |   5 ++
  xen/arch/arm/xen.lds.S|   7 +++
  xen/include/asm-arm/tee.h |  79 +++
  11 files changed, 253 insertions(+)
  create mode 100644 xen/arch/arm/tee/Kconfig
  create mode 100644 xen/arch/arm/tee/Makefile
  create mode 100644 xen/arch/arm/tee/tee.c
  create mode 100644 xen/include/asm-arm/tee.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 77b1e11..ede00c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -357,6 +357,11 @@ F: config/Stubdom.mk.in
  F:m4/stubdom.m4
  F:stubdom/
  
+TEE MEDIATORS

+M: Volodymyr Babchuk 
+S: Supported
+F: xen/arch/arm/tee/*
+
  TOOLSTACK
  M:Ian Jackson 
  M:Wei Liu 
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index d46b98c..e1f112a 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -50,6 +50,14 @@ config HAS_ITS
  prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
  depends on HAS_GICV3
  
+config ARM_TEE


The ARM in the title is a bit pointless. This Kconfig is only used for 
Arm architecture.



+   bool "Enable TEE mediators support"
+   default n
+   depends on ARM


No need for that.


+   help
+ This option enables generic TEE mediators support. It allows guests
+ to access real TEE via one of TEE mediators implemented in XEN


Missing full stop.


+
  endmenu
  
  menu "ARM errata workaround via the alternative framework"

@@ -167,3 +175,5 @@ endmenu
  source "common/Kconfig"
  
  source "drivers/Kconfig"

+
+source "arch/arm/tee/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index ede21fd..2710e0e 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -3,6 +3,7 @@ subdir-$(CONFIG_ARM_64) += arm64
  subdir-y += platforms
  subdir-$(CONFIG_ARM_64) += efi
  subdir-$(CONFIG_ACPI) += acpi
+subdir-$(CONFIG_ARM_TEE) += tee
  
  obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o

  obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 784ae39..3290d39 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -31,6 +31,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -673,6 +674,9 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,
  if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
  goto fail;
  
+/* Notify TEE that new domain was created */

+tee_domain_create(d);


I am not a big fan to see this in arch_domain_create until we see how 
this is going to fit with guest. For instance, will TEE be for every 
guests? What would be the other necessary information to configure it?...



+
  return 0;
  
  fail:

@@ -684,6 +688,9 @@ fail:
  
  void arch_domain_destroy(struct domain *d)

  {
+/* Notify TEE that domain is being destroyed */
+tee_domain_destroy(d);
+
  /* IOMMU page table is shared with P2M, always call
   * iommu_domain_destroy() before p2m_teardown().
   */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92f173b..8a4fcd8 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -47,6 +47,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  
@@ -846,6 +847,9 @@ void __init 

Re: [Xen-devel] [PATCH v6] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Jan Beulich]

>>> On 13.10.17 at 22:42,  wrote:
> @@ -4586,6 +4620,107 @@ static int do_altp2m_op(
>  return rc;
>  }
>  
> +DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
> +
> +#ifndef CHECK_hvm_altp2m_op
> +#define CHECK_hvm_altp2m_op \
> +CHECK_SIZE_(struct, hvm_altp2m_op); \
> +CHECK_FIELD_(struct, hvm_altp2m_op, version); \
> +CHECK_FIELD_(struct, hvm_altp2m_op, cmd); \
> +CHECK_FIELD_(struct, hvm_altp2m_op, domain); \
> +CHECK_FIELD_(struct, hvm_altp2m_op, pad1); \
> +CHECK_FIELD_(struct, hvm_altp2m_op, pad2);
> +#endif /* CHECK_hvm_altp2m_op */
> +
> +#ifndef CHECK_hvm_altp2m_set_mem_access_multi
> +#define CHECK_hvm_altp2m_set_mem_access_multi \
> +CHECK_SIZE_(struct, hvm_altp2m_set_mem_access_multi); \
> +CHECK_FIELD_(struct, hvm_altp2m_set_mem_access_multi, view); \
> +CHECK_FIELD_(struct, hvm_altp2m_set_mem_access_multi, pad); \
> +CHECK_FIELD_(struct, hvm_altp2m_set_mem_access_multi, nr); \
> +CHECK_FIELD_(struct, hvm_altp2m_set_mem_access_multi, opaque);
> +#endif /* CHECK_hvm_altp2m_set_mem_access_multi */

Please omit the trailing semicolons in both cases, just like the
generated macros don't have them. They're ...

> +CHECK_hvm_altp2m_op;
> +CHECK_hvm_altp2m_set_mem_access_multi;

... redundant with the ones here.

> +static int compat_altp2m_op(
> +XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +int rc = 0;
> +struct compat_hvm_altp2m_op a;
> +union
> +{
> +XEN_GUEST_HANDLE_PARAM(void) hnd;
> +struct xen_hvm_altp2m_op *altp2m_op;
> +} nat;
> +
> +if ( !hvm_altp2m_supported() )
> +return -EOPNOTSUPP;
> +
> +if ( copy_from_guest(, arg, 1) )
> +return -EFAULT;
> +
> +if ( a.pad1 || a.pad2 ||
> + (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) )
> +return -EINVAL;
> +
> +set_xen_guest_handle(nat.hnd, COMPAT_ARG_XLAT_VIRT_BASE);
> +
> +switch ( a.cmd )
> +{
> +case HVMOP_altp2m_set_mem_access_multi:
> +BUILD_BUG_ON(sizeof(struct xen_hvm_altp2m_set_mem_access_multi) <
> + sizeof(struct compat_hvm_altp2m_set_mem_access_multi));

With the checking macros above I would hope this isn't needed
anymore.

> +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list(_d_, _s_); \
> +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
> +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list(_d_, _s_); \
> +guest_from_compat_handle((_d_)->access_list, (_s_)->access_list)
> +
> XLAT_hvm_altp2m_set_mem_access_multi(_op->u.set_mem_access_multi,
> + _mem_access_multi);
> +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list
> +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list
> +break;
> +default:
> +return do_altp2m_op(arg);
> +}
> +
> +/*
> + * Manually fill the common part of the xen_hvm_altp2m_op structure 
> because
> + * the generated XLAT_hvm_altp2m_op macro doesn't correctly handle the
> + * translation of all fields from compat_hvm_altp2m_op to 
> xen_hvm_altp2m_op.
> + */

I think a variant of this comment would now better be placed ahead
of the CHECK_hvm_altp2m_* macro definitions above, with the one
here becoming brief by simply referring to the larger one.

> +nat.altp2m_op->version  = a.version;
> +nat.altp2m_op->cmd  = a.cmd;
> +nat.altp2m_op->domain   = a.domain;
> +nat.altp2m_op->pad1 = a.pad1;
> +nat.altp2m_op->pad2 = a.pad2;
> +
> +rc = do_altp2m_op(nat.hnd);
> +
> +switch ( a.cmd )
> +{
> +case HVMOP_altp2m_set_mem_access_multi:
> +/*
> + * The return code can be positive only if it is the return value
> + * of hypercall_create_continuation. In this case, the opaque value
> + * must be copied back to the guest.
> + */
> +if ( rc > 0 )
> +{

ASSERT(rc == ...);

> +a.u.set_mem_access_multi.opaque =
> +nat.altp2m_op->u.set_mem_access_multi.opaque;
> +if ( __copy_field_to_guest(guest_handle_cast(arg,
> + 
> compat_hvm_altp2m_op_t),
> +   , u.set_mem_access_multi.opaque) )
> +rc = -EFAULT;
> +}
> +break;

default:
ASSERT_UNREACHABLE();

Jan

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

Re: [Xen-devel] [PATCH v8 08/16] x86: implement set value flow for MBA

[Jan Beulich]

>>> On 16.10.17 at 05:04,  wrote:
> This patch implements set value flow for MBA including its callback
> function and domctl interface.
> 
> Signed-off-by: Yi Sun 

Reviewed-by: Jan Beulich 

> v8:
> - restore some unnecessary changes in 'cat_check_cbm'.
>   (suggested by Jan Beulich)

This reads as if you did exactly the opposite thing of what you
actually did.

> +static bool mba_sanitize_thrtl(const struct feat_node *feat, uint32_t *thrtl)
> +{
> +if ( *thrtl > feat->mba.thrtl_max )
> +return false;

Wouldn't it be better to do this check after ...

> +/*
> + * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> + * 1. Linear mode: In the linear mode the input precision is defined
> + *as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> + *input precision is 10%. Values not an even multiple of the
> + *precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> + *applied).
> + * 2. Non-linear mode: Input delay values are powers-of-two from zero
> + *to the MBA_MAX value from CPUID. In this case any values not a
> + *power of two will be rounded down the next nearest power of two.
> + */
> +if ( feat->mba.linear )
> +*thrtl -= *thrtl % (100 - feat->mba.thrtl_max);
> +else
> +{
> +/* Not power of 2. */
> +if ( *thrtl & (*thrtl - 1) )
> +*thrtl = 1 << (fls(*thrtl) - 1);
> +}

... these adjustments? That way someone specifying e.g. a linear
value of 95% would get 90% just like for 85% (s)he would get
80%.

> +return true;

If so, the return statement could simply become

return *thrtl <= feat->mba.thrtl_max;

My R-b also applies if you decide to make this change.

Jan


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

Re: [Xen-devel] [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Jan Beulich]

>>> On 16.10.17 at 14:37,  wrote:
> On 16/10/17 13:32, Jan Beulich wrote:
>> Since the emulator acts on the live hardware registers, we need to
>> prevent the compiler from using them e.g. for inlined memcpy() /
>> memset() (as gcc7 does). We can't, however, set this from the command
>> line, as otherwise the 64-bit build would face issues with functions
>> returning floating point values and being declared in standard headers.
>>
>> As the pragma isn't available prior to gcc6, we need to invoke it
>> conditionally. Luckily up to gcc6 we haven't seen generated code access
>> SIMD registers beyond what our asm()s do.
>>
>> Reported-by: George Dunlap 
>> Signed-off-by: Jan Beulich 
>> ---
>> While this doesn't affect core functionality, I think it would still be
>> nice for it to be allowed in for 4.10.
> 
> Agreed.
> 
> Has this been tested with Clang?

Sorry, no - still haven't got around to set up a suitable Clang
locally.

>  It stands a good chance of being
> compatible, but we may need an && !defined(__clang__) included.

Should non-gcc silently ignore "#pragma GCC ..." it doesn't
recognize, or not define __GNUC__ in the first place if it isn't
sufficiently compatible? I.e. if anything I'd expect we need
"#elif defined(__clang__)" to achieve the same for Clang by
some different pragma (if such exists).

Jan


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

Re: [Xen-devel] [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Andrew Cooper]

On 16/10/17 13:32, Jan Beulich wrote:
> Since the emulator acts on the live hardware registers, we need to
> prevent the compiler from using them e.g. for inlined memcpy() /
> memset() (as gcc7 does). We can't, however, set this from the command
> line, as otherwise the 64-bit build would face issues with functions
> returning floating point values and being declared in standard headers.
>
> As the pragma isn't available prior to gcc6, we need to invoke it
> conditionally. Luckily up to gcc6 we haven't seen generated code access
> SIMD registers beyond what our asm()s do.
>
> Reported-by: George Dunlap 
> Signed-off-by: Jan Beulich 
> ---
> While this doesn't affect core functionality, I think it would still be
> nice for it to be allowed in for 4.10.

Agreed.

Has this been tested with Clang?  It stands a good chance of being
compatible, but we may need an && !defined(__clang__) included.

~Andrew

>
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -4,6 +4,11 @@
>  #include 
>  #include 
>  #include 
> +
> +#if __GNUC__ >= 6
> +#pragma GCC target("no-sse")
> +#endif
> +
>  #include 
>  
>  #include 
>
>
>


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

[Xen-devel] [PATCH] x86emul: keep compiler from using {x, y, z}mm registers itself

[Jan Beulich]

Since the emulator acts on the live hardware registers, we need to
prevent the compiler from using them e.g. for inlined memcpy() /
memset() (as gcc7 does). We can't, however, set this from the command
line, as otherwise the 64-bit build would face issues with functions
returning floating point values and being declared in standard headers.

As the pragma isn't available prior to gcc6, we need to invoke it
conditionally. Luckily up to gcc6 we haven't seen generated code access
SIMD registers beyond what our asm()s do.

Reported-by: George Dunlap 
Signed-off-by: Jan Beulich 
---
While this doesn't affect core functionality, I think it would still be
nice for it to be allowed in for 4.10.

--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -4,6 +4,11 @@
 #include 
 #include 
 #include 
+
+#if __GNUC__ >= 6
+#pragma GCC target("no-sse")
+#endif
+
 #include 
 
 #include 




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

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

[Paolo Bonzini]

On 16/10/2017 14:16, Arnd Bergmann wrote:
> On Mon, Oct 16, 2017 at 2:08 PM, Paolo Bonzini  wrote:
>> On 16/10/2017 10:11, Arnd Bergmann wrote:
>>> Thanks!
>>>
>>> Since you've looked at it overall, do you have an opinion on the question
>>> how to fix the PV interface to deal with the pvclock_wall_clock overflow?
>>
>> It has to be done separately for each hypervisor.
>>
>> In KVM, for example, it is probably best to abandon
>> pvclock_read_wallclock altogether, and instead use the recently
>> introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
>> already using it and it's y2106 safe.
> 
> Right, makes sense. I see that this interface is currently implemented
> only for 64-bit x86 in kvm_emulate_hypercall(). Could this be extended
> to x86-32 and the non-x86 architectures as well?

Yes, it could be implemented for x86-32 too.  The whole pvclock concept
however is specific to x86.

Paolo

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

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

[Arnd Bergmann]

On Mon, Oct 16, 2017 at 2:08 PM, Paolo Bonzini  wrote:
> On 16/10/2017 10:11, Arnd Bergmann wrote:
>> Thanks!
>>
>> Since you've looked at it overall, do you have an opinion on the question
>> how to fix the PV interface to deal with the pvclock_wall_clock overflow?
>
> It has to be done separately for each hypervisor.
>
> In KVM, for example, it is probably best to abandon
> pvclock_read_wallclock altogether, and instead use the recently
> introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
> already using it and it's y2106 safe.

Right, makes sense. I see that this interface is currently implemented
only for 64-bit x86 in kvm_emulate_hypercall(). Could this be extended
to x86-32 and the non-x86 architectures as well?

   Arnd

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

Re: [Xen-devel] [PATCH v1 1/2] tools/libs/evtchn: Add support for restricting a handle

[Ross Lagerwall]

On 10/16/2017 12:29 PM, Ian Jackson wrote:

Ross Lagerwall writes ("Re: [PATCH v1 1/2] tools/libs/evtchn: Add support for 
restricting a handle"):

No. As far as I can see, it can only be used to bind new interdomain
events, not other events.


OK, good, thanks.


This entire file (including the description) is copied directly from
Linux's include/uapi/xen/evtchn.h so the description shouldn't be
changed here anyway.


Acked-by: Ian Jackson 

Not sure if you are targeting this at 4.9.  If you are you should have
CC'd the RM - doing that now.  From an upstream pov these changes
would make some difference to qemu depriv, improving it somewhat, and
they seem very low risk.



I wasn't targeting them at 4.10 since it bumps the version number of 
libxenevtchn and I thought it would be too late to submit a v1 of a 
patch which does that _after_ code freeze. I do agree that the change 
would be low risk.


--
Ross Lagerwall

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

[Xen-devel] [distros-debian-sid test] 72240: tolerable trouble: blocked/broken/fail/pass

[Platform Team regression test user]

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

Failures :-/ but no regressions.

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

baseline version:
 flight   72218

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-sid-netboot-pvgrubfail
 test-amd64-i386-i386-sid-netboot-pvgrub  fail
 test-amd64-i386-amd64-sid-netboot-pygrub fail
 test-arm64-arm64-armhf-sid-netboot-pygrubblocked 
 test-armhf-armhf-armhf-sid-netboot-pygrubfail
 test-amd64-amd64-i386-sid-netboot-pygrub fail



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: convert x86_platform_ops to timespec64

[Paolo Bonzini]

On 16/10/2017 10:11, Arnd Bergmann wrote:
> Thanks!
> 
> Since you've looked at it overall, do you have an opinion on the question
> how to fix the PV interface to deal with the pvclock_wall_clock overflow?

It has to be done separately for each hypervisor.

In KVM, for example, it is probably best to abandon
pvclock_read_wallclock altogether, and instead use the recently
introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
already using it and it's y2106 safe.

Paolo

> Should we add a new version now and deprecate the existing one, or
> do you think that y2106 is far enough out that we should just ignore the
> problem?


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

Re: [Xen-devel] [RFC 0/4] TEE mediator framework + OP-TEE mediator

[Julien Grall]

On 11/10/17 20:01, Volodymyr Babchuk wrote:

Hello all,


Hi Volodymyr,



I want to present TEE mediator, that was discussed earlier ([1]).

I selected design with built-in mediators. This is easiest way,
it removes many questions, it is easy to implement and maintain
(at least I hope so).


Well, it may close the technical questions but still leave the security 
impact unanswered. I would have appreciated a summary of each approach 
and explain the pros/cons.


This would help to understand that maybe it is an easy way but also 
still secure...


To be clear, this series don't look controversial at least for OP-TEE. 
What I am more concerned is about DomU supports.


Cheers,

--
Julien Grall

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

[Xen-devel] [xen-4.9-testing test] 114533: regressions - FAIL

[osstest service owner]

flight 114533 xen-4.9-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/114533/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvopsbroken  in 114501
 build-armhf-pvops5 host-build-prep fail in 114501 REGR. vs. 114118

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-ovmf-amd64 14 guest-localmigrate fail in 114501 pass 
in 114533
 test-amd64-i386-xl-qemuu-win7-amd64 18 guest-start/win.repeat fail in 114501 
pass in 114533
 test-amd64-amd64-xl-qemut-win7-amd64 14 guest-localmigrate fail pass in 114501

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-multivcpu  1 build-check(1)  blocked in 114501 n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1) blocked in 114501 n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked in 114501 n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)   blocked in 114501 n/a
 test-amd64-i386-xl-qemut-win7-amd64 18 guest-start/win.repeat fail in 114501 
blocked in 114118
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 114501 
like 114118
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 114091
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 114091
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 114118
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 114118
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-install  

Re: [Xen-devel] [PATCH v1 1/2] tools/libs/evtchn: Add support for restricting a handle

[Ian Jackson]

Ross Lagerwall writes ("Re: [PATCH v1 1/2] tools/libs/evtchn: Add support for 
restricting a handle"):
> No. As far as I can see, it can only be used to bind new interdomain 
> events, not other events.

OK, good, thanks.

> This entire file (including the description) is copied directly from 
> Linux's include/uapi/xen/evtchn.h so the description shouldn't be 
> changed here anyway.

Acked-by: Ian Jackson 

Not sure if you are targeting this at 4.9.  If you are you should have
CC'd the RM - doing that now.  From an upstream pov these changes
would make some difference to qemu depriv, improving it somewhat, and
they seem very low risk.

Ian.

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

Re: [Xen-devel] [PATCH] toolcore: Build in rumprun environment too

[Julien Grall]

On 16/10/17 11:30, Wei Liu wrote:

On Mon, Oct 16, 2017 at 11:17:12AM +0100, Ian Jackson wrote:

Otherwise,
   f942a9b4a12081d5f9a4679d06e88cb5d503396e
   xentoolcore_restrict_all: "Implement" for xenstore
breaks the build of the tools inside rumprun.

Signed-off-by: Ian Jackson 


Acked-by: Wei Liu 


Release-acked-by: Julien Grall 

Cheers,



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



--
Julien Grall

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

[Xen-devel] [PATCH] toolcore: Build in rumprun environment too

[Ian Jackson]

Otherwise,
  f942a9b4a12081d5f9a4679d06e88cb5d503396e
  xentoolcore_restrict_all: "Implement" for xenstore
breaks the build of the tools inside rumprun.

toolcore is in libs, so we need to add the CONFIG_RUMP special case to
tools/libs/Makefile and add toolcore there.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
v2: toolcore is in libs
---
 tools/Makefile  | 2 +-
 tools/libs/Makefile | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index 03d326a..84d6e3b 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -49,7 +49,7 @@ SUBDIRS-$(OCAML_TOOLS) += ocaml
 endif
 
 ifeq ($(CONFIG_RUMP),y)
-SUBDIRS-y := libxc xenstore
+SUBDIRS-y := libs libxc xenstore
 endif
 
 # For the sake of linking, set the sys-root
diff --git a/tools/libs/Makefile b/tools/libs/Makefile
index ea9a64d..88901e7 100644
--- a/tools/libs/Makefile
+++ b/tools/libs/Makefile
@@ -10,4 +10,8 @@ SUBDIRS-y += call
 SUBDIRS-y += foreignmemory
 SUBDIRS-y += devicemodel
 
+ifeq ($(CONFIG_RUMP),y)
+SUBDIRS-y := toolcore
+endif
+
 all clean install distclean uninstall: %: subdirs-%
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] toolcore: Build in rumprun environment too

[Wei Liu]

On Mon, Oct 16, 2017 at 12:23:12PM +0100, Ian Jackson wrote:
> Otherwise,
>   f942a9b4a12081d5f9a4679d06e88cb5d503396e
>   xentoolcore_restrict_all: "Implement" for xenstore
> breaks the build of the tools inside rumprun.
> 
> toolcore is in libs, so we need to add the CONFIG_RUMP special case to
> tools/libs/Makefile and add toolcore there.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v1 1/2] tools/libs/evtchn: Add support for restricting a handle

[Ross Lagerwall]

On 10/16/2017 11:53 AM, Ian Jackson wrote:

Ross Lagerwall writes ("[PATCH v1 1/2] tools/libs/evtchn: Add support for 
restricting a handle"):

+/*
+ * Restrict this file descriptor so that it can only be used to bind
+ * new interdomain events from one domain.


Can it be used to bind other kinds of events ?  The phrasing is
ambigous.



No. As far as I can see, it can only be used to bind new interdomain 
events, not other events.


This entire file (including the description) is copied directly from 
Linux's include/uapi/xen/evtchn.h so the description shouldn't be 
changed here anyway.


--
Ross Lagerwall

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

Re: [Xen-devel] [PATCH v2 2/2] xentoolcore_restrict_all: Implement for libxenevtchn

[Ian Jackson]

Ross Lagerwall writes ("[PATCH v2 2/2] xentoolcore_restrict_all: Implement for 
libxenevtchn"):
> Signed-off-by: Ross Lagerwall 
...
>  int osdep_evtchn_open(xenevtchn_handle *xce);
> diff --git a/tools/libs/toolcore/include/xentoolcore.h 
> b/tools/libs/toolcore/include/xentoolcore.h
> index be6c570..ef9c670 100644
> --- a/tools/libs/toolcore/include/xentoolcore.h
> +++ b/tools/libs/toolcore/include/xentoolcore.h
> @@ -31,11 +31,6 @@
>   * Arranges that Xen library handles (fds etc.) which are currently held
>   * by Xen libraries, can no longer be used other than to affect domid.
>   *
> - * Does not prevent effects that amount only to
> - *   - denial of service, possibly host-wide, by resource exhaustion etc.
> - *   - leak of not-very-interesting metainformation about other domains
> - * eg, specifically, event channel signals relating to other domains

Are we sure that all possible resource exhaustion attacks are now
excluded ?

Ian.

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

Re: [Xen-devel] [PATCH v1 1/2] tools/libs/evtchn: Add support for restricting a handle

[Ian Jackson]

Ross Lagerwall writes ("[PATCH v1 1/2] tools/libs/evtchn: Add support for 
restricting a handle"):
> +/*
> + * Restrict this file descriptor so that it can only be used to bind
> + * new interdomain events from one domain.

Can it be used to bind other kinds of events ?  The phrasing is
ambigous.

The code LGTM.

Ian.

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