Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Jan Beulich]

>>> On 09.10.15 at 04:56,  wrote:
> All existing commands ignore the parameter so this does
> not break the ABI.

Does it not? What about the debug mode clobbering of hypercall
argument registers? I think such length indicators need to be part
of the newly added sub-structures instead.

> This paves the way for expanding the XENVER_
> hypercall with variable size structures, such as
> "XENVER_build_id: Provide ld-embedded build-ids"
> 
> Suggested-by: Andrew Cooper 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  xen/arch/arm/traps.c| 2 +-

xen/arch/x86/x86_64/entry.S
xen/arch/x86/x86_64/compat/entry.S

(but that's moot with the comment above)

Jan


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

Re: [Xen-devel] [V5 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen

[Jan Beulich]

>>> On 09.10.15 at 10:14,  wrote:
> On Fri, Oct 09, 2015 at 01:13:11AM -0600, Jan Beulich wrote:
>> >>> On 09.10.15 at 07:49,  wrote:
>> > On Mon, Sep 28, 2015 at 10:03:12AM -0600, Jan Beulich wrote:
>> >> >>> On 21.09.15 at 13:33,  wrote:
>> >> > @@ -954,8 +975,13 @@ long arch_do_domctl(
>> >> >  v->arch.xcr0_accum = _xcr0_accum;
>> >> >  if ( _xcr0_accum & XSTATE_NONLAZY )
>> >> >  v->arch.nonlazy_xstate_used = 1;
>> >> > -memcpy(v->arch.xsave_area, _xsave_area,
>> >> > -   evc->size - 2 * sizeof(uint64_t));
>> >> > +if ( (cpu_has_xsaves || cpu_has_xsavec) &&
>> >> > +!xsave_area_compressed(_xsave_area) )
>> >> 
>> >> Is it intended to support compact input here? Where would such
>> >> come from? And if so, don't you need to validate the input (e.g.
>> >> being a certain size)?
>> >> 
>> > It is not intended to support compact input here.Just add some check here 
>> > (According to Andrew suggestion).
>> 
>> I think your reply only refers to the cpu_has_xsavec conditional,
>> but the question really was about the construct as a whole.
>> 
> There is no such case that input is compact.
> "!xsave_area_compressed(_xsave_area)" here just add some check to ensure
> _xsave_area is uncompact.

How is there not? The if() above has an else branch, so you're
taking care of both cases.

> It seem that it cause confusion. If so , I will 
> deletesuch check. Is it Ok ?

That depends on why Andrew had asked for it. If the input can't
be compressed, check it's not and bail otherwise, instead of giving
the impression of handling the case.

Jan


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

[Xen-devel] [linux-linus test] 62721: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-multivcpu  3 host-install(3)broken REGR. vs. 59254
 test-amd64-i386-xl3 host-install(3) broken REGR. vs. 59254
 test-amd64-i386-xl-qemut-debianhvm-amd64 3 host-install(3) broken REGR. vs. 
59254
 test-amd64-i386-freebsd10-amd64  3 host-install(3)  broken REGR. vs. 59254
 test-amd64-i386-xl-qemut-winxpsp3  3 host-install(3)broken REGR. vs. 59254
 test-armhf-armhf-xl-cubietruck  6 xen-bootfail REGR. vs. 59254
 test-armhf-armhf-xl   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-credit2   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail REGR. vs. 59254
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm   6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 16 
guest-localmigrate/x10 fail REGR. vs. 59254

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-xsm  3 host-install(3) broken REGR. vs. 59254
 test-amd64-amd64-xl-rtds  3 host-install(3) broken REGR. vs. 59254
 test-amd64-amd64-libvirt-qcow2  3 host-install(3) broken baseline untested
 test-amd64-i386-libvirt-raw   3 host-install(3)   broken baseline untested
 test-amd64-amd64-amd64-pvgrub  3 host-install(3)  broken baseline untested
 test-amd64-amd64-xl-qcow2 3 host-install(3)   broken baseline untested
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail   like 59254
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 59254

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass

version targeted for testing:
 linuxc6fa8e6de3dc420cba092bf155b2ed25bcd537f7
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z   92 days
Failing since 59348  2015-07-10 04:24:05 Z   91 days   51 attempts
Testing same since62721  2015-10-08 01:16:32 Z1 days1 attempts


2405 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  

Re: [Xen-devel] [PATCH v1 3/4] XENVER_build_id: Provide ld-embedded build-ids

[Ian Campbell]

On Thu, 2015-10-08 at 22:56 -0400, Konrad Rzeszutek Wilk wrote:
> @@ -1633,6 +1633,8 @@ int xc_sysctl(xc_interface *xch, struct xen_sysctl
> *sysctl);
>  
>  int xc_version(xc_interface *xch, int cmd, void *arg);
>  
> +int xc_version_len(xc_interface *xch, int cmd, void *arg, size_t len);

If we are going this route (not sure if Jan's comments on #2 invalidate
this approach) then please just update the xc_version API and the in tree
callers.

libxc doesn't have a stable API so there is no need to provide variants for
compatibility.


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

Re: [Xen-devel] [PATCH v1 1/4] xsm/libxl/xen_version: Add XSM for some of the xen_version commands.

[Ian Campbell]

On Thu, 2015-10-08 at 22:56 -0400, Konrad Rzeszutek Wilk wrote:
> The XENVER_[compile_info|changeset|commandline] are now
> guarded by an XSM check.

I can guess, but please explain/justify why this is the case for these
here.

> The rest: XENVER_[version|extraversion|capabilities|
> parameters|get_features|page_size|guest_handle] behave
> as before (no XSM check).

and correspondingly why these ones to not warrant such a change.

> As such we also modify the toolstack such that if we fail
> to get any data instead of printing (null) we just print "".

Perhaps the hypervisor should instead return "" or some suitable
string indicating why ()?

> @@ -720,4 +720,28 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG
> struct domain *d, unsigned int
>  }
>  }
>  
> +#include 
> +static XSM_INLINE int xsm_version_op (XSM_DEFAULT_ARG uint32_t op)
> +{
> +XSM_ASSERT_ACTION(XSM_PRIV);
> +switch ( op )
> +{
> +case XENVER_compile_info:
> +case XENVER_changeset:
> +case XENVER_commandline:
> +return xsm_default_action(XSM_PRIV, current->domain, NULL);
> +case XENVER_version:
> +case XENVER_extraversion:
> +case XENVER_capabilities:
> +case XENVER_platform_parameters:
> +case XENVER_get_features:
> +case XENVER_pagesize:
> +case XENVER_guest_handle:
> +/* The should _NEVER_ get here, but just in case. */

BUG_ON?

IMHO such a comment should have a "because ..." in it.

Actually, thinking about it, instead of splitting access control between
do_xen_version and here it would be more normal to have this function DTRT
and for it to be called unconditionally.

Ian.


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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Ian Campbell]

On Thu, 2015-10-08 at 22:56 -0400, Konrad Rzeszutek Wilk wrote:
> If the hypervisor is built with we will display it.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  tools/libxl/libxl.c | 16 
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c|  1 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index efa6462..f9af78c 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5249,6 +5249,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx,
> int *nr)
>  return ret;
>  }
>  
> +#define BUILD_ID_LEN 1024 /* Same size as xen_commandline. */

This either needs to be well defined in the hypercall ABI or the code here
needs to cope with expanding the buffer on ENOBUFS and trying again.

Ian.

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

Re: [Xen-devel] [PATCH] tools: remove unused wrappers for python

[Ian Campbell]

On Fri, 2015-10-09 at 06:42 +0200, Juergen Gross wrote:

> Is it okay to remove all of the unused bindings?

Please can you remind me what your motivation is here, just cleaning up
detritus or are some of these bindings actively hindering work you are
doing? Or are they known to be broken?

If it's just tidying up and not getting in your way then I'm not sure this
is worth the effort, but to answer your question above:

IMHO (and other tools maintainers may disagree) ones which are truly to the
best of our knowledge unused, yes. "To the best of our knowledge" probably
involves some due diligence and fair warning first.

It would be best to announce the intention on xen-devel in a mail with a
subject ("Intention to deprectate and remove some Python libxc bindings)
which will garner sufficient attention (i.e. not below a patch posting
thread) and Cc anyone who we think might be using the interfaces (Andy,
Zhigang, the chap from invisible-things who spoke up recently in a similar
thread if not this exact one).

It is probably worth cross posting to xen-users to get input from there.

Ian.



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

Re: [Xen-devel] [RFC] Xen 4.6 Acknowledgements

[Lars Kurth]

> On 9 Oct 2015, at 10:09, Ian Campbell  wrote:
> 
> On Thu, 2015-10-08 at 18:04 +0100, Lars Kurth wrote:
>> Hi,
>> 
>> because we are now branching and opening master before the release, I
>> have to make some changes to how I acknowledge Xen release contributions.
>> 
>> In the past, I took the time-stamps of the two RELEASE tags for a release
>> and counted contributions to xen.git and osstest.git (I didn't count qemu
>> -trad)
> 
> How do the time-stamps get used? Are you doing git log --after=X -
> -before=Y?

I was using git log -p -M --since=x --until=y, with master checked out, which 
is what was recommended in GitDM when doing reporting. Mainly because most of 
the scripts I have are used to do monthly and yearly reporting.

> Why don't you just look at the commits between the two tags (i.e. git log
> RELEASE-4.5.0..RELEASE-4.6.0 or whatever span you are using)?

That's what I do for xen.git and was planning to do in future, but 
unfortunately RELEASE-4.6.0 are not applied consistently to all repos (aka 
OSSTEST, etc). 

> The problem with using the time-stamp on a commit is that various git
> operations (git rebase, git send-email + git am) can end up preserving the
> original commit date in the authors tree, which can differ quite
> significantly from the date something was committed to be pushed to the
> repo.

Does -M fix this issue? I can't find any docs for it.

>> 3: "Hypervisor other" will list contributions in a number of related
>> repos that are listed, but I would use the timestamps of the release
>> tags. I was planning to include the following repos: osstest.git - can
>> add friends also (as testing is important), raisin, mini-os (as it used
>> to be in xen.git). 
> 
> FWIW some of the "other" repos to also gain their own tags corresponding to
> the release (e.g. mini-os). Where it exists I suppose it makes sense to use
> it?

It is too painful to do this at least for this release, and some repos don't 
contain tags at all. The scripts I have, check out all the repos I want to 
measure (e.g. all XAPI, all Mirage, ... repos) into a directory and I use 
something like 

find $i/* -maxdepth 0 -type d | xargs -i git -C {} log $args >> 
$root/output/$i/$ext.gitlog

to aggregate all the logs for all the relevant repos.  

>> I could also include qemu-traditional into (3), which I have not counted
>> in the past. So I was thinking I'd not do this. 
> 
> qemu-traditional doesn't get much traffic, but I don't see a reason to
> exclude it. The separation/change you are proposing here sounds like an
> ideal point to reintroduce it where it wasn't included before. 

That's what I was thinking and why I left it out

>> Also including Linux, BSD, ... is hard as  pinpointing the xen-related
>> parts are too hard.
> 
> I suppose qemu-upstream falls into this bucket too? (Which I suppose mght
> then be an argument for also excluding -trad?)

Agreed, but the challenge is that it is near impossible to filter out KVM, 
Linux, ... contributions.

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

[Xen-devel] osstest: elbling0 bios boot order reset

[Ian Jackson]

FTR:

09:12 <@ijc> Diziet: elbling0 seems to have stopped pxe booting and is
 always booting from it's disk:
09:12 <@ijc> 
http://logs.test-lab.xenproject.org/osstest/results/host/elbling0.html
09:12 <@ijc> 
http://logs.test-lab.xenproject.org/osstest/logs/62716/test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm/serial-elbling0.log
09:12 <@ijc> I've setflags \!blessed-{adhoc,play,real} on it (but not
 elbling1 which looks ok)

I booked the machine out and looked at the BIOS.  I reset the boot
order (to 1. network; 2. the actual sata disk; 3. "C:" (not sure what
this is for).

I didn't book out elbling1 too so I haven't double-checked the other
setttings.

When rebooted it booted from the network and found the preseed file
left by the last ts-host-install attempt.  I have reblessed it and
thrown it back.

11:09  I wish these machines wouldn't periodically forget to
   boot from the network.
11:15  It happens in Cambridge too occasionally.
11:15  I think this is about the 1st or 2nd time in the new
   colo so maybe newer machines are a bit better.

Ian.

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

Re: [Xen-devel] [V5 1/4] x86/xsaves: add basic definitions/helpers to support xsaves

[Jan Beulich]

>>> On 09.10.15 at 10:17,  wrote:
> On Mon, Sep 28, 2015 at 03:01:50AM -0600, Jan Beulich wrote:
>> >>> On 21.09.15 at 13:33,  wrote:
>> > +void save_xsave_states(struct vcpu *v, void *dest, unsigned int size)
>> 
>> Iirc it was suggested before that the function's name isn't adequate:
>> There's no saving of anything here. You're expanding already saved
>> state.
>> 
> Is void expand_xsave_states(struct vcpu *v , void *dest, unsigned int size);
> Ok ?
>> > +
>> > +void load_xsave_states(struct vcpu *v, const void *src, unsigned int size)
>> 
>> Similar comments as above apply to this function.
>> 
> Is void compress_xsave_states(struct vcpu *v , void *dest, unsigned int 
> size);
> Ok ?

Afaic - yes to both.

Jan


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

Re: [Xen-devel] [RFC] Xen 4.6 Acknowledgements

[Ian Campbell]

On Fri, 2015-10-09 at 10:57 +0100, Lars Kurth wrote:
> > On 9 Oct 2015, at 10:09, Ian Campbell  wrote:
> > 
> > On Thu, 2015-10-08 at 18:04 +0100, Lars Kurth wrote:
> > > Hi,
> > > 
> > > because we are now branching and opening master before the release, I
> > > have to make some changes to how I acknowledge Xen release
> > > contributions.
> > > 
> > > In the past, I took the time-stamps of the two RELEASE tags for a
> > > release
> > > and counted contributions to xen.git and osstest.git (I didn't count
> > > qemu
> > > -trad)
> > 
> > How do the time-stamps get used? Are you doing git log --after=X -
> > -before=Y?
> 
> I was using git log -p -M --since=x --until=y, with master checked out,
> which is what was recommended in GitDM when doing reporting. Mainly
> because most of the scripts I have are used to do monthly and yearly
> reporting.

OK, I suppose if that's what GitDM says to do there's no reason to deviate.

> > Why don't you just look at the commits between the two tags (i.e. git log
> > RELEASE-4.5.0..RELEASE-4.6.0 or whatever span you are using)?
> 
> That's what I do for xen.git and was planning to do in future, but
> unfortunately RELEASE-4.6.0 are not applied consistently to all repos
> (aka OSSTEST, etc). 

Indeed, I only meant for repos which have the tags.

> > The problem with using the time-stamp on a commit is that various git
> > operations (git rebase, git send-email + git am) can end up preserving
> > the
> > original commit date in the authors tree, which can differ quite
> > significantly from the date something was committed to be pushed to the
> > repo.
> 
> Does -M fix this issue? I can't find any docs for it.

-M is "find renames", which makes the diff of code motion easier to follow,
so I don't think so.

> 
> > > 3: "Hypervisor other" will list contributions in a number of related
> > > repos that are listed, but I would use the timestamps of the release
> > > tags. I was planning to include the following repos: osstest.git - can
> > > add friends also (as testing is important), raisin, mini-os (as it used
> > > to be in xen.git). 
> > 
> > FWIW some of the "other" repos to also gain their own tags corresponding to
> > the release (e.g. mini-os). Where it exists I suppose it makes sense to use
> > it?
> 
> It is too painful to do this at least for this release,

Sure.

>  and some repos don't contain tags at all. The scripts I have, check out
> all the repos I want to measure (e.g. all XAPI, all Mirage, ... repos)
> into a directory and I use something like 
> 
> find $i/* -maxdepth 0 -type d | xargs -i git -C {} log $args >>
> $root/output/$i/$ext.gitlog
> 
> to aggregate all the logs for all the relevant repos.  
> 
> > > I could also include qemu-traditional into (3), which I have not
> > > counted
> > > in the past. So I was thinking I'd not do this. 
> > 
> > qemu-traditional doesn't get much traffic, but I don't see a reason to
> > exclude it. The separation/change you are proposing here sounds like an
> > ideal point to reintroduce it where it wasn't included before. 
> 
> That's what I was thinking and why I left it out

I was proposing leaving it in based on that reasoning though.

(did you mean this to be below the comment about upstream?).

> > > Also including Linux, BSD, ... is hard as  pinpointing the xen
> > > -related
> > > parts are too hard.
> > 
> > I suppose qemu-upstream falls into this bucket too? (Which I suppose
> > mght
> > then be an argument for also excluding -trad?)
> 
> Agreed, but the challenge is that it is near impossible to filter out
> KVM, Linux, ... contributions.

Right.

Ian.

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

Re: [Xen-devel] patch "x86/cpufreq: relocate the driver register function" breaks cpu hot(un)plug

[Wang, Wei W]

On 09/10/2015 04:01, Konrad Rzeszutek Wilk wrote:
> On Fri, Oct 09, 2015 at 06:48:23PM +0200, Dario Faggioli wrote:
> > Hey,
> >
> > As far as my bisection goes, commit
> > 49388f11d512bb92706ce046643bfbb3c1d963c9 "x86/cpufreq: relocate the
> > driver register function" prevents me from hot unplugging pCPUs.
> >
> > Xen does not crash or anything, but dom0 is stalled. In fact, with
> > current staging, here's what I see:
> >
> > root@Zhaman:~# echo 0 > /sys/devices/system/xen_cpu/xen_cpu6/online
> > [   81.583001] INFO: rcu_sched detected stalls on CPUs/tasks: { 12} 
> > (detected
> by 3, t=5252 jiffies, g=1691, c=1690, q=76)
> > [   81.583036] Task dump for CPU 12:
> > [   81.583044] bashR  running task0  1347   1094 
> > 0x0008
> > [   81.583056]    
> 8800192c2e38
> > [   81.583070]  888472e8 0002 888472e8
> 880013817858
> > [   81.583082]   81a4 811e8137
> 8800192c2e38
> > [   81.583095] Call Trace:
> > [   81.583110]  [] ? notify_change+0x2f7/0x390
> > [   81.583148]  [] ? do_truncate+0x74/0x90
> > [   81.583158]  [] ? dput+0x26/0x230
> > [   81.583167]  [] ? terminate_walk+0x35/0x40
> > [   81.583176]  [] ? do_last+0x621/0x12c0
> > [   81.583188]  [] ? xen_pcpu_down+0x47/0x70
> > [   81.583199]  [] ? store_online+0x9d/0xb0
> > [   81.583210]  [] ? kernfs_fop_write+0x12c/0x180
> > [   81.583220]  [] ? __vfs_write+0x23/0xf0
> > [   81.583230]  [] ? __sb_start_write+0x42/0xf0
> > [   81.583241]  [] ? security_file_permission+0x21/0xa0
> > [   81.583250]  [] ? vfs_write+0xa1/0x1c0
> > [   81.583259]  [] ? filp_close+0x4f/0x70
> > [   81.583268]  [] ? SyS_write+0x42/0xb0
> > [   81.583277]  [] ? __close_fd+0x71/0xb0
> > [   81.583287]  [] ? system_call_fastpath+0x16/0x75
> > [  144.555020] INFO: rcu_sched detected stalls on CPUs/tasks: { 12}
> > (detected by 4, t=21007 jiffies, g=1691, c=1690, q=244) [  144.555046] Task
> dump for CPU 12:
> > [  144.555051] bashR  running task0  1347   1094 
> > 0x0008
> > [  144.555059]    
> > 8800192c2e38 [  144.555068]  888472e8 0002
> > 888472e8 880013817858 [  144.555076]  
> > 81a4 811e8137 8800192c2e38 [  144.555084] Call
> Trace:
> > [  144.555096]  [] ? notify_change+0x2f7/0x390 [
> > 144.555105]  [] ? do_truncate+0x74/0x90 [
> > 144.555112]  [] ? dput+0x26/0x230 [  144.555118]
> > [] ? terminate_walk+0x35/0x40 [  144.555124]
> > [] ? do_last+0x621/0x12c0 [  144.555164]
> > [] ? xen_pcpu_down+0x47/0x70 [  144.555172]
> > [] ? store_online+0x9d/0xb0 [  144.555179]
> > [] ? kernfs_fop_write+0x12c/0x180 [  144.555186]
> > [] ? __vfs_write+0x23/0xf0 [  144.555192]
> > [] ? __sb_start_write+0x42/0xf0 [  144.555200]
> > [] ? security_file_permission+0x21/0xa0
> > [  144.555206]  [] ? vfs_write+0xa1/0x1c0 [
> > 144.555212]  [] ? filp_close+0x4f/0x70 [
> > 144.555217]  [] ? SyS_write+0x42/0xb0 [  144.555223]
> > [] ? __close_fd+0x71/0xb0 [  144.555230]
> > [] ? system_call_fastpath+0x16/0x75
> >
> > If I revert that patch, the issue goes away.
> >
> > Any ideas?

Hi Dario,

Please also remove "register_cpu_notifier(_nfb)" in the 
cpufreq_register_driver function as well. (found that it has already been 
included in cpufreq_presmp_nfb()).

Best,
Wei

> I think it is due to xen-acpi-processor re-uploading the C and P states 
> whenever
> an CPU goes up. It also does this after S3 suspend.
> 
> Anyhow it may be due to the fact that cpufreq_register_driver in Xen is now
> '__init' If you remove that little thing would it work?
> 
> >
> > Regards,
> > Dario
> >
> > PS. yes, I'll implement a cpu hotplug/unplug testcase ASAP. :-)
> >
> > --
> > <> (Raistlin Majere)
> > -
> > Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software
> > Engineer, Citrix Systems R Ltd., Cambridge (UK)
> >
> 
> 
> 
> > ___
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel


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

Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Jan Beulich]

>>> On 09.10.15 at 09:06,  wrote:
>> > >>>On 08.10.2015 at 16:52  wrote:
>> >>> On 07.10.15 at 19:02,  wrote:
>> > Q3: what to do about mappings of other domains' memory (i.e. grant and
>> > foreign mappings).
>> >Between two domains, now I have only one idea to fix this tricky
>> > issue -- waitqueue.
>> >I.e. grant.
>> > For gnttab_transfer /gnttab_unmap , wait on a waitqueue before
>> > updating grant flag, until the Device-TLB flush is completed.
>> > For grant-mapped, it is safe as the modification of gnttab_unmap.
>> 
>> Hmm, wouldn't grant transfers already be taken care of by the extra 
>> references?
>> See steal_page(). Perhaps the ordering between its invocation and
>> guest_physmap_remove_page() would need to be switched (with the latter
>> getting undone if steal_page() fails). The waiting for the flush to complete 
>> could -
>> afaics - be done by using the grant-ops' inherent batching (and hence easy
>> availability of continuations). But I admit there's some hand waiving here 
>> without
>> closer inspection...
> 
> I think the extra references can NOT fix the security issue between two 
> domains.
> i.e. If domA transfers the ownership of a page to domB, but the domA still 
> take extra references of the page. I think it is not correct.

Again - see steal_page(): Pages with references beyond the single
allocation related one can't change ownership.

>> > __scheme B__
>> > Q1: - when to take the references?
>> >
>> > take the reference when the IOMMU entry is _ removed/overwritten_;
>> > in detail:
>> >  --iommu_unmap_page(), or
>> >  --ept_set_entry() [Once IOMMU shares EPT page table.]
>> >
>> > * Make sure IOMMU page should not be reallocated for
>> >  another purpose until the appropriate invalidations have been
>> >  performed.
>> > * in this case, it does not matter hot-plug ATS device
>> > pass-through or ATS device assigned in domain initialization.
>> >
>> > Q2 / Q3:
>> > The same as above __scheme A__ Q2/Q3.
>> >
>> > One question: is __scheme B__ safe? If it is safe, I prefer __scheme B__..
>> 
>> While at the first glance this looks like a neat idea - 
> 
> 
> I think this is safe and a good solution.
> I hope you can review into the __scheme B__. I need _Acked-by_ you and Tim 
> Deegan.

What do you mean here? I'm not going to ack a patch that hasn't
even got written, and while scheme B looks possible, I might still
overlook something, so I also can't up front ack that model (which
may then lead to you expecting that once implemented it gets
accepted).

Jan


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

Re: [Xen-devel] [V5 1/4] x86/xsaves: add basic definitions/helpers to support xsaves

[Shuai Ruan]

On Mon, Sep 28, 2015 at 03:01:50AM -0600, Jan Beulich wrote:
> >>> On 21.09.15 at 13:33,  wrote:
> > This patch add basic definitions/helpers which will be used in
> > later patches.
> > 
> > Signed-off-by: Shuai Ruan 
> > ---
> 
> 
> > +void save_xsave_states(struct vcpu *v, void *dest, unsigned int size)
> 
> Iirc it was suggested before that the function's name isn't adequate:
> There's no saving of anything here. You're expanding already saved
> state.
> 
Is void expand_xsave_states(struct vcpu *v , void *dest, unsigned int size);
Ok ?
> > +
> > +void load_xsave_states(struct vcpu *v, const void *src, unsigned int size)
> 
> Similar comments as above apply to this function.
> 
Is void compress_xsave_states(struct vcpu *v , void *dest, unsigned int size);
Ok ?
> 
> Jan
> 
Thanks 
Shuai

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

Re: [Xen-devel] [V5 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen

[Jan Beulich]

>>> On 09.10.15 at 07:49,  wrote:
> On Mon, Sep 28, 2015 at 10:03:12AM -0600, Jan Beulich wrote:
>> >>> On 21.09.15 at 13:33,  wrote:
>> > @@ -954,8 +975,13 @@ long arch_do_domctl(
>> >  v->arch.xcr0_accum = _xcr0_accum;
>> >  if ( _xcr0_accum & XSTATE_NONLAZY )
>> >  v->arch.nonlazy_xstate_used = 1;
>> > -memcpy(v->arch.xsave_area, _xsave_area,
>> > -   evc->size - 2 * sizeof(uint64_t));
>> > +if ( (cpu_has_xsaves || cpu_has_xsavec) &&
>> > +   !xsave_area_compressed(_xsave_area) )
>> 
>> Is it intended to support compact input here? Where would such
>> come from? And if so, don't you need to validate the input (e.g.
>> being a certain size)?
>> 
> It is not intended to support compact input here.Just add some check here 
> (According to Andrew suggestion).

I think your reply only refers to the cpu_has_xsavec conditional,
but the question really was about the construct as a whole.

Jan


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

Re: [Xen-devel] [xen-4.3-testing test] 62392: tolerable FAIL - PUSHED

[Ian Campbell]

On Thu, 2015-10-08 at 09:51 +0100, Ian Campbell wrote:
> 
> I'm considering manually forcing a rerun of xen-4.3-testing even though no
> changes are pending because I think it would be nice to have an unbroken
> chain of passes for test-

Things were looking pretty quiet so I decided to do this, flight 62742.

Ian.


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

Re: [Xen-devel] Regression: qemu crash of hvm domUs with spice (backtrace included)

[Fabio Fantoni]

Il 08/10/2015 17:58, Andreas Kinzler ha scritto:

Is this still current? I made an interesting observation:

I had no problems with SPICE and vanilla Xen 4.5.1 when using it on 
Gentoo with glibc 2.19/gcc 4.6.4.
Segfaults started when I switched to glibc 2.20/gcc 4.9.3 - I did not 
change Xen source code at all.
All this might be related to: 
https://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg02764.html


Andreas


Thanks for your mail.
The problem I had seems different, I not found the exactly cause but I 
solved using newer qemu (2.2 from unstable - now 4.6) with xen 4.5.1.

Big distro I saw already use newer qemu and should be ok.
I still using glibc < 2.20 in my debian servers, this is probably 
because I not had your problem but I think that backport the patch you 
linked can be useful for solve a qemu crash case, I'll test it in my 
next build and if I'll not found regression I'll require the backport 
for xen qemu gits.





https://github.com/Fantu/Xen/commits/rebase/m2r-staging
Latest test with regression based on latest stable-4.5, more exactly:
https://github.com/Fantu/Xen/commits/rebase/m2r-testing
Some days ago on same dom0 and domU I tried with latest stable 
version (that I use on only 2 production servers for now but I not 
saw the regression), more exactly:

https://github.com/Fantu/Xen/commits/rebase/m2r-stable-4.5
Dom0 debian 7 with kernel 3.16 from backports, seabios 1.8.1-2 from 
unstable and this xen configure:
./configure --prefix=/usr --disable-blktap1 
--disable-qemu-traditional --disable-rombios 
--with-system-seabios=/usr/share/seabios/bios-256k.bin 
--with-extra-qemuu-configure-args="--enable-spice --enable-usb-redir" 
--disable-blktap2


I suppose that there is unexpected case caused by a backports or 
missed patch/es to backports from unstable.
I not found with a fast look rilevant patch to try to revert, can 
anyone suggest me the more probable point/s for bisect and/or patch 
to revert or I must try full bisect 4.5.0->stable-4.5?







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

Re: [Xen-devel] [PATCH V7 3/7] libxl: add pvusb API

[Chun Yan Liu]

>>> On 10/1/2015 at 01:55 AM, in message <560c2204.9030...@citrix.com>, George
Dunlap  wrote: 
> On 25/09/15 03:11, Chunyan Liu wrote: 
> > Add pvusb APIs, including: 
> >  - attach/detach (create/destroy) virtual usb controller. 
> >  - attach/detach usb device 
> >  - list usb controllers and usb devices 
> >  - get information of usb controller and usb device 
> >  - some other helper functions 
> >  
> > Signed-off-by: Chunyan Liu  
> > Signed-off-by: Simon Cao  
>  
> Hey Chunyan!  This looks pretty close to being ready to check in to me. 
>  
> There are four basic comments I have.  I'm keen to get this series in so 
> that we can start doing more collaborative improvement; so I'll give my 
> comments, and then talk about timing and a plan to get this in as 
> quikcly as possible at the bottom. 
>  
>  
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c 
> > index aea4887..1e2c63e 100644 
> > --- a/tools/libxl/libxl.c 
> > +++ b/tools/libxl/libxl.c 
> > @@ -4192,11 +4192,54 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1) 
> >   
> >   
> / 
> **/ 
> >   
> > +/* Macro for defining device remove/destroy functions for usbctrl */ 
> > +/* Following functions are defined: 
> > + * libxl_device_usbctrl_remove 
> > + * libxl_device_usbctrl_destroy 
> > + */ 
> > + 
> > +#define DEFINE_DEVICE_REMOVE_EXT(type, removedestroy, f)\ 
> > +int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \ 
> > +uint32_t domid, libxl_device_##type *type,  \ 
> > +const libxl_asyncop_how *ao_how)\ 
> > +{   \ 
> > +AO_CREATE(ctx, domid, ao_how);  \ 
> > +libxl__device *device;  \ 
> > +libxl__ao_device *aodev;\ 
> > +int rc; \ 
> > +\ 
> > +GCNEW(device);  \ 
> > +rc = libxl__device_from_##type(gc, domid, type, device);\ 
> > +if (rc != 0) goto out;  \ 
> > +\ 
> > +GCNEW(aodev);   \ 
> > +libxl__prepare_ao_device(ao, aodev);\ 
> > +aodev->action = LIBXL__DEVICE_ACTION_REMOVE;\ 
> > +aodev->dev = device;\ 
> > +aodev->callback = device_addrm_aocomplete;  \ 
> > +aodev->force = f;   \ 
> > +libxl__initiate_device_##type##_remove(egc, aodev); \ 
>  
> So this seems to be exactly the same as DEFINE_DEVICE_REMOVE(), except 
> that you call libxl__initiate_device_usbctrl_remove() here rather than 
> libxl__initiate_device_remove(). 
>  
> It seems like it might be better to have a separate patch renaming 
> libxl__initiate_device_remove to libxl__initiate_device_generic_remove 
> (or something like that), and then add a parameter to the definition 
> above making it so that the definitions above pass in "generic". 
>  
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c 
> > index bee5ed5..935f25b 100644 
> > --- a/tools/libxl/libxl_device.c 
> > +++ b/tools/libxl/libxl_device.c 
> > @@ -676,6 +676,10 @@ void libxl__devices_destroy(libxl__egc *egc,  
> libxl__devices_remove_state *drs) 
> >  aodev->action = LIBXL__DEVICE_ACTION_REMOVE; 
> >  aodev->dev = dev; 
> >  aodev->force = drs->force; 
> > +if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB) { 
> > +libxl__initiate_device_usbctrl_remove(egc, aodev); 
> > +continue; 
> > +} 
> >  libxl__initiate_device_remove(egc, aodev); 
>  
> I think this would probably be more clear if you did: 
>  
> if(dev->backend_kind == LIBXL__DEVICE_KIND_VUSB) 
>   libxl__initiate_device_usbctl_remove() 
> else 
>   libxl__initiate_device_remove() 
>  
> > @@ -3951,7 +3966,10 @@ static inline void  
> libxl__update_config_vtpm(libxl__gc *gc, 
> >  #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&\ 
> > (a)->bus == (b)->bus &&  \ 
> > (a)->dev == (b)->dev) 
> > - 
> > +#define COMPARE_USB(a, b) ((a)->u.hostdev.hostbus == 
> > (b)->u.hostdev.hostbus && \ 
> > +   (a)->u.hostdev.hostaddr == 
> > (b)->u.hostdev.hostaddr) 
>  
> 

Re: [Xen-devel] [V5 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen

[Shuai Ruan]

On Fri, Oct 09, 2015 at 01:13:11AM -0600, Jan Beulich wrote:
> >>> On 09.10.15 at 07:49,  wrote:
> > On Mon, Sep 28, 2015 at 10:03:12AM -0600, Jan Beulich wrote:
> >> >>> On 21.09.15 at 13:33,  wrote:
> >> > @@ -954,8 +975,13 @@ long arch_do_domctl(
> >> >  v->arch.xcr0_accum = _xcr0_accum;
> >> >  if ( _xcr0_accum & XSTATE_NONLAZY )
> >> >  v->arch.nonlazy_xstate_used = 1;
> >> > -memcpy(v->arch.xsave_area, _xsave_area,
> >> > -   evc->size - 2 * sizeof(uint64_t));
> >> > +if ( (cpu_has_xsaves || cpu_has_xsavec) &&
> >> > + !xsave_area_compressed(_xsave_area) )
> >> 
> >> Is it intended to support compact input here? Where would such
> >> come from? And if so, don't you need to validate the input (e.g.
> >> being a certain size)?
> >> 
> > It is not intended to support compact input here.Just add some check here 
> > (According to Andrew suggestion).
> 
> I think your reply only refers to the cpu_has_xsavec conditional,
> but the question really was about the construct as a whole.
> 
> Jan
> 
There is no such case that input is compact.
"!xsave_area_compressed(_xsave_area)" here just add some check to ensure
_xsave_area is uncompact. It seem that it cause confusion. If so , I will 
deletesuch check. Is it Ok ?
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

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

Re: [Xen-devel] [PATCH v1] Add build-id to XENVER hypercall.

[Jan Beulich]

>>> On 09.10.15 at 04:56,  wrote:
> However they also change the behavior of the existing hypercall
> for XENVER_[compile_info|changeset|commandline] and make them
> dom0 accessible. This is if XSM is built in or not (though with
> XSM one can expose it to a guest if desired).

Wasn't the outcome of the previous discussion that we should not
alter default behavior for existing sub-ops? And even if I'm
misremembering, I can see reasons for not exposing the command
line, but what value has not exposing compile info and changeset
again? The more that the tool stack uses the two, and as we know
tool stacks or parts thereof can live in unprivileged domains. Plus
there is also a (hg-centric and hence generally broken) attempt to
store it in hvm_save().

> Please take a look and provide your feedback at your leisure.
> 
> Note:
>  * Hadn't tried compiling it on ARM cross compiler lately. In
>the past I had to #ifdef CONFIG_ARM as the ARM code did not
>use any ELF code so none of the ELF parts made any sense.
>  * The EFI build works kindof. It is missing an build_id.o stanza.

Hmm, I'm confused: Does this patch set work everywhere, or does
it not? In the latter case, shouldn't it be tagged RFC?

Jan


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

[Xen-devel] [PATCH 4.5] ipxe fix for gcc5

[Mark Pryor]

Signed-off-by: Mark Pryor 
---
 .../firmware/etherboot/patches/ipxe-ath9k-gcc5.patch | 20 
 .../etherboot/patches/ipxe-ath9k2-gcc5.patch | 20 
 tools/firmware/etherboot/patches/series  |  2 ++
 3 files changed, 42 insertions(+)
 create mode 100644 tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
 create mode 100644 tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch

diff --git a/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch 
b/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
new file mode 100644
index 000..722f5e1
--- /dev/null
+++ b/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
@@ -0,0 +1,20 @@
+--- ipxe.orig/src/drivers/net/ath/ath9k/ath9k_ar9003_phy.c 2011-12-10 
18:28:04.0 -0800
 ipxe/src/drivers/net/ath/ath9k/ath9k_ar9003_phy.c  2015-09-18 
14:42:38.618507334 -0700
+@@ -859,7 +859,7 @@
+   REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
+   AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
+ 
+-  if (!on != aniState->ofdmWeakSigDetectOff) {
++  if ((!on) != aniState->ofdmWeakSigDetectOff) {
+   DBG2("ath9k: "
+   "** ch %d: ofdm weak signal: %s=>%s\n",
+   chan->channel,
+@@ -1013,7 +1013,7 @@
+ AR_PHY_MRC_CCK_ENABLE, is_on);
+   REG_RMW_FIELD(ah, AR_PHY_MRC_CCK_CTRL,
+ AR_PHY_MRC_CCK_MUX_REG, is_on);
+-  if (!is_on != aniState->mrcCCKOff) {
++  if ((!is_on) != aniState->mrcCCKOff) {
+   DBG2("ath9k: "
+   "** ch %d: MRC CCK: %s=>%s\n",
+   chan->channel,
diff --git a/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch 
b/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch
new file mode 100644
index 000..a96bb51
--- /dev/null
+++ b/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch
@@ -0,0 +1,20 @@
+--- ipxe.orig/src/drivers/net/ath/ath9k/ath9k_ar5008_phy.c 2011-12-10 
18:28:04.0 -0800
 ipxe/src/drivers/net/ath/ath9k/ath9k_ar5008_phy.c  2015-09-18 
14:48:17.493922456 -0700
+@@ -1141,7 +1141,7 @@
+   REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
+   AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
+ 
+-  if (!on != aniState->ofdmWeakSigDetectOff) {
++  if ((!on) != aniState->ofdmWeakSigDetectOff) {
+   if (on)
+   ah->stats.ast_ani_ofdmon++;
+   else
+@@ -1307,7 +1307,7 @@
+   REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
+   AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
+ 
+-  if (!on != aniState->ofdmWeakSigDetectOff) {
++  if ((!on) != aniState->ofdmWeakSigDetectOff) {
+   DBG2("ath9k: "
+   "** ch %d: ofdm weak signal: %s=>%s\n",
+   chan->channel,
diff --git a/tools/firmware/etherboot/patches/series 
b/tools/firmware/etherboot/patches/series
index 2c39853..679e0f9 100644
--- a/tools/firmware/etherboot/patches/series
+++ b/tools/firmware/etherboot/patches/series
@@ -4,3 +4,5 @@ build_fix_2.patch
 build_fix_3.patch
 build-compare.patch
 build_fix_4.patch
+ipxe-ath9k-gcc5.patch 
+ipxe-ath9k2-gcc5.patch
-- 
2.1.4



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

Re: [Xen-devel] [PATCH V7 5/7] xl: add pvusb commands

[Chun Yan Liu]

>>> On 10/2/2015 at 01:02 AM, in message <560d6733.4030...@citrix.com>, George
Dunlap  wrote: 
> On 25/09/15 03:11, Chunyan Liu wrote: 
> > Add pvusb commands: usb-ctrl-attach, usb-ctrl-detach, usb-list, 
> > usb-attach and usb-detach. 
> >  
> > To attach a usb device to guest through pvusb, one could follow 
> > following example: 
> >  
> >  #xl usb-ctrl-attach test_vm version=1 num_ports=8 
>  
> So all the way back in v2 of this series, I suggested making the 
> arguments for usb-ctrl-attach and usb-attach mirror the format that is 
> found in the config file[1], at which point you replied "That could be, 
> I can update".  But you didn't change the interface in v3, so I 
> suggested it again[2], and there was no argument or discussion about it. 
>  
> (There was a long back-and-forth with Juergen at that point about 
> usb-assignable-list, so [2] may have gotten lost in the noise.) 
>  
> I still think that's the best interface to use.  Do you have reasons to 
> favor the interface you propose here?

Sorry for replying late, just back from holiday.

It's my fault not updating it, missing that after a lot of other changes, not
favor this or that. Thanks for reminding again.

- Chunyan

>  
>  -George 
>  
> [1] 
> marc.info/?i=  
>om> 
>  
> [2] 
> marc.info/?i= com> 
>  
> >  
> >  #xl usb-list test_vm 
> >  will show the usb controllers and port usage under the domain. 
> >  
> >  #xl usb-attach test_vm 1.6 
> >  will find the first usable controller:port, and attach usb 
> >  device whose bus address is 1.6 (busnum is 1, devnum is 6) 
> >  to it. One could also specify which  and which . 
> >  
> >  #xl usb-detach test_vm 0 1 
> >  will detach USB device under controller 0 port 1. 
> >  
> >  #xl usb-ctrl-detach test_vm dev_id 
> >  will destroy the controller with specified dev_id. Dev_id 
> >  can be traced in usb-list info. 
> >  
> > Signed-off-by: Chunyan Liu  
> > Signed-off-by: Simon Cao  
> > --- 
> >  docs/man/xl.pod.1 |  40  
> >  tools/libxl/xl.h  |   5 + 
> >  tools/libxl/xl_cmdimpl.c  | 232  
> ++ 
> >  tools/libxl/xl_cmdtable.c |  25 + 
> >  4 files changed, 302 insertions(+) 
> >  
> > diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 
> > index f22c3f3..4c92c78 100644 
> > --- a/docs/man/xl.pod.1 
> > +++ b/docs/man/xl.pod.1 
> > @@ -1345,6 +1345,46 @@ List pass-through pci devices for a domain. 
> >   
> >  =back 
> >   
> > +=head1 USB PASS-THROUGH 
> > + 
> > +=over 4 
> > + 
> > +=item B I 

Re: [Xen-devel] [PATCH seabios.git rel-1.7.5] fix release-1.7.5 for gcc5

[Jan Beulich]

>>> On 08.10.15 at 21:36,  wrote:
> Signed-off-by: Mark Pryor 

Without any description I cannot see what is being fixed here, or why
there are _different_ comment changes on the inclusion of the same
comment. Since I assume that whatever issue there was has been
taken care of upstream, I'd suggest - just like for the other patch - to
instead request inclusion of the respective upstream commit (again
obeying ./MAINTAINERS of the affected Xen branch).

Jan

> ---
>  src/kbd.c   | 6 +++---
>  src/mouse.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/kbd.c b/src/kbd.c
> index 33a95a3..fbcecc3 100644
> --- a/src/kbd.c
> +++ b/src/kbd.c
> @@ -11,7 +11,7 @@
>  #include "hw/ps2port.h" // ps2_kbd_command
>  #include "hw/usb-hid.h" // usb_kbd_command
>  #include "output.h" // debug_enter
> -#include "stacks.h" // stack_hop
> +#include "stacks.h" // yield
>  #include "string.h" // memset
>  #include "util.h" // kbd_init
>  
> @@ -117,8 +117,8 @@ static int
>  kbd_command(int command, u8 *param)
>  {
>  if (usb_kbd_active())
> -return stack_hop(command, (u32)param, usb_kbd_command);
> -return stack_hop(command, (u32)param, ps2_kbd_command);
> +return usb_kbd_command(command, param);
> +return ps2_kbd_command(command, param);
>  }
>  
>  // read keyboard input
> diff --git a/src/mouse.c b/src/mouse.c
> index 92ae921..100255d 100644
> --- a/src/mouse.c
> +++ b/src/mouse.c
> @@ -10,7 +10,7 @@
>  #include "hw/ps2port.h" // ps2_mouse_command
>  #include "hw/usb-hid.h" // usb_mouse_command
>  #include "output.h" // dprintf
> -#include "stacks.h" // stack_hop
> +#include "stacks.h" // stack_hop_back
>  #include "util.h" // mouse_init
>  
>  void
> @@ -27,8 +27,8 @@ static int
>  mouse_command(int command, u8 *param)
>  {
>  if (usb_mouse_active())
> -return stack_hop(command, (u32)param, usb_mouse_command);
> -return stack_hop(command, (u32)param, ps2_mouse_command);
> +return usb_mouse_command(command, param);
> +return ps2_mouse_command(command, param);
>  }
>  
>  #define RET_SUCCESS  0x00
> -- 
> 2.1.4
> 
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 




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

Re: [Xen-devel] [PATCH 4.5] ipxe fix for gcc5

[Jan Beulich]

>>> On 08.10.15 at 19:25,  wrote:
> Signed-off-by: Mark Pryor 

To be honest I'd prefer to take the upstream fix for this, i.e. what
we also have in 4.6. Plus for submissions against stable releases,
please see ./MAINTAINERS of the respective tree.

Jan

> ---
>  .../firmware/etherboot/patches/ipxe-ath9k-gcc5.patch | 20 
> 
>  .../etherboot/patches/ipxe-ath9k2-gcc5.patch | 20 
> 
>  tools/firmware/etherboot/patches/series  |  2 ++
>  3 files changed, 42 insertions(+)
>  create mode 100644 tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
>  create mode 100644 tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch
> 
> diff --git a/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch 
> b/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
> new file mode 100644
> index 000..722f5e1
> --- /dev/null
> +++ b/tools/firmware/etherboot/patches/ipxe-ath9k-gcc5.patch
> @@ -0,0 +1,20 @@
> +--- ipxe.orig/src/drivers/net/ath/ath9k/ath9k_ar9003_phy.c   2011-12-10 
> 18:28:04.0 -0800
>  ipxe/src/drivers/net/ath/ath9k/ath9k_ar9003_phy.c2015-09-18 
> 14:42:38.618507334 -0700
> +@@ -859,7 +859,7 @@
> + REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
> + AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
> + 
> +-if (!on != aniState->ofdmWeakSigDetectOff) {
> ++if ((!on) != aniState->ofdmWeakSigDetectOff) {
> + DBG2("ath9k: "
> + "** ch %d: ofdm weak signal: %s=>%s\n",
> + chan->channel,
> +@@ -1013,7 +1013,7 @@
> +   AR_PHY_MRC_CCK_ENABLE, is_on);
> + REG_RMW_FIELD(ah, AR_PHY_MRC_CCK_CTRL,
> +   AR_PHY_MRC_CCK_MUX_REG, is_on);
> +-if (!is_on != aniState->mrcCCKOff) {
> ++if ((!is_on) != aniState->mrcCCKOff) {
> + DBG2("ath9k: "
> + "** ch %d: MRC CCK: %s=>%s\n",
> + chan->channel,
> diff --git a/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch 
> b/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch
> new file mode 100644
> index 000..a96bb51
> --- /dev/null
> +++ b/tools/firmware/etherboot/patches/ipxe-ath9k2-gcc5.patch
> @@ -0,0 +1,20 @@
> +--- ipxe.orig/src/drivers/net/ath/ath9k/ath9k_ar5008_phy.c   2011-12-10 
> 18:28:04.0 -0800
>  ipxe/src/drivers/net/ath/ath9k/ath9k_ar5008_phy.c2015-09-18 
> 14:48:17.493922456 -0700
> +@@ -1141,7 +1141,7 @@
> + REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
> + AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
> + 
> +-if (!on != aniState->ofdmWeakSigDetectOff) {
> ++if ((!on) != aniState->ofdmWeakSigDetectOff) {
> + if (on)
> + ah->stats.ast_ani_ofdmon++;
> + else
> +@@ -1307,7 +1307,7 @@
> + REG_CLR_BIT(ah, AR_PHY_SFCORR_LOW,
> + AR_PHY_SFCORR_LOW_USE_SELF_CORR_LOW);
> + 
> +-if (!on != aniState->ofdmWeakSigDetectOff) {
> ++if ((!on) != aniState->ofdmWeakSigDetectOff) {
> + DBG2("ath9k: "
> + "** ch %d: ofdm weak signal: %s=>%s\n",
> + chan->channel,
> diff --git a/tools/firmware/etherboot/patches/series 
> b/tools/firmware/etherboot/patches/series
> index 2c39853..679e0f9 100644
> --- a/tools/firmware/etherboot/patches/series
> +++ b/tools/firmware/etherboot/patches/series
> @@ -4,3 +4,5 @@ build_fix_2.patch
>  build_fix_3.patch
>  build-compare.patch
>  build_fix_4.patch
> +ipxe-ath9k-gcc5.patch 
> +ipxe-ath9k2-gcc5.patch
> -- 
> 2.1.4
> 
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 




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

Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Xu, Quan]

>> >>> On 09.10.2015 at 15:18  wrote:
> >>> On 09.10.15 at 09:06,  wrote:
> >> > >>>On 08.10.2015 at 16:52  wrote:
> >> >>> On 07.10.15 at 19:02,  wrote:
 
> >> > __scheme B__
> >> > Q1: - when to take the references?
> >> >
> >> > take the reference when the IOMMU entry is _
> removed/overwritten_;
> >> > in detail:
> >> >  --iommu_unmap_page(), or
> >> >  --ept_set_entry() [Once IOMMU shares EPT page table.]
> >> >
> >> > * Make sure IOMMU page should not be reallocated for
> >> >  another purpose until the appropriate invalidations have been
> >> >  performed.
> >> > * in this case, it does not matter hot-plug ATS device
> >> > pass-through or ATS device assigned in domain initialization.
> >> >
> >> > Q2 / Q3:
> >> > The same as above __scheme A__ Q2/Q3.
> >> >
> >> > One question: is __scheme B__ safe? If it is safe, I prefer __scheme 
> >> > B__..
> >>
> >> While at the first glance this looks like a neat idea -
> >
> >
> > I think this is safe and a good solution.
> > I hope you can review into the __scheme B__. I need _Acked-by_ you and
> > Tim Deegan.
> 
> What do you mean here?

Just verify it.
If it is working, I continue to write patch based on it.
If it is not working, I continue to research ..


> I'm not going to ack a patch that hasn't even got
> written, and while scheme B looks possible, I might still overlook something, 
> so I
> also can't up front ack that model (which may then lead to you expecting that
> once implemented it gets accepted).


I am getting started to write patch based on the __scheme B__ and send out ASAP 
.
Thanks 

Quan


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

Re: [Xen-devel] [PATCH v2 1/2] x86/mem-sharing: Bulk mem-sharing entire domains

[Jan Beulich]

>>> On 08.10.15 at 22:57,  wrote:
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1293,6 +1293,37 @@ int relinquish_shared_pages(struct domain *d)
>  return rc;
>  }
>  
> +static int bulk_share(struct domain *d, struct domain *cd, unsigned long max,
> +  struct mem_sharing_op_bulk_share *bulk)
> +{
> +int rc = 0;
> +shr_handle_t sh, ch;
> +
> +while( bulk->start <= max )
> +{
> +if ( mem_sharing_nominate_page(d, bulk->start, 0, ) != 0 )
> +goto next;
> +
> +if ( mem_sharing_nominate_page(cd, bulk->start, 0, ) != 0 )
> +goto next;
> +
> +if ( !mem_sharing_share_pages(d, bulk->start, sh, cd, bulk->start, 
> ch) )
> +++(bulk->shared);

Pointless parentheses.

> +next:

Labels indented by at least one space please.

> +++(bulk->start);
> +
> +/* Check for continuation if it's not the last iteration. */
> +if ( bulk->start < max && hypercall_preempt_check() )
> +{
> +rc = 1;
> +break;

This could simple be a return statement, avoiding the need for a
local variable (the type of which would need to be changed, see
below).

> +}
> +}
> +
> +return rc;
> +}

So effectively the function right now returns a boolean. If that's
intended, its return type should reflect that (but I then wonder
whether the sense of the values shouldn't be inverted, to have
"true" mean "done").

> @@ -1467,6 +1498,59 @@ int 
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>  }
>  break;
>  
> +case XENMEM_sharing_op_bulk_share:
> +{
> +unsigned long max_sgfn, max_cgfn;
> +struct domain *cd;
> +
> +rc = -EINVAL;
> +if ( !mem_sharing_enabled(d) )
> +goto out;
> +
> +rc = rcu_lock_live_remote_domain_by_id(mso.u.bulk.client_domain,
> +   );
> +if ( rc )
> +goto out;
> +
> +rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mso.op);
> +if ( rc )
> +{
> +rcu_unlock_domain(cd);
> +goto out;
> +}
> +
> +if ( !mem_sharing_enabled(cd) )
> +{
> +rcu_unlock_domain(cd);
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +max_sgfn = domain_get_maximum_gpfn(d);
> +max_cgfn = domain_get_maximum_gpfn(cd);
> +
> +if ( max_sgfn != max_cgfn || max_sgfn < mso.u.bulk.start )

Are both domains required to be paused for this operation? If so,
shouldn't you enforce this? If not, by the time you reach the if()
the values are stale.

> +{
> +rcu_unlock_domain(cd);
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +rc = bulk_share(d, cd, max_sgfn, );
> +if ( rc )

With the boolean nature the use of "rc" here is rather confusing;
I'd suggest avoiding the use of in intermediate variable in this case.

> +{
> +if ( __copy_to_guest(arg, , 1) )
> +rc = -EFAULT;
> +else
> +rc = 
> hypercall_create_continuation(__HYPERVISOR_memory_op,
> +   "lh", 
> XENMEM_sharing_op,
> +   arg);
> +}
> +
> +rcu_unlock_domain(cd);
> +}
> +break;
> +
>  case XENMEM_sharing_op_debug_gfn:
>  {
>  unsigned long gfn = mso.u.debug.u.gfn;
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 320de91..0299746 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -447,6 +447,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t);
>  #define XENMEM_sharing_op_debug_gref5
>  #define XENMEM_sharing_op_add_physmap   6
>  #define XENMEM_sharing_op_audit 7
> +#define XENMEM_sharing_op_bulk_share8
>  
>  #define XENMEM_SHARING_OP_S_HANDLE_INVALID  (-10)
>  #define XENMEM_SHARING_OP_C_HANDLE_INVALID  (-9)
> @@ -482,7 +483,13 @@ struct xen_mem_sharing_op {
>  uint64_aligned_t client_gfn;/* IN: the client gfn */
>  uint64_aligned_t client_handle; /* IN: handle to the client page 
> */
>  domid_t  client_domain; /* IN: the client domain id */
> -} share; 
> +} share;
> +struct mem_sharing_op_bulk_share {   /* OP_BULK_SHARE */

Is the _share suffix really useful here? Even more, if you dropped
it and renamed "shared" below to "done", the same structure could
be used for a hypothetical bulk-unshare operation.

> +domid_t client_domain;   /* IN: the client domain id */
> +uint64_aligned_t 

Re: [Xen-devel] [PATCH] VT-d: don't suppress invalidation address write when it is zero

[Zhang, Yang Z]

Jan Beulich wrote on 2015-09-28:
> GFN zero is a valid address, and hence may need invalidation done for 
> it just like for any other GFN.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Yang Zhang 

> ---
> The comment right before the respective dmar_writeq() confuses me:
> What "first" TLB reg does it talk about? Is it simply stale (albeit it 
> has been there already in 3.2.x, i.e. it would then likely have been 
> stale already at the time that code got added)?

I have no idea about the 'first TLB' and didn't find any clue from spec.

> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -414,7 +414,7 @@ static int flush_iotlb_reg(void *_iommu,  {
>  struct iommu *iommu = (struct iommu *) _iommu;
>  int tlb_offset = ecap_iotlb_offset(iommu->ecap);
> -u64 val = 0, val_iva = 0;
> +u64 val = 0;
>  unsigned long flags;
>  
>  /* @@ -435,7 +435,6 @@ static int flush_iotlb_reg(void *_iommu,
>  switch ( type ) { case DMA_TLB_GLOBAL_FLUSH:
> -/* global flush doesn't need set IVA_REG */
>  val = DMA_TLB_GLOBAL_FLUSH|DMA_TLB_IVT;
>  break;
>  case DMA_TLB_DSI_FLUSH:
> @@ -443,8 +442,6 @@ static int flush_iotlb_reg(void *_iommu,
>  break; case DMA_TLB_PSI_FLUSH: val =
>  DMA_TLB_PSI_FLUSH|DMA_TLB_IVT|DMA_TLB_DID(did);
> -/* Note: always flush non-leaf currently */
> -val_iva = size_order | addr;
>  break; default: BUG();
> @@ -457,8 +454,11 @@ static int flush_iotlb_reg(void *_iommu,
> 
>  spin_lock_irqsave(>register_lock, flags);
>  /* Note: Only uses first TLB reg currently */
> -if ( val_iva )
> -dmar_writeq(iommu->reg, tlb_offset, val_iva);
> +if ( type == DMA_TLB_PSI_FLUSH )
> +{
> +/* Note: always flush non-leaf currently. */
> +dmar_writeq(iommu->reg, tlb_offset, size_order | addr);
> +}
>  dmar_writeq(iommu->reg, tlb_offset + 8, val);
>  
>  /* Make sure hardware complete it */
>


Best regards,
Yang



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

[Xen-devel] [xen-4.4-testing test] 62730: trouble: blocked/broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-raw   3 host-install(3) broken REGR. vs. 62700

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-multivcpu 15 guest-start.2fail  like 62616
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 62616
 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-localmigrate  fail like 62665

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-migrupgrade  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 build-i386-rumpuserxen6 xen-buildfail   never pass
 build-amd64-prev  5 xen-buildfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 build-i386-prev   5 xen-buildfail   never pass
 test-armhf-armhf-libvirt 11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-xend-qemut-winxpsp3 21 leak-check/checkfail never pass

version targeted for testing:
 xen  5c94f9630bf735f19df51c61817cfc6a3aebc994
baseline version:
 xen  4d99a76cfeba6d23504121a51e7750f230128d85

Last test of basis62700  2015-10-06 14:07:25 Z3 days
Testing same since62730  2015-10-08 11:12:57 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Daniel De Graaf 
  Dario Faggioli 
  Jan Beulich 
  Konrad Rzeszutek Wilk 
  Kouya Shimura 
  Quan Xu 
  Yang Zhang 

jobs:
 build-amd64-xend pass
 build-i386-xend  pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev fail
 build-i386-prev  fail
 build-amd64-pvopspass
 build-armhf-pvopspass
 

Re: [Xen-devel] [RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Campbell]

On Fri, 2015-10-09 at 17:06 +0100, Ian Jackson wrote:
> 
[...]

Agree with that analysis, thanks.

> In any case, I have tried this several times both with and without
> eatmydata and it seems to work fine for me.

Me too, today.

> I may prepare a patch to make standalone-generate-dump-flight-runvars
> handle ^C properly.

That would be nice.

I shall send a v3 without the RFC stuff in a moment.

Ian.

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Boris Ostrovsky]

On 10/09/2015 12:19 PM, Jan Beulich wrote:

On 09.10.15 at 18:09,  wrote:

On 10/09/2015 11:11 AM, Jan Beulich wrote:

On 09.10.15 at 16:00,  wrote:

On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:

On 10/09/2015 02:51 AM, Jan Beulich wrote:

On 28.09.15 at 09:13,  wrote:

When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
the host TSC with a ratio between guest TSC rate and
nanoseconds. However, the result will be incorrect if the guest TSC rate
differs from the host TSC rate. This patch fixes this problem by using
the system time as elapsed_nsec.

For both this and patch 2, while at a first glance (and taking into
account just the visible patch context) what you say seems to
make sense, the explanation is far from sufficient namely when
looking at the function as a whole. For one, effects on existing
cases need to be explicitly described, in particular why SVM's TSC
ratio code works without that change (or whether it has been
broken all along, in which case these would become backporting
candidates; input from SVM maintainers would be appreciated
too). That may in particular mean being more specific about
what is actually wrong with scaling the host TSC here (i.e. in
which way both results differ), when supposedly that matches
what the hardware does when TSC ratio is supported.

If elapsed_nsec is the time that guest has been running then how can
get_s_time(), which is system time, be the right answer here? But what
confuses me even more is that existing code is not doing that neither.

Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
I.e.

*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?


Yes, I should minus d->arch.vtsc_offset here.

In which case - afaict - the code becomes identical to that of the
TSC_MODE_ALWAYS_EMULATE case as well as the
TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
unlikely to be correct.

*elapsed_nsec = *gtsc_khz = 0; ? Because we are effectively in
TSC_MODE_NEVER.

How that? Talk here has been about TSC_MODE_DEFAULT...


AFAIUI, TSC_MODE_DEFAULT is a shorthand for saying "I will let the 
hypervisor pick whether the guest will be in TSC_MODE_ALWAYS_EMULATE or 
TSC_MODE_NEVER". d->arch.vtsc is what ends up being internal 
implementation of user-provided mode (for the most parts; I think 
hvm_cpuid() being the only true exception --- and perhaps it needs to be 
looked at).


So if we have d->arch.vtsc=0 (which is the case we are talking about 
here) then we are really in NEVER mode



-boris




That can't be right...

Why not? tsc_set_info() doesn't care about any of its other input
values when that mode is in effect.

Jan




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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Haozhong Zhang]

On Fri, Oct 09, 2015 at 11:37:06AM -0400, Boris Ostrovsky wrote:
> On 10/09/2015 10:39 AM, Jan Beulich wrote:
> On 09.10.15 at 15:41,  wrote:
> >>On 10/09/2015 02:51 AM, Jan Beulich wrote:
> >>On 28.09.15 at 09:13,  wrote:
> When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
> is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
> the host TSC with a ratio between guest TSC rate and
> nanoseconds. However, the result will be incorrect if the guest TSC rate
> differs from the host TSC rate. This patch fixes this problem by using
> the system time as elapsed_nsec.
> >>>For both this and patch 2, while at a first glance (and taking into
> >>>account just the visible patch context) what you say seems to
> >>>make sense, the explanation is far from sufficient namely when
> >>>looking at the function as a whole. For one, effects on existing
> >>>cases need to be explicitly described, in particular why SVM's TSC
> >>>ratio code works without that change (or whether it has been
> >>>broken all along, in which case these would become backporting
> >>>candidates; input from SVM maintainers would be appreciated
> >>>too). That may in particular mean being more specific about
> >>>what is actually wrong with scaling the host TSC here (i.e. in
> >>>which way both results differ), when supposedly that matches
> >>>what the hardware does when TSC ratio is supported.
> >>If elapsed_nsec is the time that guest has been running then how can
> >>get_s_time(), which is system time, be the right answer here? But what
> >>confuses me even more is that existing code is not doing that neither.
> >>
> >>Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
> >>I.e.
> >>
> >>*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?
> >Doesn't whether or not to adjust be the offset depend on d-arch.vtsc?
> 
> We only use elapsed_nsec when vtsc is set, I think. In native case (vtsc=0)
> elapsed_nsec and d->arch.vtsc_offset are ignored.
>

But it is used in tsc_set_info() if a HVM domain in TSC_MODE_DEFAULT
is migrated to a machine and the following if condition in
tsc_set_info() is false.

if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
 (has_hvm_container_domain(d) ?
  d->arch.tsc_khz == cpu_khz || cpu_has_tsc_ratio :
  incarnation == 0) )

- Haozhong

> -boris
> 
> 

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

Re: [Xen-devel] [RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Jackson]

Ian Campbell writes ("[RFC OSSTEST v2] ap-fetch-*: Support 
$AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> Still RFC because of these sqlite errors
> 
> DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight, 
> jobs.job [for Statement "INSERT INTO jobs VALUES 
> (?,'build-i386-xsm','build','queued')

After having tried to repro this without success, and discussed
various implausible theories, we think this error is due to the
following:

You ran ./standalone-generate-dump-flight-runvars but then realised
you wanted to restart it.  So you pressed ^C.  But this does not
actually kill all the children (this is indeed a problem with this
script).  You then reran the script.  Now it is racing with other
copies of make-flight, cs-job-create, etc., which can create the same
jobs in the same flights as your current run.

> DBD::SQLite::db do failed: database is locked [for Statement " DELETE 
> FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.
> DBD::SQLite::db do failed: database is locked [for Statement " DELETE 
> FROM runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.

And we think this is while you were trying to solve these problems by
setting the timeout, which you say you tried setting to zero in the
belief (justified by documentation but not apparently true) that that
means `infinite'.

> Which consistently take out the use of
> standalone-generate-dump-flight-runvars with this patch. I think
> probably because ap-fetch-* now complete instantly which makes the
> standalone-generate-dump-flight-runvars far more thunderous on the
> DB.

In any case, I have tried this several times both with and without
eatmydata and it seems to work fine for me.

I may prepare a patch to make standalone-generate-dump-flight-runvars
handle ^C properly.

Ian.

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Boris Ostrovsky]

On 10/09/2015 11:11 AM, Jan Beulich wrote:

On 09.10.15 at 16:00,  wrote:

On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:

On 10/09/2015 02:51 AM, Jan Beulich wrote:

On 28.09.15 at 09:13,  wrote:

When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
the host TSC with a ratio between guest TSC rate and
nanoseconds. However, the result will be incorrect if the guest TSC rate
differs from the host TSC rate. This patch fixes this problem by using
the system time as elapsed_nsec.

For both this and patch 2, while at a first glance (and taking into
account just the visible patch context) what you say seems to
make sense, the explanation is far from sufficient namely when
looking at the function as a whole. For one, effects on existing
cases need to be explicitly described, in particular why SVM's TSC
ratio code works without that change (or whether it has been
broken all along, in which case these would become backporting
candidates; input from SVM maintainers would be appreciated
too). That may in particular mean being more specific about
what is actually wrong with scaling the host TSC here (i.e. in
which way both results differ), when supposedly that matches
what the hardware does when TSC ratio is supported.

If elapsed_nsec is the time that guest has been running then how can
get_s_time(), which is system time, be the right answer here? But what
confuses me even more is that existing code is not doing that neither.

Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
I.e.

*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?


Yes, I should minus d->arch.vtsc_offset here.

In which case - afaict - the code becomes identical to that of the
TSC_MODE_ALWAYS_EMULATE case as well as the
TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
unlikely to be correct.


*elapsed_nsec = *gtsc_khz = 0; ? Because we are effectively in 
TSC_MODE_NEVER.


That can't be right...


-boris


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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Boris Ostrovsky]

On 10/09/2015 12:39 PM, Haozhong Zhang wrote:

On Fri, Oct 09, 2015 at 11:37:06AM -0400, Boris Ostrovsky wrote:

On 10/09/2015 10:39 AM, Jan Beulich wrote:

On 09.10.15 at 15:41,  wrote:

On 10/09/2015 02:51 AM, Jan Beulich wrote:

On 28.09.15 at 09:13,  wrote:

When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
the host TSC with a ratio between guest TSC rate and
nanoseconds. However, the result will be incorrect if the guest TSC rate
differs from the host TSC rate. This patch fixes this problem by using
the system time as elapsed_nsec.

For both this and patch 2, while at a first glance (and taking into
account just the visible patch context) what you say seems to
make sense, the explanation is far from sufficient namely when
looking at the function as a whole. For one, effects on existing
cases need to be explicitly described, in particular why SVM's TSC
ratio code works without that change (or whether it has been
broken all along, in which case these would become backporting
candidates; input from SVM maintainers would be appreciated
too). That may in particular mean being more specific about
what is actually wrong with scaling the host TSC here (i.e. in
which way both results differ), when supposedly that matches
what the hardware does when TSC ratio is supported.

If elapsed_nsec is the time that guest has been running then how can
get_s_time(), which is system time, be the right answer here? But what
confuses me even more is that existing code is not doing that neither.

Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
I.e.

*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?

Doesn't whether or not to adjust be the offset depend on d-arch.vtsc?

We only use elapsed_nsec when vtsc is set, I think. In native case (vtsc=0)
elapsed_nsec and d->arch.vtsc_offset are ignored.


But it is used in tsc_set_info() if a HVM domain in TSC_MODE_DEFAULT
is migrated to a machine and the following if condition in
tsc_set_info() is false.

if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
  (has_hvm_container_domain(d) ?
   d->arch.tsc_khz == cpu_khz || cpu_has_tsc_ratio :
   incarnation == 0) )



Ah, yes, then we do need to save it.

-boris


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

[Xen-devel] [linux-mingo-tip-master test] 62728: regressions - FAIL

[osstest service owner]

flight 62728 linux-mingo-tip-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/62728/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvh-intel  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-pvh-amd   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-pair10 xen-boot/dst_host fail REGR. vs. 60684
 test-amd64-amd64-pair 9 xen-boot/src_host fail REGR. vs. 60684
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail REGR. 
vs. 60684
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 60684
 test-amd64-amd64-xl-xsm   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-rumpuserxen-amd64  6 xen-bootfail REGR. vs. 60684
 test-amd64-amd64-xl-multivcpu  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-credit2   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-amd64-pvgrub  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-qcow2 6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-i386-pvgrub  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 60684
 test-amd64-amd64-xl-vhd   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-pygrub   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-raw   6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail REGR. vs. 60684
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-bootfail REGR. vs. 60684
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-win7-amd64  6 xen-boot  fail REGR. vs. 60684

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-libvirt  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-libvirt-xsm  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-libvirt-pair 10 xen-boot/dst_hostfail REGR. vs. 60684
 test-amd64-amd64-libvirt-pair  9 xen-boot/src_hostfail REGR. vs. 60684
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
60684
 test-amd64-amd64-libvirt-raw  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-libvirt-qcow2  6 xen-bootfail REGR. vs. 60684
 test-amd64-amd64-libvirt-vhd  6 xen-boot  fail REGR. vs. 60684
 test-amd64-amd64-xl-rtds  6 xen-boot  fail REGR. vs. 60684

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass

version targeted for testing:
 linuxf36a249c0c0dec3f0849026701e7a9811357dd2d
baseline version:
 linux69f75ebe3b1d1e636c4ce0a0ee248edacc69cbe0

Last test of basis60684  2015-08-13 04:21:46 Z   57 days
Failing since 60712  2015-08-15 18:33:48 Z   54 days   27 attempts
Testing same since62728  2015-10-08 10:48:01 Z1 days1 attempts

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
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  fail
 test-amd64-i386-xl 

[Xen-devel] [PATCH OSSTEST v3] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Campbell]

And use this in standalone-generate-dump-flight-runvars. In general I
don't think we are interested in the specific revision_* runvars when
using this tool but when it matters this new behaviour can be avoided
by setting AP_FETCH_PLACEHOLDERS=n.

This is quicker even than using memoisation on the ap-fetch
invocations and produces output like:

libvirt  build-amd64  revision_xen  ap-fetch-version-baseline:xen-unstable

This is useful when doing comparisons of before and after changes to
e.g. make-flight since they do not pickup noise if a something/someone
does a push in the middle.

The memoisation bits of standalone-generate-dump-flight-runvars are
disabled if AP_FETCH_PLACEHOLDERS=y.

Signed-off-by: Ian Campbell 
---
v3:
  - No longer RFC. Races explained to our satisfaction.
  - Shortened the line length in the example output.
  - Tweaked some wording in the commit message.

v2:
  - Fix typo which would activate this mode unless
$AP_FETCH_PLACEHOLDERS=x.
  - Require $AP_FETCH_PLACEHOLDERS=y (not just non-empty) and update
standalone-generate-runvars to only set AP_FETCH_PLACEHOLDERS=y if
it is unset.
  - Drop sqlite_use_immediate_transaction => 0,
  - Only memoize if not using placeholders (in particular don't blow
away the cache)
---
 ap-common   |  9 +
 ap-fetch-version|  2 ++
 ap-fetch-version-baseline   |  3 +++
 ap-fetch-version-baseline-late  |  2 ++
 ap-fetch-version-old|  2 ++
 standalone-generate-dump-flight-runvars | 10 --
 6 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/ap-common b/ap-common
index 91425a9..5b6e088 100644
--- a/ap-common
+++ b/ap-common
@@ -145,3 +145,12 @@ info_linux_tree () {
 
return 0
 }
+
+check_ap_fetch_placeholders () {
+   if [ "x$AP_FETCH_PLACEHOLDERS" != xy ] ; then
+   return 0
+   fi
+
+   echo "$(basename $0):$branch"
+   exit 0
+}
diff --git a/ap-fetch-version b/ap-fetch-version
index 6fa7588..f884bd3 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 if info_linux_tree "$branch"; then
repo_tree_rev_fetch_git linux \
$TREE_LINUX_THIS $TAG_LINUX_THIS $LOCALREV_LINUX
diff --git a/ap-fetch-version-baseline b/ap-fetch-version-baseline
index 2e42508..c9da82c 100755
--- a/ap-fetch-version-baseline
+++ b/ap-fetch-version-baseline
@@ -22,6 +22,9 @@ set -e -o posix
 branch=$1
 
 . ./cri-lock-repos
+. ./ap-common
+
+check_ap_fetch_placeholders
 
 : ${BASE_TREE_LINUX:=git://xenbits.xen.org/people/ianc/linux-2.6.git}
 : ${BASE_TAG_LINUX:=xen/next-2.6.32}
diff --git a/ap-fetch-version-baseline-late b/ap-fetch-version-baseline-late
index 9856ec9..dff8b05 100755
--- a/ap-fetch-version-baseline-late
+++ b/ap-fetch-version-baseline-late
@@ -27,6 +27,8 @@ new=$2
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 case "$branch" in
 
 linux-next)
diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 66d51f8..99f276a 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 : ${BASE_TAG_LINUX2639:=tested/2.6.39.x}
 : ${BASE_LOCALREV_LINUX:=daily-cron.$branch.old}
 : ${BASE_LOCALREV_LIBVIRT:=daily-cron.$branch.old}
diff --git a/standalone-generate-dump-flight-runvars 
b/standalone-generate-dump-flight-runvars
index d113927..a1907b0 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -36,11 +36,17 @@ if [ $# = 0 ]; then
set `./mg-list-all-branches`
 fi
 
-if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
+: ${AP_FETCH_PLACEHOLDERS:=y}
+export AP_FETCH_PLACEHOLDERS
+
+
+if [ "x$AP_FETCH_PLACEHOLDERS" != xy ]; then
+if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
rm -rf tmp/apmemo
mkdir tmp/apmemo
+fi
+export AP_FETCH_PFX='./memoise tmp/apmemo'
 fi
-export AP_FETCH_PFX='./memoise tmp/apmemo'
 
 # In the future it might be nice for this script to arrange to use a
 # separate standalone.db, in tmp/ probably, for each different branch.
-- 
2.5.3


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

Re: [Xen-devel] [PATCH v1] Add build-id to XENVER hypercall.

[Andrew Cooper]

On 09/10/15 09:17, Jan Beulich wrote:
 On 09.10.15 at 04:56,  wrote:
>> However they also change the behavior of the existing hypercall
>> for XENVER_[compile_info|changeset|commandline] and make them
>> dom0 accessible. This is if XSM is built in or not (though with
>> XSM one can expose it to a guest if desired).
> Wasn't the outcome of the previous discussion that we should not
> alter default behavior for existing sub-ops?

I raised a worry that some guests might break if they suddenly have
access to this information cut off.

> And even if I'm
> misremembering, I can see reasons for not exposing the command
> line, but what value has not exposing compile info and changeset
> again?

There is a fear that providing such information makes it easier for
attackers who have an exploit for a specific binary.

> The more that the tool stack uses the two, and as we know
> tool stacks or parts thereof can live in unprivileged domains.

I would argue than a fully unprivileged toolstack domain has no need for
any information from this hypercall.  It it needs some privilege, then
XSM is in use and it can be given access.

> Plus
> there is also a (hg-centric and hence generally broken) attempt to
> store it in hvm_save().

I will be addressing this in due course with my further cpuid plans.

~Andrew

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

Re: [Xen-devel] [PATCH v1 3/4] XENVER_build_id: Provide ld-embedded build-ids

[Andrew Cooper]

On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
> @@ -367,6 +368,35 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> arg,
>  if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
>  return -EFAULT;
>  return 0;
> +
> +case XENVER_build_id:
> +{
> +int rc;
> +char *p = NULL;
> +unsigned int sz = 0;
> +
> +if ( guest_handle_is_null(arg) )
> +return -EINVAL;

A NULL guest handle should return size, in the same way that we have
size queries with other hypercalls.  In the future, build-id will
probably extend to sha256 and get longer as a result.

> +
> +if ( len == 0 )
> +return -EINVAL;

This check is redundant with the "sz > len" check below.

> +
> +if ( !guest_handle_okay(arg, len) )
> +return -EINVAL;

This check is performed by copy_to_guest() below.

> +
> +rc = xen_build_id(, );
> +if ( rc )
> +return rc;
> +
> +if ( sz > len )
> +return -ENOMEM;

ENOBUFS

> +
> +if ( copy_to_guest(arg, p, sz) )
> +return -EFAULT;
> +
> +return sz;
> +}
> +
>  }
>  
>  return -ENOSYS;
> diff --git a/xen/common/version.c b/xen/common/version.c
> index b152e27..26eeadf 100644
> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -1,5 +1,9 @@
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  const char *xen_compile_date(void)
>  {
> @@ -55,3 +59,36 @@ const char *xen_banner(void)
>  {
>  return XEN_BANNER;
>  }
> +
> +#ifdef CONFIG_ARM
> +int xen_build_id(char **p, unsigned int *len)
> +{
> +return -ENODATA;
> +}
> +#else
> +#define NT_GNU_BUILD_ID 3
> +
> +extern const Elf_Note __note_gnu_build_id_start;  /* Defined in linker 
> script. */

extern const Elf_Note __note_gnu_build_id_start[],
__note_gnu_build_id_end[];

> +extern const char __note_gnu_build_id_end[];
> +int xen_build_id(char **p, unsigned int *len)
> +{
> +const Elf_Note *n = &__note_gnu_build_id_start;

const Elf_Note *n = __note_gnu_build_id_start;

> +
> +/* Something is wrong. */
> +if ( __note_gnu_build_id_end <= (char *)&__note_gnu_build_id_start )

Need to check for a full Note header as well.

if ( [1] > __note_gnu_build_id_end )

> +return -ENODATA;
> +
> +/* Check if we really have a build-id. */
> +if ( NT_GNU_BUILD_ID != n->type )
> +return -ENODATA;
> +
> +/* Sanity check, name should be "GNU" for ld-generated build-id. */
> +if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
> +return -ENODATA;
> +
> +*len = n->descsz;
> +*p = ELFNOTE_DESC(n);

This information could be cached in a couple of static variables, so the
sanity checks are only performed once.

> +
> +return 0;
> +}
> +#endif
> diff --git a/xen/include/public/version.h b/xen/include/public/version.h
> index 44f26b0..e575d6b 100644
> --- a/xen/include/public/version.h
> +++ b/xen/include/public/version.h
> @@ -30,7 +30,8 @@
>  
>  #include "xen.h"
>  
> -/* NB. All ops return zero on success, except XENVER_{version,pagesize} */
> +/* NB. All ops return zero on success, except
> + * XENVER_{version,pagesize, build_id} */
>  
>  /* arg == NULL; returns major:minor (16:16). */
>  #define XENVER_version  0
> @@ -83,6 +84,12 @@ typedef struct xen_feature_info xen_feature_info_t;
>  #define XENVER_commandline 9
>  typedef char xen_commandline_t[1024];
>  
> +#define XENVER_build_id 10
> +/*
> + * arg1 == pointer to char array, arg2 == size of char array.
> + * Return value is the actual size.

Return value is the number of bytes written, or -ve error.

~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Ian Campbell]

On Fri, 2015-10-09 at 13:29 +0100, Andrew Cooper wrote:
> On 09/10/15 09:25, Jan Beulich wrote:
> > > > > On 09.10.15 at 04:56,  wrote:
> > > All existing commands ignore the parameter so this does
> > > not break the ABI.
> > Does it not? What about the debug mode clobbering of hypercall
> > argument registers?
> 
> That is an implementation detail of the hypervisor.  It is irrelevant to
> guests whether Xen chooses to clobber the spare registers or not.

Or in other words the effect here is to clobber one _less_ register, and
the guest cannot have been relying on a register getting so clobbered (if
nothing else it doesn't happen in debug=n builds).

The flip side is that we are now no longer clobbering that register even
for existing sub-ops which do not use it (since the clobbering doesn't go
down to the subop level). So there is a risk that a guest may come to
depend on that register not being clobbered and then fail older debug=y
hypervisors.

This second scenario doesn't seem especially likely to me.

Do we not already have one or two hypercalls where subops consume different
numbers of parameters anyway? HYPERVISOR_sched_op I think has this property
and we've not been too concerned.

Ian.

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

Re: [Xen-devel] [PATCH v1] xSplice initial foundation patches.

[Konrad Rzeszutek Wilk]

On Fri, Oct 02, 2015 at 10:48:54AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 16, 2015 at 05:01:11PM -0400, Konrad Rzeszutek Wilk wrote:
> > 
> > *OK, what do you have?*
> > 
> > They are located at a git tree:
> >   git://xenbits.xen.org/people/konradwilk/xen.git xsplice.v1
> 
> I've created another branch 'xsplice.v1.1' which has some bug-fixes
> and improvements. Will be posting it soonish when I am done
> with testing.
> 
> But more importantly I would like to setup an IRC meeting
> so that we may coordinate.
> 
> I've used doodle, and the link is:
> 
>  http://doodle.com/poll/adtuwr4hs5m4i9y6
> 
> The times I've put in there are geared towards earlier hours
> but please feel free to propose other times.

The one time that works for the interested parties is 10AM EST
on the fourth Monday of the month. As such we will have an
meeting on #xsplice on irc.freenode.net on that day.

Thank you

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

Re: [Xen-devel] [PATCH 4.5] ipxe fix for gcc5

[Wei Liu]

On Thu, Oct 08, 2015 at 10:25:42AM -0700, Mark Pryor wrote:
> Signed-off-by: Mark Pryor 

There is already a fix in tree for 4.6. You can request backporting

commit 6596412d59bcde3d1a2473f341851f4c476fc9df
Author: Konrad Rzeszutek Wilk 
Date:   Mon Aug 24 15:48:58 2015 -0400

etherboot: Build fix for GCC 5.1.1

Specificially we are pulling in the upstream patch (commit
1b56452121672e6408c38ac8926bdd6998a39004)):
[ath9k] Remove confusing logic inversion in an ANI variable

Signed-off-by: Konrad Rzeszutek Wilk 
Acked-by: Wei Liu 

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

Re: [Xen-devel] [qemu-mainline test] 62725: trouble: broken/fail/pass

[Ian Campbell]

On Fri, 2015-10-09 at 10:33 +, osstest service owner wrote:
> flight 62725 qemu-mainline real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/62725/
> 
> Failures and problems with tests :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-amd64-xl-multivcpu  3 host-install(3)broken REGR. vs. 
> 62649
>  test-amd64-amd64-xl-xsm   3 host-install(3) broken REGR. vs. 
> 62649
>  test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken REGR. 
> vs. 62649
>  test-amd64-amd64-amd64-pvgrub  3 host-install(3)broken REGR. vs. 
> 62649
>  test-amd64-i386-freebsd10-i386  3 host-install(3)   broken REGR. vs. 
> 62649
>  test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 3 host-install(3) broken REGR. vs. 
> 62649

These all look like elbling0 forgetting its boot order.

> 
> Regressions which are regarded as allowable (not blocking):
>  test-amd64-i386-libvirt   3 host-install(3) broken REGR. vs. 
> 62649
>  test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken 
> REGR. vs. 62649
>  test-amd64-i386-libvirt-qcow2  3 host-install(3)broken REGR. vs. 
> 62649
>  test-amd64-i386-libvirt-vhd   3 host-install(3) broken REGR. vs. 
> 62649

So do these.


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

[Xen-devel] Intention to deprecate and remove some Python libxc bindings

[Juergen Gross]

Mostly for historical reasons Xen includes Python bindings for libxc.
They have been used by xm/xend in the past but nowadays there is only
one usage in the Xen source tree: pygrub is using xc.xeninfo().

I wrote a patch to remove all libxc Python bindings but xc.xeninfo() and
got some feedback regarding out-of-tree users. Up to now there have been
reports of users of the following bindings:

xc.getcpuinfo()
xc.domain_getinfo()
xc.tmem_control()
xc.domain_set_target_mem()
xc.domain_setmaxmem()
xc.physinfo()
xc.xeninfo()

Before removing all but above bindings I'd like to ask for any other
users of the libxc Python bindings. In case you are using one of those
and are planning to do so with Xen 4.7 and later, please stand up and
tell me, in order to keep the specific binding.

Just for reference: There are no known users of:

xc.domain_create()
xc.domain_max_vcpus()
xc.domain_dumpcore()
xc.domain_pause()
xc.domain_unpause()
xc.domain_destroy()
xc.domain_destroy_hook()
xc.domain_resume()
xc.domain_shutdown()
xc.vcpu_setaffinity()
xc.domain_sethandle()
xc.vcpu_getinfo()
xc.linux_build()
xc.getBitSize()
xc.gnttab_hvm_seed()
xc.hvm_get_param()
xc.hvm_set_param()
xc.get_device_group()
xc.test_assign_device()
xc.assign_device()
xc.deassign_device()
xc.sched_id_get()
xc.sched_credit_domain_set()
xc.sched_credit_domain_get()
xc.sched_credit2_domain_set()
xc.sched_credit2_domain_get()
xc.evtchn_alloc_unbound()
xc.evtchn_reset()
xc.physdev_map_pirq()
xc.physdev_pci_access_modify()
xc.readconsolering()
xc.topologyinfo()
xc.numainfo()
xc.shadow_control()
xc.shadow_mem_control()
xc.domain_set_memmap_limit()
xc.domain_ioport_permission()
xc.domain_irq_permission()
xc.domain_iomem_permission()
xc.pages_to_kib()
xc.domain_set_time_offset()
xc.domain_set_tsc_info()
xc.domain_disable_migrate()
xc.domain_send_trigger()
xc.send_debug_keys()
xc.domain_check_cpuid()
xc.domain_set_cpuid()
xc.domain_set_policy_cpuid()
xc.domain_set_machine_address_size()
xc.domain_suppress_spurious_page_faults()
xc.tmem_shared_auth()
xc.dom_set_memshr()
xc.cpupool_create()
xc.cpupool_destroy()
xc.cpupool_getinfo()
xc.cpupool_addcpu()
xc.cpupool_removecpu()
xc.cpupool_movedomain()
xc.cpupool_freeinfo()
xc.flask_context_to_sid()
xc.flask_sid_to_context()
xc.flask_load()
xc.flask_getenforce()
xc.flask_setenforce()
xc.flask_access()


Juergen

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

Re: [Xen-devel] [PATCH OSSTEST v3 3/3] Create a flight to test OpenStack with xen-unstable and libvirt

[Anthony PERARD]

On Thu, Oct 08, 2015 at 05:42:56PM +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST v3 3/3] Create a flight to test 
> OpenStack with xen-unstable and libvirt"):
> > On Tue, 2015-09-29 at 17:52 +0100, Ian Jackson wrote:
> > > All these revision_FOO=master are rather odd.
> > 
> > I think the revision_nova one was supposed to be $REVISION_OPENSTACK_NOVA?
> > 
> > AIUI Nova is the component which we care about (it's the "compute" bit of
> > openstack) and changes frequently compared with devstack (the overall
> > driver).
> 
> And the others we don't care about ?  Are they likely to break things
> anyway, is my point ?  Do they have uniform intercompatibility ?
> 
> > >   You seem to be creating
> > > a push gate which maintains a tested revision of `openstack-nova'.  Is
> > > that what you want ?  If so, then it's not really clear to me that
> > > what you want to do is to simply pick up new master revisions of these
> > > other git trees.  It might be better to obtain a bunch of revision
> > > ids, test them all together, and push them together ?
> > 
> > i.e. to have multiple output gates, one for each of those trees, each
> > pushed from a single flight?
> 
> Yes.
> 
> > I'm assuming you aren't talking about having N openstack flights, one for
> > each tree.
> > 
> > >   (This is not
> > > something that osstest can do right now but it doesn't seem
> > > difficult.)
> > 
> > It seems like quite a lot of faff in cr-daily-branch (what does $tree mean,
> > which $NEW_REVISION and $OLD_REVISION do we look at), ap-push now needs to
> > operate on a list or something (arguments?).
> > 
> > Getting this stuff right (i.e. testing) is quite hard even with access to a
> > production instance.
> 
> I can think of ways of doing it.  I wasn't expecting Anthony to
> produce them :-).  Right now I'm trying to understand the situation,
> and if I think we want to push multiple trees at once I'll write the
> code.
> 
> > The series today records the actual built versions, so we still get
> > bisections, what would having multiple output gates buy us in practice over
> > that?
> 
> I guess the main advantage is that other osstest `branches' could have
> a stable set of openstack things, and that the reporting of `what
> changed' would be accurate.
> 
> > Maybe the explicit =master stuff above should be removed, or replaced with
> > $REVISION_OPENSTACK_FOO which apart from NOVA are _not_ set by cr-daily
> > -branch (resulting in cloning master)?
> 
> If we do want to just ignore this issue of the other trees, I do
> indeed see no particular reason to explicitly set all the runvars to
> master.
> 
> > I think we want a push gate just for the regression tracking, but not
> > really for its own sake.

Yes, that is what I had in mind, testing OpenStack with tips of xen and
libvirt.

I'll remove all those revision_* runvars.

> In that case we I think yes don't really need to push multiple branches.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v3 7/9] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

[Julien Grall]

On 08/10/15 19:36, Julien Grall wrote:
> On 08/10/15 15:25, Ian Campbell wrote:
>>> If the concern is the behavior is changed, I'm happy to rework this code
>>> to keep exactly the same behavior. I.e any 32-bit write containing
>>> a 0 byte will be ignored. This is not optimal but at least I'm not
>>> opening the pandora box of fixing every single error in the code touch
>>> by this series.
>>
>> I'm okay with the new behaviour, I think Stefano was willing to tolerate it
>> (based on ).
> 
> It's what I understand too. I will a resend a new version tomorrow.
> 
>> So if we aren't going to fix it to DTRT WRT writing zero to a target then I
>> think we can go with the current variant and not change to ignoring any
>> word with a zero byte in it.
> 
> I'm thinking to split this patch in two:
>   - One which will turn the current ITARGETSR emulation in a function
> with the change of behavior when writing zero to a target.
> - The other to optimize the way we store the target.
> 
> This will keep the second patch nearly mechanical and avoid to change
> multiple behavior within the same patch.

While looking to split the patch in two part I've noticed another error
in the current emulation of ITARGETSR.

For a byte access, the byte will always be in r[0:7] of the register.
Although we are assuming that if the guest is writing to byte N (0 <= N
< 3), the byte will be in r[N * 8: ((N + 1)* 8) - 1]. Therefore any
guest using byte access won't be able to change the target:

target = (target << (8 * (gicd_reg & 0x3)));
target &= r;

I plan to fix it in patch #1 and request a backport for Xen 4.6 and Xen
4.5. I can do a separate patch if we don't want the cleanup. Note that
the second patch will be a requirement to fix 32-bit access to 64-bit
register.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 7/9] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

[Ian Campbell]

On Fri, 2015-10-09 at 12:24 +0100, Julien Grall wrote:

> I plan to fix it in patch #1 and request a backport for Xen 4.6 and Xen
> 4.5. I can do a separate patch if we don't want the cleanup.

I'm not quite sure what you are proposing but please put logical changes
and cleanups into separate patches and we will evaluate them for backport
individually (taking dependencies into account too).

Ian.

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

[Xen-devel] [OSSTEST PATCH 3/3] smoke tests: Consider osstest revision when reusing builds

[Ian Jackson]

Build results done with one version of osstest are not necessarily
reuseable with a different version of osstest.  For example, the suite
may have changed.  The smoke tests try to reuse builds from
xen-unstable but if osstest changes incompatibly, the smoke tests
might repeatedly fail until a xen-unstable flight using the new
osstest completes.

(This issue is a problem for bisection too but that's less critical
and there is less of an easy answer.)

Address this by also considering the osstest revision when searching
for builds to reuse, so the smoke tests only reuse builds made with
the same version of osstest.  (If we are running with uncommitted
changes, we ignore that aspect and instead insist on only builds which
used the committed revision of osstest: this makes testing the
xen-unstable-smoke machinery marginally easier, and will make no
difference to production runs.)

This introduces a new problem, though: after an osstest push, there
will be no suitable builds until the next xen-unstable flight passes.
So each smoke test would run its own build.  This would delay the
smoke tests, and waste capacity.

To address this we permit the smoke tests to reuse (i) builds from a
suitable osstest flight (hopefully there will be an osstest flight
which justified the push of the osstest version we are running) or
(ii) previous builds done by a xen-unstable-smoke test (this is a
useful backstop).

sg-check-tested always reports the highest-numbered flight which
matches all the specified conditions, so overall this means that:

1. Normally, the most recent relevant build for each job will be a
xen-unstable build; xen-unstable-smoke will reuse those.  Recent
flights on the osstest branch will be unsuitable because they use
different osstest; and there will be no recent relevant builds on
xen-unstable-smoke because xen-unstable-smoke will prefer to reuse its
own old builds rather than make new ones.

2. After a normal osstest push (whether force-pushed manually on the
basis of a test flight, or automatically pushed), the xen-unstable
builds are unsuitable.  However, the osstest push _is_ suitable, and
its builds will be used.

3. After a manual force push of an untested osstest, there no suitable
builds on either xen-unstable or osstest.  The first
xen-unstable-smoke run will have to do all the builds.  However,
subsequent xen-unstable-smoke runs can just pick up those builds.
These same builds will be reused until a xen-unstable flight using the
new osstest produces a passing build.

(If the force pushed osstest causes a build to break, then
xen-unstable-smoke will keep retrying and failing that individual
build until a fix is pushed to osstest#production or xen#staging.)

We honour an environemnt variable SMOKE_HARNESS_REV to override the
automatic determination of the desired test harness revision.

Signed-off-by: Ian Jackson 
---
 cr-daily-branch |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/cr-daily-branch b/cr-daily-branch
index c7cc33b..364238c 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -281,9 +281,14 @@ flight=`$makeflight $branch $xenbranch $OSSTEST_BLESSING 
"$@"`
 
 case $branch in
 xen-unstable-smoke)
+   harness_rev=$(perl -e 'use Osstest; print get_harness_rev();')
+   : ${SMOKE_HARNESS_REV:=${harness_rev%+}}
+
./mg-adjust-flight-makexrefs -v $flight \
'!build-amd64 !build-amd64-libvirt !build-armhf build-*' \
-   --debug --branch=xen-unstable --blessings=real
+   --debug --blessings=real\
+   --branch=xen-unstable,xen-unstable-smoke,osstest\
+   --revision-osstest=$SMOKE_HARNESS_REV
# Even adhoc or play flights ought to reuse only real
# previous builds.
;;
-- 
1.7.10.4


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

[Xen-devel] [OSSTEST PATCH 2/3] sg-check-tested: Honour multiple branches (comma-separated)

[Ian Jackson]

No functional change with existing invocations.

Signed-off-by: Ian Jackson 
---
 sg-check-tested |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sg-check-tested b/sg-check-tested
index 1a3afa3..43f2854 100755
--- a/sg-check-tested
+++ b/sg-check-tested
@@ -80,8 +80,9 @@ END
AND   r.flight = flights.flight)
 END
 } elsif (m/^--branch=(.*)$/) {
-push @conds_vars, $1;
-push @conds, "branch = ?";
+my @branches = split /\,/, $1;
+push @conds_vars, @branches;
+push @conds, "(".(join " OR ", map { "branch = ?" } @branches).")";
 } elsif (m/^--blessings=(.*)$/) {
 my @blessings= split /\,/, $1;
 push @conds_vars, @blessings;
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v1 1/4] xsm/libxl/xen_version: Add XSM for some of the xen_version commands.

[Andrew Cooper]

On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
> The XENVER_[compile_info|changeset|commandline] are now
> guarded by an XSM check.
>
> The rest: XENVER_[version|extraversion|capabilities|
> parameters|get_features|page_size|guest_handle] behave
> as before (no XSM check).
>
> We allow the initial domain to see these while the other
> guests are not permitted.
>
> As such we also modify the toolstack such that if we fail
> to get any data instead of printing (null) we just print "".
>
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  tools/flask/policy/policy/modules/xen/xen.te |  2 +-
>  tools/libxl/libxl.c  |  3 +++
>  xen/common/kernel.c  | 11 ++-
>  xen/include/xsm/dummy.h  | 24 
>  xen/include/xsm/xsm.h|  6 ++
>  xen/xsm/dummy.c  |  1 +
>  xen/xsm/flask/hooks.c| 25 +
>  xen/xsm/flask/policy/access_vectors  |  2 ++
>  8 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te 
> b/tools/flask/policy/policy/modules/xen/xen.te
> index d35ae22..d0ad758 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -86,7 +86,7 @@ allow dom0_t dom0_t:domain {
>  };
>  allow dom0_t dom0_t:domain2 {
>   set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
> - get_vnumainfo psr_cmt_op psr_cat_op
> + get_vnumainfo psr_cmt_op psr_cat_op version_use
>  };
>  allow dom0_t dom0_t:resource { add remove };
>  
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 22bbc29..efa6462 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5272,6 +5272,7 @@ const libxl_version_info* 
> libxl_get_version_info(libxl_ctx *ctx)
>  xc_version(ctx->xch, XENVER_extraversion, _extra);
>  info->xen_version_extra = strdup(u.xen_extra);
>  
> +memset(_cc, 0, sizeof(xen_compile_info_t));

FILLZERO()

>  xc_version(ctx->xch, XENVER_compile_info, _cc);
>  info->compiler = strdup(u.xen_cc.compiler);
>  info->compile_by = strdup(u.xen_cc.compile_by);
> @@ -5281,6 +5282,7 @@ const libxl_version_info* 
> libxl_get_version_info(libxl_ctx *ctx)
>  xc_version(ctx->xch, XENVER_capabilities, _caps);
>  info->capabilities = strdup(u.xen_caps);
>  
> +memset(_cc, 0, sizeof(xen_changeset_info_t));
>  xc_version(ctx->xch, XENVER_changeset, _chgset);
>  info->changeset = strdup(u.xen_chgset);
>  
> @@ -5289,6 +5291,7 @@ const libxl_version_info* 
> libxl_get_version_info(libxl_ctx *ctx)
>  
>  info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
>  
> +memset(_commandline, 0, sizeof(xen_commandline_t));
>  xc_version(ctx->xch, XENVER_commandline, _commandline);
>  info->commandline = strdup(u.xen_commandline);
>  
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 6a3196a..210ec99 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -226,9 +227,17 @@ void __init do_initcalls(void)
>  /*
>   * Simple hypercalls.
>   */
> -
> +#define XENVER_CMD_XSM_CHECK( (1U << XENVER_compile_info) | \
> +  (1U << XENVER_changeset) | \
> +  (1U << XENVER_commandline) )
>  DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> +if ( ( 1 << cmd ) & XENVER_CMD_XSM_CHECK )
> +{
> +int rc = xsm_version_op(XSM_PRIV, cmd);
> +if ( rc )
> +return rc;

This call should be unconditional.  It is not going to make a
measureable difference on XENVER_version latency.

~Andrew

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

Re: [Xen-devel] [OSSTEST PATCH 2/3] sg-check-tested: Honour multiple branches (comma-separated)

[Ian Campbell]

On Fri, 2015-10-09 at 12:48 +0100, Ian Jackson wrote:
> No functional change with existing invocations.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

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

Re: [Xen-devel] [PATCH v1] Add build-id to XENVER hypercall.

[Jan Beulich]

>>> On 09.10.15 at 14:15,  wrote:
> On 09/10/15 09:17, Jan Beulich wrote:
>> The more that the tool stack uses the two, and as we know
>> tool stacks or parts thereof can live in unprivileged domains.
> 
> I would argue than a fully unprivileged toolstack domain has no need for
> any information from this hypercall.  It it needs some privilege, then
> XSM is in use and it can be given access.

True.

Jan


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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Haozhong Zhang]

On Fri, Oct 09, 2015 at 12:51:32AM -0600, Jan Beulich wrote:
> >>> On 28.09.15 at 09:13,  wrote:
> > When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
> > is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
> > the host TSC with a ratio between guest TSC rate and
> > nanoseconds. However, the result will be incorrect if the guest TSC rate
> > differs from the host TSC rate. This patch fixes this problem by using
> > the system time as elapsed_nsec.
> 
> For both this and patch 2, while at a first glance (and taking into
> account just the visible patch context) what you say seems to
> make sense, the explanation is far from sufficient namely when
> looking at the function as a whole. For one, effects on existing
> cases need to be explicitly described, in particular why SVM's TSC
> ratio code works without that change (or whether it has been
> broken all along, in which case these would become backporting
> candidates; input from SVM maintainers would be appreciated
> too). That may in particular mean being more specific about
> what is actually wrong with scaling the host TSC here (i.e. in
> which way both results differ), when supposedly that matches
> what the hardware does when TSC ratio is supported.
> 
> Then a reason needs to be given why the similar logic in the
> PVRDTSCP case does not also get adjusted.
> 
> Plus, looking at the respective code in tsc_set_info(), I'm
> getting the impression that what you're trying to do is not in line
> with what is intended so far: Especially the comment there
> suggests that the intention is for the guest TSC to be made
> match the host one. Considering migration this indeed looks
> suspicious, but then that would need changing too.
>

Do you mean the following comment?
/*
 * In default mode use native TSC if the host has safe TSC and:
 *  HVM/PVH: host and guest frequencies are the same (either
 *   "naturally" or via TSC scaling)
 *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
 */
 
To my understanding,

1. "naturally" responds to the case that a domain is
   newly created (rather than being migrated from other machine) so that
   its TSC frequency (d->arch.tsc_khz) is identical to the host TSC
   frequency (cpu_khz).

2. "via TSC scaling" means the case that the domain is migrated from
   another machine of different host TSC rate so that d->arch.tsc_khz
   != cpu_khz. In this case the guest still reads the (host) TSC
   natively, but SVM TSC ratio makes sure that TSC value is a scaled
   host TSC. This point can be confirmed by svm_tsc_ratio_load() which
   sets MSR_AMD64_TSC_RATIO to d->arch.tsc_khz/cpu_khz.

If my understanding, especially the second point, is correct, this
patch set intends to do the same thing with VMX TSC scaling.

Boris, I notice this comment was added by your commit 82713ec8. Is my
understanding correct?

Thanks,
Haozhong

> Jan
> 

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

Re: [Xen-devel] [OSSTEST PATCH v3 2/3] Testing cpupools: recipe for it and job definition

[Ian Campbell]

On Sat, 2015-10-03 at 02:39 +0200, Dario Faggioli wrote:
> Signed-off-by: Dario Faggioli 
> ---
> Cc: Ian Jackson 
> Cc: Ian Campbell 
> Cc: Juergen Gross 

This looks correct to me as it stands, but I think it will be impacted by
the changes relating to host flags for numbers of cpus etc.

> ---
> Changes from v2:
>  * restrict test generation to xl only.
> 
> Changes from v1:
>  * added invocation to ts-guest-stop in the recipe to kill
>leak-check complaints (which went unnoticed during v1
>testing, sorry)
>  * moved the test before the "ARM cutoff", and remove the
>per-arch filtering, so that the test can run on ARM
>hardware too
> ---
>  make-flight |   12 
>  sg-run-job  |7 +++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/make-flight b/make-flight
> index 8c75a9c..d27a02c 100755
> --- a/make-flight
> +++ b/make-flight
> @@ -373,6 +373,16 @@ do_multivcpu_tests () {
>  $debian_runvars all_hostflags=$most_hostflags
>  }
>  
> +do_cpupools_tests () {
> +  if [ x$toolstack != xxl -a $xenarch != $dom0arch ]; then
> +return
> +  fi
> +
> +  job_create_test test-$xenarch$kern-$dom0arch-xl-cpupools\
> +test-cpupools xl $xenarch $dom0arch   \
> +$debian_runvars all_hostflags=$most_hostflags
> +}
> +
>  do_passthrough_tests () {
>if [ $xenarch != amd64 -o $dom0arch != amd64 -o "$kern" != "" ]; then
>  return
> @@ -498,6 +508,8 @@ test_matrix_do_one () {
>do_rtds_tests
>do_credit2_tests
>  
> +  do_cpupools_tests
> +
># No further arm tests at the moment
>if [ $dom0arch = armhf ]; then
>return
> diff --git a/sg-run-job b/sg-run-job
> index 66145b8..ea48a03 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -296,6 +296,13 @@ proc run-job/test-debianhvm {} {
>  test-guest debianhvm
>  }
>  
> +proc need-hosts/test-cpupools {} { return host }
> +proc run-job/test-cpupools {} {
> +install-guest-debian
> +run-ts . = ts-cpupools + host debian
> +run-ts . = ts-guest-stop + host debian
> +}
> +
>  proc setup-test-pair {} {
>  run-ts . =  ts-debian-install  dst_host
>  run-ts . =  ts-debian-fixupdst_host  +
> debian
> 

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

[Xen-devel] [qemu-mainline test] 62725: trouble: broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-multivcpu  3 host-install(3)broken REGR. vs. 62649
 test-amd64-amd64-xl-xsm   3 host-install(3) broken REGR. vs. 62649
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken REGR. 
vs. 62649
 test-amd64-amd64-amd64-pvgrub  3 host-install(3)broken REGR. vs. 62649
 test-amd64-i386-freebsd10-i386  3 host-install(3)   broken REGR. vs. 62649
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 3 host-install(3) broken REGR. vs. 
62649

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-libvirt   3 host-install(3) broken REGR. vs. 62649
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken 
REGR. vs. 62649
 test-amd64-i386-libvirt-qcow2  3 host-install(3)broken REGR. vs. 62649
 test-amd64-i386-libvirt-vhd   3 host-install(3) broken REGR. vs. 62649
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 
62649
 test-armhf-armhf-xl-rtds 15 guest-start.2fail blocked in 62649

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-vhd   9 debian-di-installfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-vhd9 debian-di-installfail   never pass
 test-amd64-amd64-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass

version targeted for testing:
 qemuu5fdb4671b08e0d1631447e81348b2b50a6b85bf7
baseline version:
 qemuuc0b520dfb8890294a9f8879f4759172900585995

Last test of basis62649  2015-10-04 02:25:33 Z5 days
Failing since 62696  2015-10-06 11:41:51 Z2 days2 attempts
Testing same since62725  2015-10-08 07:48:45 Z1 days1 attempts


People who touched revisions under test:
  Amit Shah 
  Bastian Koppelmann 
  Bill Paul 
  Chen 

[Xen-devel] [xen-unstable baseline-only test] 38140: regressions - FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 38140 xen-unstable real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38140/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-pygrub  19 guest-start.2 fail REGR. vs. 38126
 test-amd64-amd64-xl-raw  10 guest-start   fail REGR. vs. 38126

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-rumpuserxen-i386 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail REGR. vs. 38126
 test-amd64-amd64-rumpuserxen-amd64 13 rumpuserxen-demo-xenstorels/xenstorels 
fail like 38126
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
like 38126
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 38126
 test-amd64-amd64-xl-qemut-winxpsp3  9 windows-install  fail like 38126

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass

version targeted for testing:
 xen  a23ce429779011de127e8ff6c9bf3486d87154d5
baseline version:
 xen  90f2e2a307fc6a6258c39cc87b3b2bf9441c0fa7

Last test of basis38126  2015-10-05 18:49:57 Z3 days
Testing same since38140  2015-10-08 22:54:41 Z0 days1 attempts


People who touched revisions under test:
  Daniel De Graaf 
  Dario Faggioli 
  Doug Goldstein 
  George Dunlap 
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Konrad Rzeszutek Wilk 
  Roger Pau Monné 
  Wei Wang 

jobs:
 build-amd64-xsm 

Re: [Xen-devel] [PATCH v1 3/4] XENVER_build_id: Provide ld-embedded build-ids

[Martin Pohlack]

On 09.10.2015 04:56, Konrad Rzeszutek Wilk wrote:
> @@ -367,6 +368,35 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> arg,
>  if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
>  return -EFAULT;
>  return 0;
> +
> +case XENVER_build_id:
> +{
> +int rc;
> +char *p = NULL;
> +unsigned int sz = 0;
> +
> +if ( guest_handle_is_null(arg) )
> +return -EINVAL;
> +
> +if ( len == 0 )
> +return -EINVAL;
> +
> +if ( !guest_handle_okay(arg, len) )
> +return -EINVAL;

Shouldn't this return -EFAULT?

> +
> +rc = xen_build_id(, );
> +if ( rc )
> +return rc;
> +
> +if ( sz > len )
> +return -ENOMEM;
> +
> +if ( copy_to_guest(arg, p, sz) )
> +return -EFAULT;
> +
> +return sz;
> +}
> +
>  }
>  
>  return -ENOSYS;

Martin

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


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

[Xen-devel] [OSSTEST PATCH 1/3] Move get_harness_rev to Osstest from Osstest::Executive.

[Ian Jackson]

No functional change.

Signed-off-by: Ian Jackson 
---
 Osstest.pm   |   17 +
 Osstest/Executive.pm |   18 +-
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/Osstest.pm b/Osstest.pm
index 4a763c6..969a2d0 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -38,6 +38,7 @@ BEGIN {
   main_revision_job_cond other_revision_job_suffix
   $dbh_tests db_retry db_retry_retry db_retry_abort
   db_begin_work db_prepare
+  get_harness_rev
   ensuredir get_filecontents_core_quiet system_checked
   nonempty visible_undef show_abs_time
   %arch_debian2xen %arch_xen2debian $cfgvar_re
@@ -333,6 +334,22 @@ sub main_revision_job_cond ($) {
 return "(${\ other_revision_job_suffix($jobfield,'x') } = '')";
 }
 
+sub get_harness_rev () {
+$!=0; $?=0;  my $rev= `git rev-parse HEAD^0`;
+die "$? $!" unless defined $rev;
+
+$rev =~ s/\n$//;
+die "$rev ?" unless $rev =~ m/^[0-9a-f]+$/;
+
+my $diffr= system 'git diff --exit-code HEAD >/dev/null';
+if ($diffr) {
+die "$diffr $! ?" if $diffr != 256;
+$rev .= '+';
+}
+
+return $rev;
+}
+
 sub get_filecontents_core_quiet ($) { # ENOENT => undef
 my ($path) = @_;
 if (!open GFC, '<', $path) {
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index bb2663a..01915ac 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -44,7 +44,7 @@ BEGIN {
 our ($VERSION, @ISA, @EXPORT, @EXPORT_OK, %EXPORT_TAGS);
 $VERSION = 1.00;
 @ISA = qw(Exporter);
-@EXPORT  = qw(get_harness_rev grabrepolock_reexec
+@EXPORT  = qw(grabrepolock_reexec
   findtask @all_lock_tables
   restrictflight_arg restrictflight_cond
   report_run_getinfo report_altcolour
@@ -128,22 +128,6 @@ sub grabrepolock_reexec {
 }
 }
 
-sub get_harness_rev () {
-$!=0; $?=0;  my $rev= `git rev-parse HEAD^0`;
-die "$? $!" unless defined $rev;
-
-$rev =~ s/\n$//;
-die "$rev ?" unless $rev =~ m/^[0-9a-f]+$/;
-
-my $diffr= system 'git diff --exit-code HEAD >/dev/null';
-if ($diffr) {
-die "$diffr $! ?" if $diffr != 256;
-$rev .= '+';
-}
-
-return $rev;
-}
-
 #-- database access --#
 
 sub opendb_state () {
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Andrew Cooper]

On 09/10/15 09:25, Jan Beulich wrote:
 On 09.10.15 at 04:56,  wrote:
>> All existing commands ignore the parameter so this does
>> not break the ABI.
> Does it not? What about the debug mode clobbering of hypercall
> argument registers?

That is an implementation detail of the hypervisor.  It is irrelevant to
guests whether Xen chooses to clobber the spare registers or not.

> I think such length indicators need to be part
> of the newly added sub-structures instead.

I disagree. Having this as a hypercall parameter is ABI compatible, and
avoids unnecessary copy_from_guest()

~Andrew

>
>> This paves the way for expanding the XENVER_
>> hypercall with variable size structures, such as
>> "XENVER_build_id: Provide ld-embedded build-ids"
>>
>> Suggested-by: Andrew Cooper 
>> Signed-off-by: Konrad Rzeszutek Wilk 
>> ---
>>  xen/arch/arm/traps.c| 2 +-
> xen/arch/x86/x86_64/entry.S
> xen/arch/x86/x86_64/compat/entry.S
>
> (but that's moot with the comment above)
>
> Jan
>


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

Re: [Xen-devel] [PATCH 0/2] Bulk mem-share identical domains

[Wei Liu]

On Thu, Oct 08, 2015 at 03:07:19PM -0600, Tamas K Lengyel wrote:
> In case you miss it, there is now soft-reset support which dumps all
> > memory plus various states from one domain to another, and toolstack
> > will take care of QEMU and various userspace bits. This might be useful
> > to you?
> >
> > To be clear, this is just FYI, not suggesting we block this series.
> >
> > Wei.
> >
> 
> Hi Wei,
> it might be very useful but on a casual scan I couldn't really find much on
> the soft-reset option (no xl cmdline option and only a single call to
> xc_domain_soft_reset in libxl.c). For cloning I would need the origin
> domain to remain loaded as before (at least the memory, qemu can be killed)
> and then I would only need the QEMU setup bits from soft-reset. Any
> pointers on how to go about this would be very helpful ;)
> 

Soft-reset is in fact a slightly modified version of save / restore.  I
don't think you can directly use soft-reset to clone a domain.  What I
meant was you might be able to reuse some of the code in soft-reset, at
least on toolstack side.

For example, you can invent a hypercall to share all pages and transfer
states from a guest to another. In toolstack, you create a new domain,
save original domain's QEMU state, issue aforementioned hypercall (*),
and restore QEMU. It would still require some coding to disentangle
toolstack code to do what you need, but these different phases already
exist in toolstack code for soft-reset except for the hypercall.

What makes your need different from soft-reset is that, a) the hypercall is
different b) you don't destroy the original domain afterwards.

YMMV.

Wei.

> Thanks,
> Tamas

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

Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Jan Beulich]

>>> On 09.10.15 at 14:46,  wrote:
> On Fri, 2015-10-09 at 13:29 +0100, Andrew Cooper wrote:
>> On 09/10/15 09:25, Jan Beulich wrote:
>> > > > > On 09.10.15 at 04:56,  wrote:
>> > > All existing commands ignore the parameter so this does
>> > > not break the ABI.
>> > Does it not? What about the debug mode clobbering of hypercall
>> > argument registers?
>> 
>> That is an implementation detail of the hypervisor.  It is irrelevant to
>> guests whether Xen chooses to clobber the spare registers or not.
> 
> Or in other words the effect here is to clobber one _less_ register, and
> the guest cannot have been relying on a register getting so clobbered (if
> nothing else it doesn't happen in debug=n builds).

No, the one less register clobbered is in the first clobbering phase,
where _unused_ inputs get clobbered (for hypervisor internal
consumption). The second clobbering phase destroys all _used_
input registers' contents (the guest visible values), and _this_ is
what results in ABI breakage (because callers assuming the
hypercall to take two arguments assume that the 3rd argument
register will retain its contents.

Jan


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

Re: [Xen-devel] [OSSTEST PATCH v3 3/3] ts-logs-capture: include some cpupools info in the captured logs.

[Ian Campbell]

On Sat, 2015-10-03 at 02:39 +0200, Dario Faggioli wrote:
> Signed-off-by: Dario Faggioli 

Acked-by: Ian Campbell 

There's probably no need for this to wait for the rest.

> ---
> Cc: Ian Jackson 
> Cc: Ian Campbell 
> Cc: Juergen Gross 
> ---
> Changes from v2:
>  * new patch, the introduction of which was suggested
>during review.
> ---
>  ts-logs-capture |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ts-logs-capture b/ts-logs-capture
> index b99b1db..b1e7012 100755
> --- a/ts-logs-capture
> +++ b/ts-logs-capture
> @@ -186,6 +186,8 @@ sub fetch_logs_host () {
>   'cat /proc/cpuinfo',
>   'xl list',
>   'xl vcpu-list',
> + 'xl cpupool-list',
> + 'xl cpupool-list -c',
>   'xm list',
>   'xm list --long',
>   'xenstore-ls -fp',
> 

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Jan Beulich]

>>> On 09.10.15 at 15:41,  wrote:
> On 10/09/2015 02:51 AM, Jan Beulich wrote:
> On 28.09.15 at 09:13,  wrote:
>>> When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
>>> is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
>>> the host TSC with a ratio between guest TSC rate and
>>> nanoseconds. However, the result will be incorrect if the guest TSC rate
>>> differs from the host TSC rate. This patch fixes this problem by using
>>> the system time as elapsed_nsec.
>> For both this and patch 2, while at a first glance (and taking into
>> account just the visible patch context) what you say seems to
>> make sense, the explanation is far from sufficient namely when
>> looking at the function as a whole. For one, effects on existing
>> cases need to be explicitly described, in particular why SVM's TSC
>> ratio code works without that change (or whether it has been
>> broken all along, in which case these would become backporting
>> candidates; input from SVM maintainers would be appreciated
>> too). That may in particular mean being more specific about
>> what is actually wrong with scaling the host TSC here (i.e. in
>> which way both results differ), when supposedly that matches
>> what the hardware does when TSC ratio is supported.
> 
> If elapsed_nsec is the time that guest has been running then how can 
> get_s_time(), which is system time, be the right answer here? But what 
> confuses me even more is that existing code is not doing that neither.
> 
> Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side? 
> I.e.
> 
> *elapsed_nsec = get_s_time() - d->arch.vtsc_offset?

Doesn't whether or not to adjust be the offset depend on d-arch.vtsc?

Jan


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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Haozhong Zhang]

On Fri, Oct 09, 2015 at 12:31:53PM -0400, Boris Ostrovsky wrote:
> On 10/09/2015 12:19 PM, Jan Beulich wrote:
> On 09.10.15 at 18:09,  wrote:
> >>On 10/09/2015 11:11 AM, Jan Beulich wrote:
> >>On 09.10.15 at 16:00,  wrote:
> On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:
> >On 10/09/2015 02:51 AM, Jan Beulich wrote:
> >On 28.09.15 at 09:13,  wrote:
> >>>When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
> >>>is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
> >>>the host TSC with a ratio between guest TSC rate and
> >>>nanoseconds. However, the result will be incorrect if the guest TSC 
> >>>rate
> >>>differs from the host TSC rate. This patch fixes this problem by using
> >>>the system time as elapsed_nsec.
> >>For both this and patch 2, while at a first glance (and taking into
> >>account just the visible patch context) what you say seems to
> >>make sense, the explanation is far from sufficient namely when
> >>looking at the function as a whole. For one, effects on existing
> >>cases need to be explicitly described, in particular why SVM's TSC
> >>ratio code works without that change (or whether it has been
> >>broken all along, in which case these would become backporting
> >>candidates; input from SVM maintainers would be appreciated
> >>too). That may in particular mean being more specific about
> >>what is actually wrong with scaling the host TSC here (i.e. in
> >>which way both results differ), when supposedly that matches
> >>what the hardware does when TSC ratio is supported.
> >If elapsed_nsec is the time that guest has been running then how can
> >get_s_time(), which is system time, be the right answer here? But what
> >confuses me even more is that existing code is not doing that neither.
> >
> >Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
> >I.e.
> >
> >*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?
> >
> Yes, I should minus d->arch.vtsc_offset here.
> >>>In which case - afaict - the code becomes identical to that of the
> >>>TSC_MODE_ALWAYS_EMULATE case as well as the
> >>>TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
> >>>unlikely to be correct.
> >>*elapsed_nsec = *gtsc_khz = 0; ? Because we are effectively in
> >>TSC_MODE_NEVER.
> >How that? Talk here has been about TSC_MODE_DEFAULT...
> 
> AFAIUI, TSC_MODE_DEFAULT is a shorthand for saying "I will let the
> hypervisor pick whether the guest will be in TSC_MODE_ALWAYS_EMULATE or
> TSC_MODE_NEVER". d->arch.vtsc is what ends up being internal implementation
> of user-provided mode (for the most parts; I think hvm_cpuid() being the
> only true exception --- and perhaps it needs to be looked at).
> 
> So if we have d->arch.vtsc=0 (which is the case we are talking about here)
> then we are really in NEVER mode
>

Not quite understand this. Is tsc_set_info() the only place to set
d->arch.tsc_mode ? Though it may decide d->arch.vtsc should be 1, it
still sets d->arch.tsc_mode to the user provided TSC mode for a
non-pvh domain. And then in tsc_get_info(), it should never fall into
TSC_MODE_NEVER_EMULATE branch if d->arch.tsc_mode is not.

- Haozhong

> 
> -boris
> 
> >
> >>That can't be right...
> >Why not? tsc_set_info() doesn't care about any of its other input
> >values when that mode is in effect.
> >
> >Jan
> >
> 

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

[Xen-devel] [PATCH] x86/shadow: Fix missing newline in dprintk()

[Andrew Cooper]

to avoid console corruption.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Tim Deegan 
---
 xen/arch/x86/mm/shadow/common.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 3759232..58f131c 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1580,7 +1580,7 @@ void shadow_free(struct domain *d, mfn_t smfn)
 if ( !d->arch.paging.p2m_alloc_failed )
 {
 d->arch.paging.p2m_alloc_failed = 1;
-dprintk(XENLOG_ERR, "d%i failed to allocate from shadow pool",
+dprintk(XENLOG_ERR, "d%i failed to allocate from shadow pool\n",
 d->domain_id);
 }
 paging_unlock(d);
-- 
1.7.10.4


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

[Xen-devel] patch "x86/cpufreq: relocate the driver register function" breaks cpu hot(un)plug

[Dario Faggioli]

Hey,

As far as my bisection goes, commit
49388f11d512bb92706ce046643bfbb3c1d963c9 "x86/cpufreq: relocate the
driver register function" prevents me from hot unplugging pCPUs.

Xen does not crash or anything, but dom0 is stalled. In fact, with
current staging, here's what I see:

root@Zhaman:~# echo 0 > /sys/devices/system/xen_cpu/xen_cpu6/online 
[   81.583001] INFO: rcu_sched detected stalls on CPUs/tasks: { 12} (detected 
by 3, t=5252 jiffies, g=1691, c=1690, q=76)
[   81.583036] Task dump for CPU 12:
[   81.583044] bashR  running task0  1347   1094 0x0008
[   81.583056]     
8800192c2e38
[   81.583070]  888472e8 0002 888472e8 
880013817858
[   81.583082]   81a4 811e8137 
8800192c2e38
[   81.583095] Call Trace:
[   81.583110]  [] ? notify_change+0x2f7/0x390
[   81.583148]  [] ? do_truncate+0x74/0x90
[   81.583158]  [] ? dput+0x26/0x230
[   81.583167]  [] ? terminate_walk+0x35/0x40
[   81.583176]  [] ? do_last+0x621/0x12c0
[   81.583188]  [] ? xen_pcpu_down+0x47/0x70
[   81.583199]  [] ? store_online+0x9d/0xb0
[   81.583210]  [] ? kernfs_fop_write+0x12c/0x180
[   81.583220]  [] ? __vfs_write+0x23/0xf0
[   81.583230]  [] ? __sb_start_write+0x42/0xf0
[   81.583241]  [] ? security_file_permission+0x21/0xa0
[   81.583250]  [] ? vfs_write+0xa1/0x1c0
[   81.583259]  [] ? filp_close+0x4f/0x70
[   81.583268]  [] ? SyS_write+0x42/0xb0
[   81.583277]  [] ? __close_fd+0x71/0xb0
[   81.583287]  [] ? system_call_fastpath+0x16/0x75
[  144.555020] INFO: rcu_sched detected stalls on CPUs/tasks: { 12} (detected 
by 4, t=21007 jiffies, g=1691, c=1690, q=244)
[  144.555046] Task dump for CPU 12:
[  144.555051] bashR  running task0  1347   1094 0x0008
[  144.555059]     
8800192c2e38
[  144.555068]  888472e8 0002 888472e8 
880013817858
[  144.555076]   81a4 811e8137 
8800192c2e38
[  144.555084] Call Trace:
[  144.555096]  [] ? notify_change+0x2f7/0x390
[  144.555105]  [] ? do_truncate+0x74/0x90
[  144.555112]  [] ? dput+0x26/0x230
[  144.555118]  [] ? terminate_walk+0x35/0x40
[  144.555124]  [] ? do_last+0x621/0x12c0
[  144.555164]  [] ? xen_pcpu_down+0x47/0x70
[  144.555172]  [] ? store_online+0x9d/0xb0
[  144.555179]  [] ? kernfs_fop_write+0x12c/0x180
[  144.555186]  [] ? __vfs_write+0x23/0xf0
[  144.555192]  [] ? __sb_start_write+0x42/0xf0
[  144.555200]  [] ? security_file_permission+0x21/0xa0
[  144.555206]  [] ? vfs_write+0xa1/0x1c0
[  144.555212]  [] ? filp_close+0x4f/0x70
[  144.555217]  [] ? SyS_write+0x42/0xb0
[  144.555223]  [] ? __close_fd+0x71/0xb0
[  144.555230]  [] ? system_call_fastpath+0x16/0x75

If I revert that patch, the issue goes away.

Any ideas? 

Regards,
Dario

PS. yes, I'll implement a cpu hotplug/unplug testcase ASAP. :-)

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



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

Re: [Xen-devel] [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent

[Dario Faggioli]

On Fri, 2015-10-09 at 14:05 +0100, Andrew Cooper wrote:
> On 08/10/15 21:39, Dario Faggioli wrote:
> > On Thu, 2015-10-08 at 16:20 +0100, Andrew Cooper wrote:

> > > On 08/10/15 15:58, George Dunlap wrote:
> > > > Generic scheduling code is called from interrupt contexts --
> > > > namely,
> > > > vcpu_wake()
> > > There are a lot of codepaths, but I cant see one which is
> > > definitely
> > > called with interrupts disables.  (OTOH, I can see several where
> > > interrupts are definitely enabled).
> > > 
> > Sorry, it's certainly me, but I don't think I understand this.
> > 
> > AFAICT, you are saying that you fail to find in the code,
> > situations
> > the scheduler code is invoked, with interrupts already disabled, is
> > this correct? In particular "definitely called with interrupt
> > disabled"
> > is what confuses me... :-/
> 
> Given a brief look at the code, I can't spot a codepath where it is
> obvious that interrupts are disabled.
> 
Ok, thanks for rephrasing it (and for bearing with me :-).

> > Also (assuming what I just said is what you meant), are you
> > referring
> > "just" to schedule(), or even to other scheduler's code, which also
> > disables interrupt when taking the lock(s) it needs, like, e.g.,
> > vcpu_wake() as George said?
> 
> I was looking on callchains involving vcpu_wake().
> 
Ok, thanks for clarifying this too.

AFAIUI for now, the issue is more whether we are in interrupt context,
rather than whether or not interrupt are disabled already. That is,
whether or not, by not deactivating interrupt when taking the
{v,p}cpu_schedule_lock(), we can deadlock a pCPU (and, in case that can
happen, how to avoid that).

Anyway, I think this is interesting (it is for me, at least :-)) and
worthwhile enough to investigate a bit more.

I'll dig and report.

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



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

[Xen-devel] [OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Jackson]

This is all mad.

Signed-off-by: Ian Jackson 
---
 standalone-generate-dump-flight-runvars |   24 
 1 file changed, 24 insertions(+)

diff --git a/standalone-generate-dump-flight-runvars 
b/standalone-generate-dump-flight-runvars
index a1907b0..efedd5c 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -58,8 +58,32 @@ perbranch () {
 flight=check_${branch//[-._]/_}
 }
 
+# Good grief, handling background proceesses from shell is a pain.
+#
+# For stupid historical reasons, background processes start with
+# SIGINT (and QUIT) ignored (SuSv3 2.11).  bash does not offer a
+# way to ask it not to do this.
+#
+# There is no way to reset this in bash (bash 4.2.37 manpage section
+# on `trap' builtin), so we use perl.  However, there is still a race:
+# if the signal arrives just after the fork, after the shell has (in
+# the child) set it to to IGN, but before Perl has put it back, the
+# child might still escape.  So in the child we check our parent.
+#
+# I _think_ that that any signal which arrives before the assignment
+# to $SIG{} will definitely have caused our parent to vanish and us to
+# be reparented to pid 1 by the time we do the getppid check.  But TBH
+# I can't find any clear support for this requirement.  So the result
+# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
+# after starting.
+
 for branch in $@; do
 perbranch
+perl -e '
+$SIG{$_}=DFL foreach qw(INT QUIT HUP);
+   kill 1, $$ unless getppid=='$$';
+   exec @ARGV or die $!;
+' \
 ./standalone make-flight -f $flight $branch >$log 2>&1 &
 procs+=" $branch=$!"
 done
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Andrew Cooper]

On 09/10/15 13:46, Ian Campbell wrote:
> On Fri, 2015-10-09 at 13:29 +0100, Andrew Cooper wrote:
>> On 09/10/15 09:25, Jan Beulich wrote:
>> On 09.10.15 at 04:56,  wrote:
 All existing commands ignore the parameter so this does
 not break the ABI.
>>> Does it not? What about the debug mode clobbering of hypercall
>>> argument registers?
>> That is an implementation detail of the hypervisor.  It is irrelevant to
>> guests whether Xen chooses to clobber the spare registers or not.
> Or in other words the effect here is to clobber one _less_ register, and
> the guest cannot have been relying on a register getting so clobbered (if
> nothing else it doesn't happen in debug=n builds).

Correct - that is definitely a better phrasing.

> The flip side is that we are now no longer clobbering that register even
> for existing sub-ops which do not use it (since the clobbering doesn't go
> down to the subop level). So there is a risk that a guest may come to
> depend on that register not being clobbered and then fail older debug=y
> hypervisors.
>
> This second scenario doesn't seem especially likely to me.
>
> Do we not already have one or two hypercalls where subops consume different
> numbers of parameters anyway? HYPERVISOR_sched_op I think has this property
> and we've not been too concerned.

SCHEDOP_shutdown(suspend) effectively has a third parameter for PV
guests, but that is done entirely by the toolstack using the VMs
register context.

Xen isn't aware of it, and the duration of do_sched_op() does have the
register clobbered.

~Andrew

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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Andrew Cooper]

On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
> +rc = xc_version_len(ctx->xch, XENVER_build_id, _id, 
> BUILD_ID_LEN);
> +if (rc > 0) {
> +unsigned int i;
> +
> +info->build_id = (char *)malloc((rc * 2) + 1);
> +
> +for (i = 0; i < rc && (i + 1) * 2 < BUILD_ID_LEN; i++)
> +snprintf(>build_id[i * 2], 3, "%02hhx", u.build_id[i]);
> +
> +info->build_id[i*2]='\0';
> +} else
> +info->build_id = strdup("");

info->build_id is unconditionally leaked, given this patch.

~Andrew

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

[Xen-devel] [RFC OSSTEST v2] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Campbell]

And use this in standalone-generate-dump-flight-runvars. In general I
don't think we are interested in the specific revision_* runvars when
using this tool but it can be avoided using AP_FETCH_PLACEHOLDERS=n
when calling standalone-generate-dump-flight-runvars.

This is quicker even than using memoisation on the ap-fetch
invocations and produces output like:

libvirtbuild-amd64
revision_xenap-fetch-version-baseline:xen-unstable

By doing this the diffs of before and after changes to e.g.
make-flight don't pickup noise if a something/someone does a push in
the middle.

The memoisation bits of standalone-generate-dump-flight-runvars are
disable if AP_FETCH_PLACEHOLDERS=y.

Still RFC because of these sqlite errors

DBD::SQLite::db do failed: UNIQUE constraint failed: jobs.flight, jobs.job 
[for Statement "INSERT INTO jobs VALUES 
(?,'build-i386-xsm','build','queued')

or

DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM 
runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.
DBD::SQLite::db do failed: database is locked [for Statement " DELETE FROM 
runvars WHERE flight = ?  "] at Osstest/JobDB/Standalone.pm line 67.

Which consistently take out the use of standalone-generate-dump-flight-runvars
with this patch. I think probably because ap-fetch-* now complete
instantly which makes the standalone-generate-dump-flight-runvars far
more thunderous on the DB.

Signed-off-by: Ian Campbell 
---
v2:
  - Fix typo which would activate this mode unless
$AP_FETCH_PLACEHOLDERS=x.
  - Require $AP_FETCH_PLACEHOLDERS=y (not just non-empty) and update
standalone-generate-runvars to only set AP_FETCH_PLACEHOLDERS=y if
it is unset.
  - Drop sqlite_use_immediate_transaction => 0,
  - Only memoize if not using placeholders (in particular don't blow
away the cache)

The SQL errors seem to reproduce less reliably in this iteration than
before. I haven't got a FC why.
---
 ap-common   |  9 +
 ap-fetch-version|  2 ++
 ap-fetch-version-baseline   |  3 +++
 ap-fetch-version-baseline-late  |  2 ++
 ap-fetch-version-old|  2 ++
 standalone-generate-dump-flight-runvars | 10 --
 6 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/ap-common b/ap-common
index 91425a9..5b6e088 100644
--- a/ap-common
+++ b/ap-common
@@ -145,3 +145,12 @@ info_linux_tree () {
 
return 0
 }
+
+check_ap_fetch_placeholders () {
+   if [ "x$AP_FETCH_PLACEHOLDERS" != xy ] ; then
+   return 0
+   fi
+
+   echo "$(basename $0):$branch"
+   exit 0
+}
diff --git a/ap-fetch-version b/ap-fetch-version
index 6fa7588..f884bd3 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 if info_linux_tree "$branch"; then
repo_tree_rev_fetch_git linux \
$TREE_LINUX_THIS $TAG_LINUX_THIS $LOCALREV_LINUX
diff --git a/ap-fetch-version-baseline b/ap-fetch-version-baseline
index 2e42508..c9da82c 100755
--- a/ap-fetch-version-baseline
+++ b/ap-fetch-version-baseline
@@ -22,6 +22,9 @@ set -e -o posix
 branch=$1
 
 . ./cri-lock-repos
+. ./ap-common
+
+check_ap_fetch_placeholders
 
 : ${BASE_TREE_LINUX:=git://xenbits.xen.org/people/ianc/linux-2.6.git}
 : ${BASE_TAG_LINUX:=xen/next-2.6.32}
diff --git a/ap-fetch-version-baseline-late b/ap-fetch-version-baseline-late
index 9856ec9..dff8b05 100755
--- a/ap-fetch-version-baseline-late
+++ b/ap-fetch-version-baseline-late
@@ -27,6 +27,8 @@ new=$2
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 case "$branch" in
 
 linux-next)
diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 66d51f8..99f276a 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -25,6 +25,8 @@ branch=$1
 select_xenbranch
 . ./ap-common
 
+check_ap_fetch_placeholders
+
 : ${BASE_TAG_LINUX2639:=tested/2.6.39.x}
 : ${BASE_LOCALREV_LINUX:=daily-cron.$branch.old}
 : ${BASE_LOCALREV_LIBVIRT:=daily-cron.$branch.old}
diff --git a/standalone-generate-dump-flight-runvars 
b/standalone-generate-dump-flight-runvars
index d113927..a1907b0 100755
--- a/standalone-generate-dump-flight-runvars
+++ b/standalone-generate-dump-flight-runvars
@@ -36,11 +36,17 @@ if [ $# = 0 ]; then
set `./mg-list-all-branches`
 fi
 
-if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
+: ${AP_FETCH_PLACEHOLDERS:=y}
+export AP_FETCH_PLACEHOLDERS
+
+
+if [ "x$AP_FETCH_PLACEHOLDERS" != xy ]; then
+if [ "x$AP_FETCH_MEMO_KEEP" = x ]; then
rm -rf tmp/apmemo
mkdir tmp/apmemo
+fi
+export AP_FETCH_PFX='./memoise tmp/apmemo'
 fi
-export AP_FETCH_PFX='./memoise tmp/apmemo'
 
 # In the future it might be nice for this script to arrange to use a
 # separate standalone.db, in tmp/ probably, for each different branch.
-- 
2.5.3

Re: [Xen-devel] [PATCH v1] Add build-id to XENVER hypercall.

[Konrad Rzeszutek Wilk]

On Fri, Oct 09, 2015 at 01:15:42PM +0100, Andrew Cooper wrote:
> On 09/10/15 09:17, Jan Beulich wrote:
>  On 09.10.15 at 04:56,  wrote:
> >> However they also change the behavior of the existing hypercall
> >> for XENVER_[compile_info|changeset|commandline] and make them
> >> dom0 accessible. This is if XSM is built in or not (though with
> >> XSM one can expose it to a guest if desired).
> > Wasn't the outcome of the previous discussion that we should not
> > alter default behavior for existing sub-ops?
> 
> I raised a worry that some guests might break if they suddenly have
> access to this information cut off.

Let me double-confirm that the guests are OK with this being
gone. I did ran tests to see if the worked, but hadn't actually tried
acessing (/sys/hypervisor/xen*) the values.

> 
> > And even if I'm
> > misremembering, I can see reasons for not exposing the command
> > line, but what value has not exposing compile info and changeset
> > again?
> 
> There is a fear that providing such information makes it easier for
> attackers who have an exploit for a specific binary.
> 
> > The more that the tool stack uses the two, and as we know
> > tool stacks or parts thereof can live in unprivileged domains.
> 
> I would argue than a fully unprivileged toolstack domain has no need for
> any information from this hypercall.  It it needs some privilege, then
> XSM is in use and it can be given access.

What he said :-)
> 
> > Plus
> > there is also a (hg-centric and hence generally broken) attempt to
> > store it in hvm_save().
> 
> I will be addressing this in due course with my further cpuid plans.
> 
> ~Andrew

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

Re: [Xen-devel] [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent

[Andrew Cooper]

On 08/10/15 21:39, Dario Faggioli wrote:
> On Thu, 2015-10-08 at 16:20 +0100, Andrew Cooper wrote:
>> On 08/10/15 15:58, George Dunlap wrote:
>>> Generic scheduling code is called from interrupt contexts --
>>> namely,
>>> vcpu_wake()
>> There are a lot of codepaths, but I cant see one which is definitely
>> called with interrupts disables.  (OTOH, I can see several where
>> interrupts are definitely enabled).
>>
> Sorry, it's certainly me, but I don't think I understand this.
>
> AFAICT, you are saying that you fail to find in the code, situations
> the scheduler code is invoked, with interrupts already disabled, is
> this correct? In particular "definitely called with interrupt disabled"
> is what confuses me... :-/

Given a brief look at the code, I can't spot a codepath where it is
obvious that interrupts are disabled.

>
> Also (assuming what I just said is what you meant), are you referring
> "just" to schedule(), or even to other scheduler's code, which also
> disables interrupt when taking the lock(s) it needs, like, e.g.,
> vcpu_wake() as George said?

I was looking on callchains involving vcpu_wake().

>
>>> , which for the credit scheduler wants to put things on the
>>> runqueue.  Lock taken in interrupt context => interrupts must be
>>> disabled whenever taking the lock, yes?
>> Correct, which is the purpose of the ASSERT()s in the _irq() and
>> _irqsave() variants.
>>
> What ASSERT()? I don't find any assert in _spin_lock_irqsave() (which
> thing makes sense to me):

Ah yes - _irqsave() wouldn't want an assertion.

~Andrew

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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Ian Campbell]

On Fri, 2015-10-09 at 13:59 +0100, Andrew Cooper wrote:
> On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
> > +rc = xc_version_len(ctx->xch, XENVER_build_id, _id,
> > BUILD_ID_LEN);
> > +if (rc > 0) {
> > +unsigned int i;
> > +
> > +info->build_id = (char *)malloc((rc * 2) + 1);
> > +
> > +for (i = 0; i < rc && (i + 1) * 2 < BUILD_ID_LEN; i++)
> > +snprintf(>build_id[i * 2], 3, "%02hhx",
> > u.build_id[i]);
> > +
> > +info->build_id[i*2]='\0';
> > +} else
> > +info->build_id = strdup("");
> 
> info->build_id is unconditionally leaked, given this patch.

It should be freed by libxl_version_info_dispose, which any correct callers
should already be using.


Ian.

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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Andrew Cooper]

On 09/10/15 14:06, Ian Campbell wrote:
> On Fri, 2015-10-09 at 13:59 +0100, Andrew Cooper wrote:
>> On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
>>> +rc = xc_version_len(ctx->xch, XENVER_build_id, _id,
>>> BUILD_ID_LEN);
>>> +if (rc > 0) {
>>> +unsigned int i;
>>> +
>>> +info->build_id = (char *)malloc((rc * 2) + 1);
>>> +
>>> +for (i = 0; i < rc && (i + 1) * 2 < BUILD_ID_LEN; i++)
>>> +snprintf(>build_id[i * 2], 3, "%02hhx",
>>> u.build_id[i]);
>>> +
>>> +info->build_id[i*2]='\0';
>>> +} else
>>> +info->build_id = strdup("");
>> info->build_id is unconditionally leaked, given this patch.
> It should be freed by libxl_version_info_dispose, which any correct callers
> should already be using.

Ah - so it will.  Sorry for the noise.

~Andrew

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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Ian Campbell]

On Fri, 2015-10-09 at 14:06 +0100, Ian Campbell wrote:
> On Fri, 2015-10-09 at 13:59 +0100, Andrew Cooper wrote:
> > On 09/10/15 03:56, Konrad Rzeszutek Wilk wrote:
> > > +rc = xc_version_len(ctx->xch, XENVER_build_id, _id,
> > > BUILD_ID_LEN);
> > > +if (rc > 0) {
> > > +unsigned int i;
> > > +
> > > +info->build_id = (char *)malloc((rc * 2) + 1);
> > > +
> > > +for (i = 0; i < rc && (i + 1) * 2 < BUILD_ID_LEN; i++)
> > > +snprintf(>build_id[i * 2], 3, "%02hhx",
> > > u.build_id[i]);
> > > +
> > > +info->build_id[i*2]='\0';
> > > +} else
> > > +info->build_id = strdup("");
> > 
> > info->build_id is unconditionally leaked, given this patch.
> 
> It should be freed by libxl_version_info_dispose, which any correct callers
> should already be using.

This is a special case which is caching the result in the CTX, and the call
to dispose is actually in libxl_ctx_free not the caller, so the code is OK
but my "any correct callers" comment was bogus.

The caller can't/shouldn't call dispose because libxl_get_version_info
returns a const.

Ian.

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

Re: [Xen-devel] [OSSTEST PATCH 3/3] smoke tests: Consider osstest revision when reusing builds

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 3/3] smoke tests: Consider osstest 
revision when reusing builds"):
> Probably a bad idea, but I wonder: would comparing the hostflags required
> of the two build hosts take care of some of this?
> 
> e.g. some random job I just pulled up:
> 
> share-build-wheezy-amd64,arch-amd64,suite-wheezy,purpose-build
> 
> This is a bit tenuous though, since really it is $r{$ident_suite} //
> $c{DebianSuite} which matters.

Also I don't think grobbling around in the runvars looking at
all_hostflags etc. is right.

> [...]
> > 3. After a manual force push of an untested osstest, there no suitable

Fixed (and the other typo).

> > builds on either xen-unstable or osstest.  The first
> > xen-unstable-smoke run will have to do all the builds.  However,
> > subsequent xen-unstable-smoke runs can just pick up those builds.
> > These same builds will be reused until a xen-unstable flight using the
> > new osstest produces a passing build.
> 
> 4. After a push from another tree whose gated output is used by xen
> -unstable-smoke (e.g. the linux-X.Y for the default kernel revision) then
> there will be no suitable builds in either the latest xen-unstable or
> osstest (neither of which are likely to have seen the linux push and built
> it before a smoke run occurs) in which case xen-unstable-smoke will do that
> build and then subsequently reuse it until a xen-unstable or osstest flight
> occurs which uses that Linux tree.
> 
> (is that worth mentioning? is it correct?)

No.  This reuse machinery does not consider the versions of anything -
except, now, osstest itself.  This is because the other trees'
versions are supposed to be intercompatible.

> Acked-by: Ian Campbell 

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Boris Ostrovsky]

On 10/09/2015 02:51 AM, Jan Beulich wrote:

On 28.09.15 at 09:13,  wrote:

When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
the host TSC with a ratio between guest TSC rate and
nanoseconds. However, the result will be incorrect if the guest TSC rate
differs from the host TSC rate. This patch fixes this problem by using
the system time as elapsed_nsec.

For both this and patch 2, while at a first glance (and taking into
account just the visible patch context) what you say seems to
make sense, the explanation is far from sufficient namely when
looking at the function as a whole. For one, effects on existing
cases need to be explicitly described, in particular why SVM's TSC
ratio code works without that change (or whether it has been
broken all along, in which case these would become backporting
candidates; input from SVM maintainers would be appreciated
too). That may in particular mean being more specific about
what is actually wrong with scaling the host TSC here (i.e. in
which way both results differ), when supposedly that matches
what the hardware does when TSC ratio is supported.


If elapsed_nsec is the time that guest has been running then how can 
get_s_time(), which is system time, be the right answer here? But what 
confuses me even more is that existing code is not doing that neither.


Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side? 
I.e.


*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?

-boris



Then a reason needs to be given why the similar logic in the
PVRDTSCP case does not also get adjusted.

Plus, looking at the respective code in tsc_set_info(), I'm
getting the impression that what you're trying to do is not in line
with what is intended so far: Especially the comment there
suggests that the intention is for the guest TSC to be made
match the host one. Considering migration this indeed looks
suspicious, but then that would need changing too.

Jan


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



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

Re: [Xen-devel] [PATCH v1 4/4] libxl: info: Display build_id of the hypervisor.

[Ian Campbell]

On Thu, 2015-10-08 at 22:56 -0400, Konrad Rzeszutek Wilk wrote:
> If the hypervisor is built with we will display it.

I think there is a word missing in this sentence. Perhaps "it" after
"with", or better "a build_id" or "blah blah feature enabled".

> @@ -5295,8 +5298,21 @@ const libxl_version_info*
> libxl_get_version_info(libxl_ctx *ctx)
>  xc_version(ctx->xch, XENVER_commandline, _commandline);
>  info->commandline = strdup(u.xen_commandline);
>  
> +rc = xc_version_len(ctx->xch, XENVER_build_id, _id, 
> BUILD_ID_LEN);

Please see tools/libxl/CODING_STYLE. A variable called rc must only ever
contain libxl error codes (ERROR_*), which xc_version_len does not return.

"r" or "ret" is appropriate for the return value from a system call or
xc_*.

> +if (rc > 0) {

Do you intentionally silently ignore a failure here? I think at the very
least you should LOG all but the ones which are expected and which you have
deemed tolerable.

> +unsigned int i;
> +
> +info->build_id = (char *)malloc((rc * 2) + 1);

Lack of an error check here, but in any case libxl__zalloc(NOGC, ...)
instead, so it isn't needed anyway.

> +
> +for (i = 0; i < rc && (i + 1) * 2 < BUILD_ID_LEN; i++)
> +snprintf(>build_id[i * 2], 3, "%02hhx", 
> u.build_id[i]);
> +
> +info->build_id[i*2]='\0';

You can drop after switching to libxl__zalloc, since the buffer starts out
zeroed.

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index d6ef9a2..232749b 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -353,6 +353,7 @@ libxl_version_info = Struct("version_info", [
>  ("virt_start",uint64),
>  ("pagesize",  integer),
>  ("commandline",   string),
> +("build_id",  string),

A #define LIBXL_HAVE_* in libxl.h is required to signal the presence of
this new field.


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

Re: [Xen-devel] [OSSTEST PATCH 3/3] smoke tests: Consider osstest revision when reusing builds

[Ian Campbell]

On Fri, 2015-10-09 at 14:22 +0100, Ian Jackson wrote:
> > (is that worth mentioning? is it correct?)
> 
> No.  This reuse machinery does not consider the versions of anything -
> except, now, osstest itself.  This is because the other trees'
> versions are supposed to be intercompatible.

Ah, I'm always forgetting that. Sorry for the noise.

Ian.

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Haozhong Zhang]

On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:
> On 10/09/2015 02:51 AM, Jan Beulich wrote:
> On 28.09.15 at 09:13,  wrote:
> >>When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
> >>is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
> >>the host TSC with a ratio between guest TSC rate and
> >>nanoseconds. However, the result will be incorrect if the guest TSC rate
> >>differs from the host TSC rate. This patch fixes this problem by using
> >>the system time as elapsed_nsec.
> >For both this and patch 2, while at a first glance (and taking into
> >account just the visible patch context) what you say seems to
> >make sense, the explanation is far from sufficient namely when
> >looking at the function as a whole. For one, effects on existing
> >cases need to be explicitly described, in particular why SVM's TSC
> >ratio code works without that change (or whether it has been
> >broken all along, in which case these would become backporting
> >candidates; input from SVM maintainers would be appreciated
> >too). That may in particular mean being more specific about
> >what is actually wrong with scaling the host TSC here (i.e. in
> >which way both results differ), when supposedly that matches
> >what the hardware does when TSC ratio is supported.
> 
> If elapsed_nsec is the time that guest has been running then how can
> get_s_time(), which is system time, be the right answer here? But what
> confuses me even more is that existing code is not doing that neither.
> 
> Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
> I.e.
> 
> *elapsed_nsec = get_s_time() - d->arch.vtsc_offset?
>

Yes, I should minus d->arch.vtsc_offset here.

> -boris
> 
> >
> >Then a reason needs to be given why the similar logic in the
> >PVRDTSCP case does not also get adjusted.
> >
> >Plus, looking at the respective code in tsc_set_info(), I'm
> >getting the impression that what you're trying to do is not in line
> >with what is intended so far: Especially the comment there
> >suggests that the intention is for the guest TSC to be made
> >match the host one. Considering migration this indeed looks
> >suspicious, but then that would need changing too.
> >
> >Jan
> >
> >
> >___
> >Xen-devel mailing list
> >Xen-devel@lists.xen.org
> >http://lists.xen.org/xen-devel
> 

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

Re: [Xen-devel] [PATCH v2 1/2] x86/mem-sharing: Bulk mem-sharing entire domains

[Tamas K Lengyel]

On Fri, Oct 9, 2015 at 1:51 AM, Jan Beulich  wrote:

> >>> On 08.10.15 at 22:57,  wrote:
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1293,6 +1293,37 @@ int relinquish_shared_pages(struct domain *d)
> >  return rc;
> >  }
> >
> > +static int bulk_share(struct domain *d, struct domain *cd, unsigned
> long max,
> > +  struct mem_sharing_op_bulk_share *bulk)
> > +{
> > +int rc = 0;
> > +shr_handle_t sh, ch;
> > +
> > +while( bulk->start <= max )
> > +{
> > +if ( mem_sharing_nominate_page(d, bulk->start, 0, ) != 0 )
> > +goto next;
> > +
> > +if ( mem_sharing_nominate_page(cd, bulk->start, 0, ) != 0 )
> > +goto next;
> > +
> > +if ( !mem_sharing_share_pages(d, bulk->start, sh, cd,
> bulk->start, ch) )
> > +++(bulk->shared);
>
> Pointless parentheses.
>
>
Pointless but harmless and I like this style better.


> > +next:
>
> Labels indented by at least one space please.
>

Ack.


>
> > +++(bulk->start);
> > +
> > +/* Check for continuation if it's not the last iteration. */
> > +if ( bulk->start < max && hypercall_preempt_check() )
> > +{
> > +rc = 1;
> > +break;
>
> This could simple be a return statement, avoiding the need for a
> local variable (the type of which would need to be changed, see
> below).
>

It's reused in the caller to indicate where the mso copyback happens and rc
is of type int in the caller.


>
> > +}
> > +}
> > +
> > +return rc;
> > +}
>
> So effectively the function right now returns a boolean. If that's
> intended, its return type should reflect that (but I then wonder
> whether the sense of the values shouldn't be inverted, to have
> "true" mean "done").
>

It does but it's return is assigned to rc which is used to decide where
copyback happens.


>
> > @@ -1467,6 +1498,59 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> >  }
> >  break;
> >
> > +case XENMEM_sharing_op_bulk_share:
> > +{
> > +unsigned long max_sgfn, max_cgfn;
> > +struct domain *cd;
> > +
> > +rc = -EINVAL;
> > +if ( !mem_sharing_enabled(d) )
> > +goto out;
> > +
> > +rc =
> rcu_lock_live_remote_domain_by_id(mso.u.bulk.client_domain,
> > +   );
> > +if ( rc )
> > +goto out;
> > +
> > +rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mso.op);
> > +if ( rc )
> > +{
> > +rcu_unlock_domain(cd);
> > +goto out;
> > +}
> > +
> > +if ( !mem_sharing_enabled(cd) )
> > +{
> > +rcu_unlock_domain(cd);
> > +rc = -EINVAL;
> > +goto out;
> > +}
> > +
> > +max_sgfn = domain_get_maximum_gpfn(d);
> > +max_cgfn = domain_get_maximum_gpfn(cd);
> > +
> > +if ( max_sgfn != max_cgfn || max_sgfn < mso.u.bulk.start )
>
> Are both domains required to be paused for this operation? If so,
> shouldn't you enforce this? If not, by the time you reach the if()
> the values are stale.
>

It's expected that the user has exclusive tool-side lock on the domains
before issuing this hypercall and that the domains are paused already.


>
> > +{
> > +rcu_unlock_domain(cd);
> > +rc = -EINVAL;
> > +goto out;
> > +}
> > +
> > +rc = bulk_share(d, cd, max_sgfn, );
> > +if ( rc )
>
> With the boolean nature the use of "rc" here is rather confusing;
> I'd suggest avoiding the use of in intermediate variable in this case.
>

I don't see where the confusion is - rc indicates there is work left to do
and hypercall continuation needs to be setup. I could move this block into
bulk_share itself.


>
> > +{
> > +if ( __copy_to_guest(arg, , 1) )
> > +rc = -EFAULT;
> > +else
> > +rc =
> hypercall_create_continuation(__HYPERVISOR_memory_op,
> > +   "lh",
> XENMEM_sharing_op,
> > +   arg);
> > +}
> > +
> > +rcu_unlock_domain(cd);
> > +}
> > +break;
> > +
> >  case XENMEM_sharing_op_debug_gfn:
> >  {
> >  unsigned long gfn = mso.u.debug.u.gfn;
> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> > index 320de91..0299746 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -447,6 +447,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t);
> >  #define XENMEM_sharing_op_debug_gref5
> >  #define 

Re: [Xen-devel] [PATCH OSSTEST v3] ap-fetch-*: Support $AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v3] ap-fetch-*: Support 
$AP_FETCH_PLACEHOLDERS envvar which outputs a placeholder"):
> And use this in standalone-generate-dump-flight-runvars. In general I
> don't think we are interested in the specific revision_* runvars when
> using this tool but when it matters this new behaviour can be avoided
> by setting AP_FETCH_PLACEHOLDERS=n.
> 
> This is quicker even than using memoisation on the ap-fetch
> invocations and produces output like:
> 
> libvirt  build-amd64  revision_xen  ap-fetch-version-baseline:xen-unstable
> 
> This is useful when doing comparisons of before and after changes to
> e.g. make-flight since they do not pickup noise if a something/someone
> does a push in the middle.
> 
> The memoisation bits of standalone-generate-dump-flight-runvars are
> disabled if AP_FETCH_PLACEHOLDERS=y.
> 
> Signed-off-by: Ian Campbell 

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [OSSTEST PATCH] standalone-generate-dump-flight-runvars: Handle ^C properly

[Ian Jackson]

Ian Jackson writes ("[OSSTEST PATCH] standalone-generate-dump-flight-runvars: 
Handle ^C properly"):
> This is all mad.
...
> +# I _think_ that that any signal which arrives before the assignment
> +# to $SIG{} will definitely have caused our parent to vanish and us to
> +# be reparented to pid 1 by the time we do the getppid check.  But TBH
> +# I can't find any clear support for this requirement.  So the result
> +# may still be slightly racy in the case that s-g-d-f-r is ^C'd right
> +# after starting.

The test program below demonstrates that this assuption is not true.
However, a better approach is likely to be absurdly complex, involving
the parent shell having an INT trap which conducts an explicit
rendezvous with ?each child.

Ian.

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

static pid_t willdie, child, parent;

static void runtest(void) {
willdie = getpid();  assert(willdie>=0);
child = fork();  assert(child>=0);
if (!child) {
kill(willdie, SIGUSR1);
parent = getppid();
//sleep(1);
if (parent==1) _exit(0);
fprintf(stderr,"willdie=%ld parent=%ld\n",
(long)willdie,(long)parent);
_exit(1);
}
for (;;)
;
}

int main(int argc, char **argv) {
int st;

for (;;) {
willdie = fork();  assert(willdie>=0);
if (!willdie) runtest();
pid_t got = waitpid(willdie, , 0);
assert(got==willdie);
assert(WIFSIGNALED(st) && WTERMSIG(st)==SIGUSR1);
}
}

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

Re: [Xen-devel] [PATCH] x86/shadow: Fix missing newline in dprintk()

[Tim Deegan]

At 18:01 +0100 on 09 Oct (113710), Andrew Cooper wrote:
> to avoid console corruption.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Tim Deegan 

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

Re: [Xen-devel] [PATCH v2 1/2] x86/mem-sharing: Bulk mem-sharing entire domains

[Tamas K Lengyel]

On Fri, Oct 9, 2015 at 7:26 AM, Andrew Cooper 
wrote:

> On 08/10/15 21:57, Tamas K Lengyel wrote:
> > diff --git a/xen/arch/x86/mm/mem_sharing.c
> b/xen/arch/x86/mm/mem_sharing.c
> > index a95e105..4cdddb1 100644
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1293,6 +1293,37 @@ int relinquish_shared_pages(struct domain *d)
> >  return rc;
> >  }
> >
> > +static int bulk_share(struct domain *d, struct domain *cd, unsigned
> long max,
> > +  struct mem_sharing_op_bulk_share *bulk)
> > +{
> > +int rc = 0;
> > +shr_handle_t sh, ch;
> > +
> > +while( bulk->start <= max )
> > +{
> > +if ( mem_sharing_nominate_page(d, bulk->start, 0, ) != 0 )
>
> This swallows the error from mem_sharing_nominate_page().  Some errors
> might be safe to ignore in this context, but ones like ENOMEM most
> certainly are not.
>
> You should record the error into rc and switch ( rc ) to ignore/process
> the error, passing hard errors straight up.
>

In my experience all errors here are safe to ignore. Yes, if an ENOMEM
condition presents itself the sharing will be incomplete (or even 0). There
isn't much the user can do about that other than killing another domain to
free up memory.. which will happen anyway automatically when a clone domain
first hits an unshare operation. These conditions are better handled with
the memsharing event listener.


>
> > +goto next;
> > +
> > +if ( mem_sharing_nominate_page(cd, bulk->start, 0, ) != 0 )
> > +goto next;
> > +
> > +if ( !mem_sharing_share_pages(d, bulk->start, sh, cd,
> bulk->start, ch) )
> > +++(bulk->shared);
> > +
> > +next:
> > +++(bulk->start);
> > +
> > +/* Check for continuation if it's not the last iteration. */
> > +if ( bulk->start < max && hypercall_preempt_check() )
> > +{
> > +rc = 1;
>
> Using -ERESTART here allows...
>
> > +break;
> > +}
> > +}
> > +
> > +return rc;
> > +}
> > +
> >  int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> >  {
> >  int rc;
> > @@ -1467,6 +1498,59 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
> >  }
> >  break;
> >
> > +case XENMEM_sharing_op_bulk_share:
> > +{
> > +unsigned long max_sgfn, max_cgfn;
> > +struct domain *cd;
> > +
> > +rc = -EINVAL;
> > +if ( !mem_sharing_enabled(d) )
> > +goto out;
> > +
> > +rc =
> rcu_lock_live_remote_domain_by_id(mso.u.bulk.client_domain,
> > +   );
> > +if ( rc )
> > +goto out;
> > +
> > +rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mso.op);
> > +if ( rc )
> > +{
> > +rcu_unlock_domain(cd);
> > +goto out;
> > +}
> > +
> > +if ( !mem_sharing_enabled(cd) )
> > +{
> > +rcu_unlock_domain(cd);
> > +rc = -EINVAL;
> > +goto out;
> > +}
> > +
> > +max_sgfn = domain_get_maximum_gpfn(d);
> > +max_cgfn = domain_get_maximum_gpfn(cd);
> > +
> > +if ( max_sgfn != max_cgfn || max_sgfn < mso.u.bulk.start )
> > +{
> > +rcu_unlock_domain(cd);
> > +rc = -EINVAL;
> > +goto out;
> > +}
> > +
> > +rc = bulk_share(d, cd, max_sgfn, );
> > +if ( rc )
>
> ... this check to be selective.
>

Sure, but I don't have a usecase for returning the error code to the user
from the nominate/sharing op. The reason for that is that just return the
error condition by itself is not very uself. Furthermore, the gfn at which
the operation failed would have to be passed to allow for debugging. But
for debugging the user could also just do this loop on his side where he
would readily have this information. So I would rather just keep this op as
simple as possible.


>
> > +{
> > +if ( __copy_to_guest(arg, , 1) )
>
> This __copy_to_guest() needs to happen unconditionally before creating
> the continuation, as it contains the continuation information.
>
>
It does happen before setting up the continuation and the continuation only
gets setup if this succeeds.


> It also needs to happen on the success path, so .shared is correct.
>

It does happen on the success path below for !rc for all sharing ops.

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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Boris Ostrovsky]

On 10/09/2015 12:51 PM, Haozhong Zhang wrote:

On Fri, Oct 09, 2015 at 12:31:53PM -0400, Boris Ostrovsky wrote:

On 10/09/2015 12:19 PM, Jan Beulich wrote:

On 09.10.15 at 18:09,  wrote:

On 10/09/2015 11:11 AM, Jan Beulich wrote:

On 09.10.15 at 16:00,  wrote:

On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:

On 10/09/2015 02:51 AM, Jan Beulich wrote:

On 28.09.15 at 09:13,  wrote:

When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
the host TSC with a ratio between guest TSC rate and
nanoseconds. However, the result will be incorrect if the guest TSC rate
differs from the host TSC rate. This patch fixes this problem by using
the system time as elapsed_nsec.

For both this and patch 2, while at a first glance (and taking into
account just the visible patch context) what you say seems to
make sense, the explanation is far from sufficient namely when
looking at the function as a whole. For one, effects on existing
cases need to be explicitly described, in particular why SVM's TSC
ratio code works without that change (or whether it has been
broken all along, in which case these would become backporting
candidates; input from SVM maintainers would be appreciated
too). That may in particular mean being more specific about
what is actually wrong with scaling the host TSC here (i.e. in
which way both results differ), when supposedly that matches
what the hardware does when TSC ratio is supported.

If elapsed_nsec is the time that guest has been running then how can
get_s_time(), which is system time, be the right answer here? But what
confuses me even more is that existing code is not doing that neither.

Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
I.e.

*elapsed_nsec = get_s_time() - d->arch.vtsc_offset?


Yes, I should minus d->arch.vtsc_offset here.

In which case - afaict - the code becomes identical to that of the
TSC_MODE_ALWAYS_EMULATE case as well as the
TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
unlikely to be correct.

*elapsed_nsec = *gtsc_khz = 0; ? Because we are effectively in
TSC_MODE_NEVER.

How that? Talk here has been about TSC_MODE_DEFAULT...

AFAIUI, TSC_MODE_DEFAULT is a shorthand for saying "I will let the
hypervisor pick whether the guest will be in TSC_MODE_ALWAYS_EMULATE or
TSC_MODE_NEVER". d->arch.vtsc is what ends up being internal implementation
of user-provided mode (for the most parts; I think hvm_cpuid() being the
only true exception --- and perhaps it needs to be looked at).

So if we have d->arch.vtsc=0 (which is the case we are talking about here)
then we are really in NEVER mode


Not quite understand this. Is tsc_set_info() the only place to set
d->arch.tsc_mode ?


Yes.


Though it may decide d->arch.vtsc should be 1, it
still sets d->arch.tsc_mode to the user provided TSC mode for a
non-pvh domain. And then in tsc_get_info(), it should never fall into
TSC_MODE_NEVER_EMULATE branch if d->arch.tsc_mode is not.


I was trying to say that TSC behavior in current incarnation is 
equivalent to _NEVER if d->arch.vtsc is 0. But when we call 
tsc_get_info() we can not handle it out of _NEVER case (because, as you 
pointed out, d->arch.vtsc may change after migration). And we don't.


-boris




- Haozhong


-boris


That can't be right...

Why not? tsc_set_info() doesn't care about any of its other input
values when that mode is in effect.

Jan


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



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

Re: [Xen-devel] [PATCH] libxc: remove superpages option for pv domains

[Konrad Rzeszutek Wilk]

On Fri, Oct 09, 2015 at 04:12:12PM +0100, Ian Campbell wrote:
> On Thu, 2015-10-08 at 17:23 +0200, Juergen Gross wrote:
> > The pv domain builder currently supports the additional flag
> > "superpages" to build a pv domain with 2MB pages. This feature isn't
> > being used by any component other than the python xc bindings.
> > 
> > Remove the flag and it's support from the xc bindings and the domain
> > builder
> > 
> > Signed-off-by: Juergen Gross 
> 
> Konrad,
> 
> In <20151008011056.ga22...@andromeda.dapyr.net> you indicated you were ok
> with this, may we take that as an:
> 
> Acked-by: Konrad Rzeszutek Wilk 
> 
> for this change?
> 
> (Or maybe even an
> Acked-by: Konrad Rzeszutek Wilk 
> ?)
That one.


Acked-by: Konrad Rzeszutek Wilk 
> 
> Thanks,
> Ian.
> 
> > ---
> >  tools/libxc/include/xc_dom.h  |   1 -
> >  tools/libxc/xc_dom_x86.c  | 271 
> > --
> >  tools/python/xen/lowlevel/xc/xc.c |  10 +-
> >  3 files changed, 118 insertions(+), 164 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index e52b023..ae2b985 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -157,7 +157,6 @@ struct xc_dom_image {
> >  
> >  xc_interface *xch;
> >  domid_t guest_domid;
> > -int8_t superpages;
> >  int claim_enabled; /* 0 by default, 1 enables it */
> >  int shadow_enabled;
> >  
> > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> > index dd331bf..9d11dd3 100644
> > --- a/tools/libxc/xc_dom_x86.c
> > +++ b/tools/libxc/xc_dom_x86.c
> > @@ -1005,181 +1005,140 @@ static int meminit_pv(struct xc_dom_image *dom)
> >  return rc;
> >  }
> >  
> > -if ( dom->superpages )
> > +/* try to claim pages for early warning of insufficient memory avail
> > */
> > +if ( dom->claim_enabled )
> >  {
> > -int count = dom->total_pages >> SUPERPAGE_2MB_SHIFT;
> > -xen_pfn_t extents[count];
> > -
> > -dom->p2m_size = dom->total_pages;
> > -dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) *
> > -  dom->p2m_size);
> > -if ( dom->p2m_host == NULL )
> > -return -EINVAL;
> > -
> > -DOMPRINTF("Populating memory with %d superpages", count);
> > -for ( pfn = 0; pfn < count; pfn++ )
> > -extents[pfn] = pfn << SUPERPAGE_2MB_SHIFT;
> > -rc = xc_domain_populate_physmap_exact(dom->xch, dom
> > ->guest_domid,
> > -   count,
> > SUPERPAGE_2MB_SHIFT, 0,
> > -   extents);
> > +rc = xc_domain_claim_pages(dom->xch, dom->guest_domid,
> > +   dom->total_pages);
> >  if ( rc )
> >  return rc;
> > +}
> >  
> > -/* Expand the returned mfn into the p2m array */
> > -pfn = 0;
> > -for ( i = 0; i < count; i++ )
> > -{
> > -mfn = extents[i];
> > -for ( j = 0; j < SUPERPAGE_2MB_NR_PFNS; j++, pfn++ )
> > -dom->p2m_host[pfn] = mfn + j;
> > -}
> > +/* Setup dummy vNUMA information if it's not provided. Note
> > + * that this is a valid state if libxl doesn't provide any
> > + * vNUMA information.
> > + *
> > + * The dummy values make libxc allocate all pages from
> > + * arbitrary physical nodes. This is the expected behaviour if
> > + * no vNUMA configuration is provided to libxc.
> > + *
> > + * Note that the following hunk is just for the convenience of
> > + * allocation code. No defaulting happens in libxc.
> > + */
> > +if ( dom->nr_vmemranges == 0 )
> > +{
> > +nr_vmemranges = 1;
> > +vmemranges = dummy_vmemrange;
> > +vmemranges[0].start = 0;
> > +vmemranges[0].end   = (uint64_t)dom->total_pages << PAGE_SHIFT;
> > +vmemranges[0].flags = 0;
> > +vmemranges[0].nid   = 0;
> > +
> > +nr_vnodes = 1;
> > +vnode_to_pnode = dummy_vnode_to_pnode;
> > +vnode_to_pnode[0] = XC_NUMA_NO_NODE;
> >  }
> >  else
> >  {
> > -/* try to claim pages for early warning of insufficient memory
> > avail */
> > -if ( dom->claim_enabled ) {
> > -rc = xc_domain_claim_pages(dom->xch, dom->guest_domid,
> > -   dom->total_pages);
> > -if ( rc )
> > -return rc;
> > -}
> > +nr_vmemranges = dom->nr_vmemranges;
> > +nr_vnodes = dom->nr_vnodes;
> > +vmemranges = dom->vmemranges;
> > +vnode_to_pnode = dom->vnode_to_pnode;
> > +}
> >  
> > -/* Setup dummy vNUMA information if it's not provided. Note
> > - * that this is a valid state if libxl doesn't 

[Xen-devel] [libvirt test] 62727: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  ef4d14a116fabd14d0b806ade46ae02825e275ce
baseline version:
 libvirt  1a83a068e4b59b137eac7e34607e55d023c90894

Last test of basis62701  2015-10-06 15:08:52 Z3 days
Testing same since62727  2015-10-08 10:12:55 Z1 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Cole Robinson 
  John Ferlan 
  Maxim Nestratov 
  Michal Privoznik 
  Peter Krempa 

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



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

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

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

Re: [Xen-devel] patch "x86/cpufreq: relocate the driver register function" breaks cpu hot(un)plug

[Konrad Rzeszutek Wilk]

On Fri, Oct 09, 2015 at 06:48:23PM +0200, Dario Faggioli wrote:
> Hey,
> 
> As far as my bisection goes, commit
> 49388f11d512bb92706ce046643bfbb3c1d963c9 "x86/cpufreq: relocate the
> driver register function" prevents me from hot unplugging pCPUs.
> 
> Xen does not crash or anything, but dom0 is stalled. In fact, with
> current staging, here's what I see:
> 
> root@Zhaman:~# echo 0 > /sys/devices/system/xen_cpu/xen_cpu6/online 
> [   81.583001] INFO: rcu_sched detected stalls on CPUs/tasks: { 12} (detected 
> by 3, t=5252 jiffies, g=1691, c=1690, q=76)
> [   81.583036] Task dump for CPU 12:
> [   81.583044] bashR  running task0  1347   1094 
> 0x0008
> [   81.583056]     
> 8800192c2e38
> [   81.583070]  888472e8 0002 888472e8 
> 880013817858
> [   81.583082]   81a4 811e8137 
> 8800192c2e38
> [   81.583095] Call Trace:
> [   81.583110]  [] ? notify_change+0x2f7/0x390
> [   81.583148]  [] ? do_truncate+0x74/0x90
> [   81.583158]  [] ? dput+0x26/0x230
> [   81.583167]  [] ? terminate_walk+0x35/0x40
> [   81.583176]  [] ? do_last+0x621/0x12c0
> [   81.583188]  [] ? xen_pcpu_down+0x47/0x70
> [   81.583199]  [] ? store_online+0x9d/0xb0
> [   81.583210]  [] ? kernfs_fop_write+0x12c/0x180
> [   81.583220]  [] ? __vfs_write+0x23/0xf0
> [   81.583230]  [] ? __sb_start_write+0x42/0xf0
> [   81.583241]  [] ? security_file_permission+0x21/0xa0
> [   81.583250]  [] ? vfs_write+0xa1/0x1c0
> [   81.583259]  [] ? filp_close+0x4f/0x70
> [   81.583268]  [] ? SyS_write+0x42/0xb0
> [   81.583277]  [] ? __close_fd+0x71/0xb0
> [   81.583287]  [] ? system_call_fastpath+0x16/0x75
> [  144.555020] INFO: rcu_sched detected stalls on CPUs/tasks: { 12} (detected 
> by 4, t=21007 jiffies, g=1691, c=1690, q=244)
> [  144.555046] Task dump for CPU 12:
> [  144.555051] bashR  running task0  1347   1094 
> 0x0008
> [  144.555059]     
> 8800192c2e38
> [  144.555068]  888472e8 0002 888472e8 
> 880013817858
> [  144.555076]   81a4 811e8137 
> 8800192c2e38
> [  144.555084] Call Trace:
> [  144.555096]  [] ? notify_change+0x2f7/0x390
> [  144.555105]  [] ? do_truncate+0x74/0x90
> [  144.555112]  [] ? dput+0x26/0x230
> [  144.555118]  [] ? terminate_walk+0x35/0x40
> [  144.555124]  [] ? do_last+0x621/0x12c0
> [  144.555164]  [] ? xen_pcpu_down+0x47/0x70
> [  144.555172]  [] ? store_online+0x9d/0xb0
> [  144.555179]  [] ? kernfs_fop_write+0x12c/0x180
> [  144.555186]  [] ? __vfs_write+0x23/0xf0
> [  144.555192]  [] ? __sb_start_write+0x42/0xf0
> [  144.555200]  [] ? security_file_permission+0x21/0xa0
> [  144.555206]  [] ? vfs_write+0xa1/0x1c0
> [  144.555212]  [] ? filp_close+0x4f/0x70
> [  144.555217]  [] ? SyS_write+0x42/0xb0
> [  144.555223]  [] ? __close_fd+0x71/0xb0
> [  144.555230]  [] ? system_call_fastpath+0x16/0x75
> 
> If I revert that patch, the issue goes away.
> 
> Any ideas? 

I think it is due to xen-acpi-processor re-uploading the C and P states
whenever an CPU goes up. It also does this after S3 suspend.

Anyhow it may be due to the fact that cpufreq_register_driver in Xen is now
'__init' If you remove that little thing would it work?

> 
> Regards,
> Dario
> 
> PS. yes, I'll implement a cpu hotplug/unplug testcase ASAP. :-)
> 
> -- 
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)
> 



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


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

Re: [Xen-devel] [OSSTEST PATCH 2/2] make-flight: create the vNUMA HVM test job

[Ian Campbell]

On Fri, 2015-10-02 at 01:17 +0200, Dario Faggioli wrote:

>  make-flight |   36 ++--

FWIW the make-flight side of this looks fine to me.

We discussed the save-restore allowances already.


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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Jan Beulich]

>>> On 09.10.15 at 16:35,  wrote:
> On Fri, Oct 09, 2015 at 12:51:32AM -0600, Jan Beulich wrote:
>> >>> On 28.09.15 at 09:13,  wrote:
>> > When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
>> > is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
>> > the host TSC with a ratio between guest TSC rate and
>> > nanoseconds. However, the result will be incorrect if the guest TSC rate
>> > differs from the host TSC rate. This patch fixes this problem by using
>> > the system time as elapsed_nsec.
>> 
>> For both this and patch 2, while at a first glance (and taking into
>> account just the visible patch context) what you say seems to
>> make sense, the explanation is far from sufficient namely when
>> looking at the function as a whole. For one, effects on existing
>> cases need to be explicitly described, in particular why SVM's TSC
>> ratio code works without that change (or whether it has been
>> broken all along, in which case these would become backporting
>> candidates; input from SVM maintainers would be appreciated
>> too). That may in particular mean being more specific about
>> what is actually wrong with scaling the host TSC here (i.e. in
>> which way both results differ), when supposedly that matches
>> what the hardware does when TSC ratio is supported.
>> 
>> Then a reason needs to be given why the similar logic in the
>> PVRDTSCP case does not also get adjusted.
>> 
>> Plus, looking at the respective code in tsc_set_info(), I'm
>> getting the impression that what you're trying to do is not in line
>> with what is intended so far: Especially the comment there
>> suggests that the intention is for the guest TSC to be made
>> match the host one. Considering migration this indeed looks
>> suspicious, but then that would need changing too.
>>
> 
> Do you mean the following comment?
> /*
>  * In default mode use native TSC if the host has safe TSC and:
>  *  HVM/PVH: host and guest frequencies are the same (either
>  *   "naturally" or via TSC scaling)
>  *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
>  */
>
> To my understanding,
> 
> 1. "naturally" responds to the case that a domain is
>newly created (rather than being migrated from other machine) so that
>its TSC frequency (d->arch.tsc_khz) is identical to the host TSC
>frequency (cpu_khz).
> 
> 2. "via TSC scaling" means the case that the domain is migrated from
>another machine of different host TSC rate so that d->arch.tsc_khz
>!= cpu_khz. In this case the guest still reads the (host) TSC
>natively, but SVM TSC ratio makes sure that TSC value is a scaled
>host TSC. This point can be confirmed by svm_tsc_ratio_load() which
>sets MSR_AMD64_TSC_RATIO to d->arch.tsc_khz/cpu_khz.

I.e. they are _not_ the same (unless the quotient happens to be 1,
in which case scaling wouldn't be necessary in the first place). I.e.
imo the comment would need to be

/*
 * In default mode use native TSC if the host has safe TSC and:
 *  HVM/PVH: host and guest frequencies are the same or TSC
 *   scaling is in use
 *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
 */

Jan


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

Re: [Xen-devel] [PATCH v1 2/4] xen-version: Add third parameter (len) to the do_version hypercall.

[Ian Campbell]

On Fri, 2015-10-09 at 08:38 -0600, Jan Beulich wrote:
> > > > On 09.10.15 at 14:46,  wrote:
> > On Fri, 2015-10-09 at 13:29 +0100, Andrew Cooper wrote:
> > > On 09/10/15 09:25, Jan Beulich wrote:
> > > > > > > On 09.10.15 at 04:56,  wrote:
> > > > > All existing commands ignore the parameter so this does
> > > > > not break the ABI.
> > > > Does it not? What about the debug mode clobbering of hypercall
> > > > argument registers?
> > > 
> > > That is an implementation detail of the hypervisor.  It is irrelevant
> > > to
> > > guests whether Xen chooses to clobber the spare registers or not.
> > 
> > Or in other words the effect here is to clobber one _less_ register,
> > and
> > the guest cannot have been relying on a register getting so clobbered
> > (if
> > nothing else it doesn't happen in debug=n builds).
> 
> No, the one less register clobbered is in the first clobbering phase,
> where _unused_ inputs get clobbered (for hypervisor internal
> consumption). The second clobbering phase destroys all _used_
> input registers' contents (the guest visible values), and _this_ is
> what results in ABI breakage (because callers assuming the
> hypercall to take two arguments assume that the 3rd argument
> register will retain its contents.

Ah, yes, that's correct. My mistake.

Ian.

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

Re: [Xen-devel] [PATCH 1/2] xen/arm: Add support of PSCI v1.0 for the host

[Ian Campbell]

On Thu, 2015-10-08 at 19:45 +0100, Julien Grall wrote:
> From Xen point of view, PSCI v0.2 and PSCI v1.0 are very similar. All
> the PSCI calls used within Xen (PSCI_VERSION, CPU_ON, SYSTEM_OFF and
> SYSTEM_RESET) behaves exactly the same.
> 
> While there is no compatible string to represent PSCI v1.0 in the DT,
> it's possible to detect it using the function PSCI_VERSION.
> 
> The compatible string is now used to detect if the platform may support
> PSCI v0.2 or higher.

The actual implementation here looks for precisely 0.2 or 1.0, not >= 0.2
as suggested by this statement.

The PSCI 1.0 spec says (section 5.3.1, intended use of PSCI_VERSION) that
for any 1.y version must be compatible with 1.x when y>x (for those
functions which existed in 1.x, y might have more).

IOW an OS supporting 1.0 should work with any 1.x.

(which begs the question why there is not a "arm,psci-1.x" compat string,
Mark/Andre?)

> 
> Signed-off-by: Julien Grall 
> 
> ---
> 
> Cc: Andre Przywara 
> Cc: Mark Rutland 
> ---
>  xen/arch/arm/psci.c|  9 +
>  xen/include/asm-arm/psci.h | 13 +
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 172c6e7..53ee2e4 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -122,15 +122,16 @@ int __init psci_init_0_2(void)
>  
>  psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>  
> -if ( psci_ver != XEN_PSCI_V_0_2 )
> +if ( psci_ver != PSCI_VERSION(0, 2) && psci_ver != PSCI_VERSION(1, 0) )

Based on the above I think this should read:

if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_MAJOR_VERSION(psci_ver) != 1 )

>  {
> -printk("Error: PSCI version %#x is not supported.\n", psci_ver);
> -return -EOPNOTSUPP;
> +printk("Error: Conflicting PSCI version detected (%#x)\n", psci_ver);

Conflicting with what?

I think perhaps you meant "Unrecognised" or "Unsupported"?

Also please format the version like you did below with %u.%u.

>  }
>  
>  psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
>  
> -printk(XENLOG_INFO "Using PSCI-0.2 for SMP bringup\n");
> +printk(XENLOG_INFO "Using PSCI-%u.%u for SMP bringup\n",
> +   PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
>  
>  return 0;
>  }


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

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()

[Jan Beulich]

>>> On 09.10.15 at 16:00,  wrote:
> On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:
>> On 10/09/2015 02:51 AM, Jan Beulich wrote:
>> On 28.09.15 at 09:13,  wrote:
>> >>When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation
>> >>is used, the existing tsc_get_info() calculates elapsed_nsec by scaling
>> >>the host TSC with a ratio between guest TSC rate and
>> >>nanoseconds. However, the result will be incorrect if the guest TSC rate
>> >>differs from the host TSC rate. This patch fixes this problem by using
>> >>the system time as elapsed_nsec.
>> >For both this and patch 2, while at a first glance (and taking into
>> >account just the visible patch context) what you say seems to
>> >make sense, the explanation is far from sufficient namely when
>> >looking at the function as a whole. For one, effects on existing
>> >cases need to be explicitly described, in particular why SVM's TSC
>> >ratio code works without that change (or whether it has been
>> >broken all along, in which case these would become backporting
>> >candidates; input from SVM maintainers would be appreciated
>> >too). That may in particular mean being more specific about
>> >what is actually wrong with scaling the host TSC here (i.e. in
>> >which way both results differ), when supposedly that matches
>> >what the hardware does when TSC ratio is supported.
>> 
>> If elapsed_nsec is the time that guest has been running then how can
>> get_s_time(), which is system time, be the right answer here? But what
>> confuses me even more is that existing code is not doing that neither.
>> 
>> Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side?
>> I.e.
>> 
>> *elapsed_nsec = get_s_time() - d->arch.vtsc_offset?
>>
> 
> Yes, I should minus d->arch.vtsc_offset here.

In which case - afaict - the code becomes identical to that of the
TSC_MODE_ALWAYS_EMULATE case as well as the
TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
unlikely to be correct.

Jan


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

Re: [Xen-devel] [PATCH 2/2] xen/arm: Replace XEN_PSCI_* by PSCI_VERSION(major, minor)

[Ian Campbell]

On Thu, 2015-10-08 at 19:45 +0100, Julien Grall wrote:
> It will avoid to introduce a new XEN_PSCI_* define every time we support
> a new version of PSCI in Xen.
> 
> Also fix the coding style in modified place.
> 
> Signed-off-by: Julien Grall 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH 1/2] xen/arm: Add support of PSCI v1.0 for the host

[Julien Grall]

Hi Ian,

On 09/10/15 16:08, Ian Campbell wrote:
> On Thu, 2015-10-08 at 19:45 +0100, Julien Grall wrote:
>> From Xen point of view, PSCI v0.2 and PSCI v1.0 are very similar. All
>> the PSCI calls used within Xen (PSCI_VERSION, CPU_ON, SYSTEM_OFF and
>> SYSTEM_RESET) behaves exactly the same.
>>
>> While there is no compatible string to represent PSCI v1.0 in the DT,
>> it's possible to detect it using the function PSCI_VERSION.
>>
>> The compatible string is now used to detect if the platform may support
>> PSCI v0.2 or higher.
> 
> The actual implementation here looks for precisely 0.2 or 1.0, not >= 0.2
> as suggested by this statement.

The first implementation I did was based on the Linux one which is
working checking if the PSCI version if >= 0.2.

Although I changed my mind before sending the patch because I was worry
to see Xen breaking badly when booting on another version of PSCI.

> 
> The PSCI 1.0 spec says (section 5.3.1, intended use of PSCI_VERSION) that
> for any 1.y version must be compatible with 1.x when y>x (for those
> functions which existed in 1.x, y might have more).
> IOW an OS supporting 1.0 should work with any 1.x.

Right, I will update the check.

> (which begs the question why there is not a "arm,psci-1.x" compat string,
> Mark/Andre?)
> 
>>
>> Signed-off-by: Julien Grall 
>>
>> ---
>>
>> Cc: Andre Przywara 
>> Cc: Mark Rutland 
>> ---
>>  xen/arch/arm/psci.c|  9 +
>>  xen/include/asm-arm/psci.h | 13 +
>>  2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
>> index 172c6e7..53ee2e4 100644
>> --- a/xen/arch/arm/psci.c
>> +++ b/xen/arch/arm/psci.c
>> @@ -122,15 +122,16 @@ int __init psci_init_0_2(void)
>>  
>>  psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>>  
>> -if ( psci_ver != XEN_PSCI_V_0_2 )
>> +if ( psci_ver != PSCI_VERSION(0, 2) && psci_ver != PSCI_VERSION(1, 0) )
> 
> Based on the above I think this should read:
> 
> if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_MAJOR_VERSION(psci_ver) != 1 )
> 
>>  {
>> -printk("Error: PSCI version %#x is not supported.\n", psci_ver);
>> -return -EOPNOTSUPP;
>> +printk("Error: Conflicting PSCI version detected (%#x)\n", 
>> psci_ver);
> 
> Conflicting with what?
> I think perhaps you meant "Unrecognised" or "Unsupported"?

It's just a mistake. When I first wrote the patch the check was:
PSCI_MAJOR_VERSION(psci_vers) == 0 && PSCI_MINOR_VERSION(psci_vers) < 2

Although, I was worry about allowing to many version of PSCI. I will use
your suggestion.

> Also please format the version like you did below with %u.%u.

I will do.

> 
>>  }
>>  
>>  psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
>>  
>> -printk(XENLOG_INFO "Using PSCI-0.2 for SMP bringup\n");
>> +printk(XENLOG_INFO "Using PSCI-%u.%u for SMP bringup\n",
>> +   PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
>>  
>>  return 0;
>>  }
> 

Regards,

-- 
Julien Grall

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