date:20150917



> -Original Message-
> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of George
> Dunlap
> Sent: Thursday, September 17, 2015 12:57 AM
> To: Jan Beulich
> Cc: Wu, Feng; Tian, Kevin; Keir Fraser; Andrew Cooper; Dario Faggioli;
> xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for
> VT-d posted interrupts
> 
> On Mon, Sep 7, 2015 at 1:54 PM, Jan Beulich  wrote:
>  On 25.08.15 at 03:57,  wrote:
> >> --- a/xen/arch/x86/domain.c
> >> +++ b/xen/arch/x86/domain.c
> >> @@ -1573,6 +1573,22 @@ static void __context_switch(void)
> >>  per_cpu(curr_vcpu, cpu) = n;
> >>  }
> >>
> >> +static inline void pi_ctxt_switch_from(struct vcpu *prev)
> >> +{
> >> +/*
> >> + * When switching from non-idle to idle, we only do a lazy context
> switch.
> >> + * However, in order for posted interrupt (if available and enabled) 
> >> to
> >> + * work properly, we at least need to update the descriptors.
> >> + */
> >> +if ( prev->arch.pi_ctxt_switch_from && !is_idle_vcpu(prev) )
> >> +prev->arch.pi_ctxt_switch_from(prev);
> >> +}
> >> +
> >> +static inline void pi_ctxt_switch_to(struct vcpu *next)
> >> +{
> >> +if ( next->arch.pi_ctxt_switch_to && !is_idle_vcpu(next) )
> >> +next->arch.pi_ctxt_switch_to(next);
> >> +}
> >>
> >>  void context_switch(struct vcpu *prev, struct vcpu *next)
> >>  {
> >> @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct
> vcpu *next)
> >>
> >>  set_current(next);
> >>
> >> +pi_ctxt_switch_from(prev);
> >> +
> >>  if ( (per_cpu(curr_vcpu, cpu) == next) ||
> >>   (is_idle_domain(nextd) && cpu_online(cpu)) )
> >>  {
> >> +pi_ctxt_switch_to(next);
> >>  local_irq_enable();
> >
> > This placement, if really intended that way, needs explanation (in a
> > comment) and perhaps even renaming of the involved symbols, as
> > looking at it from a general perspective it seems wrong (with
> > pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
> > want this only when switching back to what got switched out lazily
> > before, i.e. this would be not something to take place on an arbitrary
> > context switch). As to possible alternative names - maybe make the
> > hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
> 
> Why on earth is this more clear than what he had before?
> 
> In the first call, he's not "preparing" anything -- he's actually
> switching the PI context out for prev.  And in the second call, he's
> not "cancelling" anything -- he's actually switching the PI context in
> for next.  The names you suggest are actively confusing, not helpful.
> 
> But before talking about how to make things more clear, one side
> question -- do we need to actually call pi_ctxt_switch_to() in
> __context_switch()?
> 
> The only other place __context_switch() is called is
> from__sync_local_execstate().  But the only reason that needs to be
> called is because sometimes we *don't* call __context_switch(), and so
> there are things on the cpu that aren't copied into the vcpu struct.

Thanks for the comments!

>From my understanding, __sync_local_execstate() can only get called
in the following two cases:
#1) this_cpu(curr_vcpu) == current, in this case, __context_switch() is
not called.
#2) this_cpu(curr_vcpu) != current, and current == idle_vcpu, that means
we just switched from a non-idle vCPU to idle vCPU, so here we need to
call __context_switch() to copy things to the original vcpu struct.

Please correct me if the above understanding is wrong or incomplete?

I think calling pi_ctxt_switch_to() in __context_switch() is needed when
we are switching to a non-idle vCPU (we need change the PI state of the
target vCPU), and the call is not needed when switching to idle vCPU.
So if the above understanding is correct, I think you suggestion below
is really good, it makes things clearer.

> 
> That doesn't apply to the PI state -- for one, nothing is copied from
> the processor; and for two, pi_ctxt_switch_from() is called
> unconditionally anyway.
> 
> Would it make more sense to call pi_context_switch(prev, next) just
> after "set_current"?

I think it is a good point.

Thanks,
Feng

> 
> (Keeping in mind I totally may have missed something...)
> 
>  -George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [xen-4.6-testing test] 62015: regressions - FAIL

2015-09-17 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 61745

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start.2 fail REGR. vs. 61745
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 19 guest-start/debianhvm.repeat 
fail like 61745

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 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-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-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-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-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-qcow2 11 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-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-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  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-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-i386-libvirt  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-i386-xl-qemuu-win7-amd64 17 guest-stop  fail 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-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  70d63e48077f8fee8eda6d8d95eeda52a34d9077
baseline version:
 xen  a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d

Last test of basis61745  2015-09-10 11:13:18 Z6 days
Failing since 61839  2015-09-12 07:49:27 Z4 days2 attempts
Testing same since62015  2015-09-14 22:22:30 Z2 days1 attempts


People who touched revisions under test:
  Boris Ostrovsky 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Kevin Tian 
  Kouya Shimura 
  Stefano Stabellini 
  Tiejun Chen 
  Wei Liu 

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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts



> -Original Message-
> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of George
> Dunlap
> Sent: Thursday, September 17, 2015 1:08 AM
> To: Wu, Feng
> Cc: Jan Beulich; Tian, Kevin; Keir Fraser; Andrew Cooper; Dario Faggioli;
> xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for
> VT-d posted interrupts
> 
> On Thu, Sep 10, 2015 at 9:59 AM, Wu, Feng  wrote:
> >> >> >> Calling vcpu_unblock() in the middle of context_switch()? Why? And
> >> >> >> is this safe?
> >> >> >
> >> >> > I cannot see anything unsafe so far, can some scheduler maintainer
> >> >> > help to confirm it? Dario? George?
> >> >>
> >> >> But first and foremost you ought to answer the "why".
> >> >
> >> > I think if you think this solution is unsafe, you need point out where it
> >> > is, not
> >> > just ask "is this safe ?", I don't think this is an effective comments.
> >>
> >> It is my general understanding that people wanting code to be
> >> included have to prove their code is okay, not reviewers to prove
> >> the code is broken.
> >>
> >> > Anyway, I go through the main path of the code as below, I really don't 
> >> > find
> >> > anything unsafe here.
> >> >
> >> > context_switch() -->
> >> > pi_ctxt_switch_from() -->
> >> > vmx_pre_ctx_switch_pi() -->
> >> > vcpu_unblock() -->
> >> > vcpu_wake() -->
> >> > SCHED_OP(VCPU2OP(v), wake, v) -->
> >> > csched_vcpu_wake() -->
> >> > __runq_insert()
> >> > __runq_tickle()
> >> >
> >> > If you continue to think it is unsafe, pointing out the place will be
> >> > welcome!
> >>
> >> Once again - I didn't say it's unsafe (and hence can't point at a
> >> particular place). What bothers me is that vcpu_unblock() affects
> >> scheduler state, and altering scheduler state from inside the
> >> context switch path looks problematic at the first glance. I'd be
> >> happy if I was told there is no such problem, but be aware that
> >> this would have to include latent ones: Even if right now things
> >> are safe, having such feedback have the potential of becoming
> >> an actual problem with a later scheduler change is already an
> >> issue, as finding the problem is then likely going to be rather hard
> >> (and I suspect it's not going to be you to debug this).
> >
> > What I can say is that after investigation, I don't find anything bad
> > for this vcpu_unblock(), I tested it in my experiment. So that is why
> > I'd like to ask some ideas from scheduler expects.
> 
> You still didn't answer Jan's question as to why you chose to call it there.

The reason why I need to call vcpu_unblock() here is that we are about
to block the vCPU and put it on the blocking list if everything goes okay,
but if during updating the posted-interrupt descriptor 'ON' is set, which
means there is a new interrupt coming for this vCPU, we shouldn't block
it, so I call vcpu_unblock to do it.

> 
> As to why Jan is suspicious, the hypervisor code is incredibly
> complicated, and even hypervisor maintainers are only mortals. :-) One
> way we deal with the complication is by trying to restrict the ways
> different code interacts, so that people reading, writing, and
> maintaining the code can make simplifying assumptions to make it
> easier to grasp / harder to make mistakes.
> 
> People reading or working on vcpu_wake() code expect it to be called
> from interrupt contexts, and also expect it to be called from normal
> hypercalls.  They *don't* expect it to be called from in the middle of
> a context switch, where "current", the registers, and all those things
> are all up in the air.  It's possible that the code *already*
> implicitly assumes it's not called from context switch (i.e., that v
> == current means certain state), and even if not, it's possible
> someone will make that assumption in the future, causing a very
> hard-to-detect bug.
> 
> Now I haven't looked at the code in detail yet; it may be that the
> only way to make PI work is to make this call.  If that's the case we
> need to carefully examine assumptions, and carefully comment the code
> to make sure people working on the scheduling code continue to think
> carefully about their assumptions in the future.  But if we can get
> away without doing that, it will make things much easier.
> 
> But it's certainly reasonable to expect the maintainers to offer you
> an alternate solution if your proposed solution is unacceptable. :-)
> Let me take a look and see what I think.

Really appreciate your detailed description about this, it is reasonable,
and thank you from your comments!

Thanks,
Feng

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

Re: [Xen-devel] [PATCH v1 5/5] xsplice: Use ld-embedded build-ids

2015-09-17 Thread Martin Pohlack

On 17.09.2015 00:31, Andrew Cooper wrote:
> On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote:
>> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper 
>>  wrote:
>>> On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
 From: Martin Pohlack 

 The mechanism to get this is via the XSPLICE_OP and
 we add a new subsequent hypercall to retrieve the
 binary build-id. The hypercall allows an arbirarty
 size (the buffer is provided to the hypervisor) - however
 by default the toolstack will allocate it up to 128
 bytes.

 We also add two places for the build-id to be printed:
  - xsplice keyhandler. We cannot use 'hh' in the hypervisor
snprintf handler (as it is not implemented) so instead
we use an simpler way to print it.
  - In the 'xen-xsplice' tool add an extra parameter - build-id
to print this as an human readable value.

 Note that one can also retrieve the value by 'readelf -h xen-syms'.

 Signed-off-by: Martin Pohlack 
 Signed-off-by: Konrad Rzeszutek Wilk 
 ---
  tools/libxc/include/xenctrl.h |  1 +
  tools/libxc/xc_misc.c | 26 +
  tools/misc/xen-xsplice.c  | 39 
  xen/arch/x86/Makefile |  4 +-
  xen/arch/x86/xen.lds.S|  5 +++
  xen/common/xsplice.c  | 86
>>> +++
  xen/include/public/sysctl.h   | 18 +
  xen/include/xen/version.h |  1 +
  8 files changed, 178 insertions(+), 2 deletions(-)

 diff --git a/tools/libxc/include/xenctrl.h
>>> b/tools/libxc/include/xenctrl.h
 index 2cd982d..946ddc0 100644
 --- a/tools/libxc/include/xenctrl.h
 +++ b/tools/libxc/include/xenctrl.h
 @@ -2860,6 +2860,7 @@ int xc_xsplice_apply(xc_interface *xch, char
>>> *id);
  int xc_xsplice_revert(xc_interface *xch, char *id);
  int xc_xsplice_unload(xc_interface *xch, char *id);
  int xc_xsplice_check(xc_interface *xch, char *id);
 +int xc_xsplice_build_id(xc_interface *xch, char *build_id, unsigned
>>> int max);
>>>
>>> The build id of the current running hypervisor should belong in the
>>> xeninfo hypercall.  It is not specific to xsplice.
>>
>> However in the previous reviews it was pointed out that it should only be 
>> accessible to dom0.
>>
>> Or to any domains as long as the XSM allows (and is turned on) - so not the 
>> default dummy one.
>>
>> That is a bit of 'if' extra complexity which I am not sure is worth it?
> 
> DomU can already read the build information such as changeset, compile
> time, etc.  Build-id is no more special or revealing.

I would take this as an argument *against* giving DomU access to those
pieces of information in details and not as an argument for
*additionally* giving it access to build-id.

With build-id we have the chance to shape a not-yet-established API and
I would like to follow the Principle of least privilege wherever it
makes sense.

To reach a similar security level with the existing API, it might make
sense to limit DomU access to compile date, compile time, compiled by,
compiled domain, compiler version and command line details, xen extra
version, and xen changeset.  Basically anything that might help DomUs to
uniquely identify a Xen build.

The approach can not directly drop access to those fields, as that would
break an existing API, but it could restrict the detail level handed out
to DomU.

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Ingo Molnar

* Andy Lutomirski  wrote:

> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.
> 
> This is IMO awful: it papers over bugs.  In particular, KVM gueests
> might be unwittingly depending on this behavior because
> CONFIG_KVM_GUEST currently depends on CONFIG_PARAVIRT.  I'm not
> aware of any such problems, but applying this series would be a good
> way to shake them out.
> 
> Fix it so that the MSR operations work the same on CONFIG_PARAVIRT=n
> and CONFIG_PARAVIRT=y as long as Xen isn't being used.  The Xen
> maintainers are welcome to make a similar change on top of this.
> 
> Since there's plenty of time before the next merge window, I think
> we should apply and fix anything that breaks.

No, I think we should at most generate a warning instead, and not crash the 
kernel 
via rdmsr()!

Most big distro kernels on bare metal have CONFIG_PARAVIRT=y (I checked Ubuntu 
and 
Fedora), so we are potentially exposing a lot of users to problems.

Crashing the bootup on an unknown MSR is bad. Many MSR reads and writes are 
non-critical and returning the 'safe' result is much better than crashing or 
hanging the bootup.

( We should double check that rdmsr()/wrmsr() results are never left 
  uninitialized, but are set to zero or so, for cases where the return code is 
not 
  checked. )

Thanks,

Ingo

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

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

2015-09-17 Thread Chun Yan Liu



>>> On 9/9/2015 at 12:52 AM, in message <55ef1244@citrix.com>, George Dunlap
 wrote: 
> On 09/08/2015 03:17 PM, Ian Campbell wrote: 
> > On Mon, 2015-08-10 at 18:35 +0800, Chunyan Liu wrote: 
> >  
> > Sorry for the delay, between 4.6 freeze crunch, conference and vacation 
> > I've been a bit swamped. 
> >  
> > I'm just going to comment on the APIs (mainly public libxl.h and .idl) in 
> > this pass. 
> >  
> >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> >> index 5f9047c..05b6331 100644 
> >> --- a/tools/libxl/libxl.h 
> >> +++ b/tools/libxl/libxl.h 
> >> @@ -123,6 +123,23 @@ 
> >>  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 
> >>   
> >>  /* 
> >> + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of 
> >  
> > And cold-plug, no? 
>  
> So you should probably say something like "indicates functions for 
> plugging in USB devices through pvusb -- both hotplug and at domain 
> creation time." 
>  
> >> +libxl_usbctrl_type = Enumeration("usbctrl_type", [ 
> >> +(0, "AUTO"), 
> >  
> > What are the proposed semantics of using LIBXL_USBCTRL_TYPE_AUTO? 
>  
> Generally "DTRT".  Meaning: 
> 1. If your domain has no devicemodel, use PV. 
> 2. If your device has a devicemodel, and no PV drivers have peen 
> detected, use the devicemodel. 
> 3. If your device has a devicemodel, but PV drivers have been detected, 
> use PV. 
>  
> At the moment we don't have a way to check for PV drivers, so this just 
> collapses down to "PV for domains without a DM and DM for domains with a 
> DM." 
>  
> >  
> >> +(1, "PV"), 
> >> +(2, "QEMU"), 
> >  
> > Is "QEMU" what we want here, as opposed to, say, "EMU" (similar to NICs)? 
>  
> I had this as "DEVICEMODEL", since what we mean is that we want the 
> device model to provide access (and in theory in the future we may use a 
> different device model).  But "EMU" works for me too. 
>  
> > I think we probably don't want to go as fine grained as "XHCI" and "EHCI" 
> > etc, do we? I see we have a version field below, is it intended that there 
> > be some way to select between e.g. UHCI and OHCI (which IIRC are different 
> > USB 1.0 controllers). 
> >  
> > Maybe these questions should all be left aside for when QMEU support is 
> > actually added (AFAICT this field is just a placeholder)? In fact I glanced 
> > at the code and was surprised to find nothing checking for 
> > LIBXL_USBCTRL_TYPE at all, did I miss something? 
> >  
> > I think the two choices are: 
> >  
> > We can decide quickly and easily what the option(s) other than PV should be 
> > here and you include it in the IDL, but you would then need to check 
> > usbctrl->type == PV at various points, not silently treat all options as 
> > PV. 
> >  
> > Or this becomes a long conversation in which case I think your best bet 
> > would be to leave the enum with just the PV (and maybe AUTO) entries and 
> > leave the decision on the name for the emulated option to the series which 
> > implements that. 
>  
> I think the idea was to simply offer 1, 2, and 3 as options, and for the 
> devicemodel version, choose a suitable controller (or set of 
> controllers) for each option; similar to what usbversion= does now.

 
Hi, George,

I'm still confused about the expected look concerning PV/EMU type handling in
this patch series.

In earlier version, we tried to extract common things in libxl_usb.c and put
pvusb specific thing in libxl_pvusb.c, prefixed with pvusb_xxx. As you
suggested, we can leave that when EMU USB patch series added.

Now, about how to handle PV/EMU type in this patch series, I can think of 3 
ways:

1. We define the enumeration (contains PV/AUTO only, user interface only allows
'pv' or 'not specified', so we handle everything in 'pv' way without further 
check. Leave check and other adjusting things when EMU USB patch series added.

2. We check domain type and set proper type if not specified (i.e. 'pv' for PV 
guest, 'emu' for HVM guest). In add/remove function, check if type='emu', 
report  
'not supported' directly; otherwise, continue do following things. When EMU USB
patch serires added, need to extract common things and adjust the check place.

3. Same as 2, but extract common things, only in PV/EMU USB specific part, check
type, if type='emu', report 'not supported'; otherwise, do pvusb work. When 
adding EMU USB patch series, only need to add EMU USB specific things in the
type='emu' branch.

Which one is expected? Or none?

- Chunyan

>  
> >  
> >> +]) 
> >> + 
> >> +libxl_usbdev_type = Enumeration("usbdev_type", [ 
> >> +(0, "invalid"), 
> >> +(1, "hostdev"), 
> >> +]) 
> >> + 
> >> +libxl_device_usbctrl = Struct("device_usbctrl", [ 
> >> +("type", libxl_usbctrl_type), 
> >> +("devid", libxl_devid), 
> >> +("version", integer), 
> >> +("ports", integer), 
> >> +("backend_domid", libxl_domid), 
> >> +("backend_domname", string), 
> >> +   ]) 
> >> + 
> >> +libxl_device_usb =

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

On Thu, 2015-09-17 at 01:22 +, osstest service owner wrote:
> flight 62004 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/62004/
>
>  test-amd64-amd64-libvirt-pairpass   
>  test-amd64-i386-libvirt-pair pass   

Although the overall flight failed these two are the first pass of the
libvirt migration tests after the recent fixed went in to osstest and
libxl.

Looks like xen-4.6-testing is already passing too (flight 62015).

Ian.

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

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

2015-09-17 Thread Chun Yan Liu



>>> On 9/9/2015 at 12:52 AM, in message <55ef1244@citrix.com>, George Dunlap
 wrote: 
> On 09/08/2015 03:17 PM, Ian Campbell wrote: 
> > On Mon, 2015-08-10 at 18:35 +0800, Chunyan Liu wrote: 
> >  
> > Sorry for the delay, between 4.6 freeze crunch, conference and vacation 
> > I've been a bit swamped. 
> >  
> > I'm just going to comment on the APIs (mainly public libxl.h and .idl) in 
> > this pass. 
> >  
> >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> >> index 5f9047c..05b6331 100644 
> >> --- a/tools/libxl/libxl.h 
> >> +++ b/tools/libxl/libxl.h 
> >> @@ -123,6 +123,23 @@ 
> >>  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 
> >>   
> >>  /* 
> >> + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of 
> >  
> > And cold-plug, no? 
>  
> So you should probably say something like "indicates functions for 
> plugging in USB devices through pvusb -- both hotplug and at domain 
> creation time." 
>  
> >> +libxl_usbctrl_type = Enumeration("usbctrl_type", [ 
> >> +(0, "AUTO"), 
> >  
> > What are the proposed semantics of using LIBXL_USBCTRL_TYPE_AUTO? 
>  
> Generally "DTRT".  Meaning: 
> 1. If your domain has no devicemodel, use PV. 
> 2. If your device has a devicemodel, and no PV drivers have peen 
> detected, use the devicemodel. 
> 3. If your device has a devicemodel, but PV drivers have been detected, 
> use PV. 
>  
> At the moment we don't have a way to check for PV drivers, so this just 
> collapses down to "PV for domains without a DM and DM for domains with a 
> DM." 
>  
> >  
> >> +(1, "PV"), 
> >> +(2, "QEMU"), 
> >  
> > Is "QEMU" what we want here, as opposed to, say, "EMU" (similar to NICs)? 
>  
> I had this as "DEVICEMODEL", since what we mean is that we want the 
> device model to provide access (and in theory in the future we may use a 
> different device model).  But "EMU" works for me too. 
>  
> > I think we probably don't want to go as fine grained as "XHCI" and "EHCI" 
> > etc, do we? I see we have a version field below, is it intended that there 
> > be some way to select between e.g. UHCI and OHCI (which IIRC are different 
> > USB 1.0 controllers). 
> >  
> > Maybe these questions should all be left aside for when QMEU support is 
> > actually added (AFAICT this field is just a placeholder)? In fact I glanced 
> > at the code and was surprised to find nothing checking for 
> > LIBXL_USBCTRL_TYPE at all, did I miss something? 
> >  
> > I think the two choices are: 
> >  
> > We can decide quickly and easily what the option(s) other than PV should be 
> > here and you include it in the IDL, but you would then need to check 
> > usbctrl->type == PV at various points, not silently treat all options as 
> > PV. 
> >  
> > Or this becomes a long conversation in which case I think your best bet 
> > would be to leave the enum with just the PV (and maybe AUTO) entries and 
> > leave the decision on the name for the emulated option to the series which 
> > implements that. 
>  
> I think the idea was to simply offer 1, 2, and 3 as options, and for the 
> devicemodel version, choose a suitable controller (or set of 
> controllers) for each option; similar to what usbversion= does now.

 
Hi, George,

I'm still confused about the expected look concerning PV/EMU type handling in
this patch series.

In earlier version, we tried to extract common things in libxl_usb.c and put
pvusb specific thing in libxl_pvusb.c, prefixed with pvusb_xxx. As you
suggested, we can leave that when EMU USB patch series added.

Now, about how to handle PV/EMU type in this patch series, I can think of 3 
ways:

1. We define the enumeration (contains PV/AUTO only, user interface only allows
'pv' or 'not specified', so we handle everything in 'pv' way without further 
check. Leave check and other adjusting things when EMU USB patch series added.

2. We check domain type and set proper type if not specified (i.e. 'pv' for PV 
guest, 'emu' for HVM guest). In add/remove function, check if type='emu', 
report  
'not supported' directly; otherwise, continue do following things. When EMU USB
patch serires added, need to extract common things and adjust the check place.

3. Same as 2, but extract common things, only in PV/EMU USB specific part, check
type, if type='emu', report 'not supported'; otherwise, do pvusb work. When 
adding EMU USB patch series, only need to add EMU USB specific things in the
type='emu' branch.

Which one is expected? Or none?

- Chunyan

>  
> >  
> >> +]) 
> >> + 
> >> +libxl_usbdev_type = Enumeration("usbdev_type", [ 
> >> +(0, "invalid"), 
> >> +(1, "hostdev"), 
> >> +]) 
> >> + 
> >> +libxl_device_usbctrl = Struct("device_usbctrl", [ 
> >> +("type", libxl_usbctrl_type), 
> >> +("devid", libxl_devid), 
> >> +("version", integer), 
> >> +("ports", integer), 
> >> +("backend_domid", libxl_domid), 
> >> +("backend_domname", string), 
> >> +   ]) 
> >> + 
> >> +libxl_device_usb =

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling



> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> Sent: Thursday, September 17, 2015 1:18 AM
> To: Wu, Feng
> Cc: xen-devel@lists.xen.org; Tian, Kevin; Keir Fraser; George Dunlap; Andrew
> Cooper; Jan Beulich
> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
> logic
> handling
> 
> On Fri, 2015-09-11 at 16:29 +0800, Feng Wu wrote:
> > This patch includes the following aspects:
> > - Handling logic when vCPU is blocked:
> > * Add a global vector to wake up the blocked vCPU
> >   when an interrupt is being posted to it (This part
> >   was sugguested by Yang Zhang ).
> > * Define two per-cpu variables:
> >   1. pi_blocked_vcpu:
> > A list storing the vCPUs which were blocked
> > on this pCPU.
> >
> >   2. pi_blocked_vcpu_lock:
> > The spinlock to protect pi_blocked_vcpu.
> >
> > - Add some scheduler hooks, this part was suggested
> >   by Dario Faggioli .
> > * vmx_pre_ctx_switch_pi()
> >   It is called before context switch, we update the
> >   posted interrupt descriptor when the vCPU is preempted,
> >   go to sleep, or is blocked.
> >
> > * vmx_post_ctx_switch_pi()
> >   It is called after context switch, we update the posted
> >   interrupt descriptor when the vCPU is going to run.
> >
> > * arch_vcpu_wake_prepare()
> >   It will be called when waking up the vCPU, we update
> >   the posted interrupt descriptor when the vCPU is
> >   unblocked.
> >
> > CC: Keir Fraser 
> > CC: Jan Beulich 
> > CC: Andrew Cooper 
> > CC: Kevin Tian 
> > CC: George Dunlap 
> > CC: Dario Faggioli 
> > Sugguested-by: Dario Faggioli 
> > Signed-off-by: Feng Wu 
> > Reviewed-by: Dario Faggioli 
> > ---
> > v7:
> > - Merge [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted
> interrupts
> >   and "[PATCH v6 14/18] vmx: posted-interrupt handling when vCPU is
> blocked"
> >   into this patch, so it is self-contained and more convenient
> >   for code review.
> > - Make 'pi_blocked_vcpu' and 'pi_blocked_vcpu_lock' static
> > - Coding style
> > - Use per_cpu() instead of this_cpu() in pi_wakeup_interrupt()
> > - Move ack_APIC_irq() to the beginning of pi_wakeup_interrupt()
> > - Rename 'pi_ctxt_switch_from' to 'ctxt_switch_prepare'
> > - Rename 'pi_ctxt_switch_to' to 'ctxt_switch_cancel'
> > - Use 'has_hvm_container_vcpu' instead of 'is_hvm_vcpu'
> > - Use 'spin_lock' and 'spin_unlock' when the interrupt has been
> >   already disabled.
> > - Rename arch_vcpu_wake_prepare to vmx_vcpu_wake_prepare
> > - Define vmx_vcpu_wake_prepare in xen/arch/x86/hvm/hvm.c
> > - Call .pi_ctxt_switch_to() __context_switch() instead of directly
> >   calling vmx_post_ctx_switch_pi() in vmx_ctxt_switch_to()
> > - Make .pi_block_cpu unsigned int
> > - Use list_del() instead of list_del_init()
> > - Coding style
> >
> > One remaining item:
> > Jan has concern about calling vcpu_unblock() in vmx_pre_ctx_switch_pi(),
> > need Dario or George's input about this.
> >
> Hi,
> 
> Sorry for the delay in replying, I was on PTO for a few time.

That's fine. Thanks for your reply!

> 
> Coming to the issue, well, it's a though call.
> 
> First of all, Feng, have you tested this with a debug build of Xen? I'm
> asking because it looks to me that you're ending up calling vcpu_wake()
> with IRQ disabled which, if my brain is not too rusty after a few weeks
> of vacation, should result in check_lock() (in xen/common/spinlock.c)
> complaining, doesn't it?
> 
> In fact, in principle this is not too much different from what happens
> in other places. More specifically, what we have is a vcpu being
> re-inserted in  a runqueue, and the need for re-running the scheduler on
> a(some) PCPU(s) is evaluated. That is similar to what happens in Credit2
> (and in RTDS) in csched2_context_saved(), which is called from within
> context_saved(), still from the context switch code (if
> __CSFLAG_delayed_runq_add is true).
> 
> So it's not the thing per se that is that terrible, IMO. The differences
> between that and your case are:
>  - in the Credit2 case, it happens later down in the context switch
>path (which would look already better to me) and, more important,
>with IRQs already re-enabled;
>  - in the Credit2 case, the effect that something like that can have on
>the scheduler is much more evident, as it happens inside a scheduler
>hook, rather than buried down in arch specific code, which makes me a
>lot less concerned about the possibility of latent issues Jan was
>hinting at, with which I concur.
> 
> So, I guess, first of all, can you confirm whether or not it's exploding
> in debug builds?

Does the

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

On 17/09/15 00:33, Andy Lutomirski wrote:
> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.
>
> This is IMO awful: it papers over bugs.  In particular, KVM gueests
> might be unwittingly depending on this behavior because
> CONFIG_KVM_GUEST currently depends on CONFIG_PARAVIRT.  I'm not
> aware of any such problems, but applying this series would be a good
> way to shake them out.
>
> Fix it so that the MSR operations work the same on CONFIG_PARAVIRT=n
> and CONFIG_PARAVIRT=y as long as Xen isn't being used.  The Xen
> maintainers are welcome to make a similar change on top of this.

The Xen side of things need some further modification before this would
be a safe operation to perform.

On the wrmsr side of things alone, this is the list of things Xen
currently objects to and injects #GP faults for.

(XEN) traps.c:2692:d0v0 Domain attempted WRMSR c081 from
0xe023e008 to 0x00230010.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR c082 from
0x82d0b000 to 0x81560060.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR c083 from
0x82d0b020 to 0x81558100.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR 0174 from
0xe008 to 0x0010.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR 0175 from
0x8300ac0f7fc0 to 0x.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR 0176 from
0x82d08023fd50 to 0x815616d0.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR c083 from
0x82d0b020 to 0x81561910.
(XEN) traps.c:2692:d0v0 Domain attempted WRMSR c084 from
0x00074700 to 0x00047700.

However, it would be certainly be worth teaching PVops not to play with
MSRs it doesn't own.

~Andrew

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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling



> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> Sent: Thursday, September 17, 2015 4:48 PM
> To: Wu, Feng
> Cc: xen-devel@lists.xen.org; Tian, Kevin; Keir Fraser; George Dunlap; Andrew
> Cooper; Jan Beulich
> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
> logic
> handling
> 
> On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:
> 
> > > -Original Message-
> > > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> 
> > > So, I guess, first of all, can you confirm whether or not it's exploding
> > > in debug builds?
> >
> > Does the following information in Config.mk mean it is a debug build?
> >
> > # A debug build of Xen and tools?
> > debug ?= y
> > debug_symbols ?= $(debug)
> >
> I think so. But as I said in my other email, I was wrong, and this is
> probably not an issue.
> 
> > > And in either case (just tossing out ideas) would it be
> > > possible to deal with the "interrupt already raised when blocking" case:
> >
> > Thanks for the suggestions below!
> >
> :-)
> 
> > >  - later in the context switching function ?
> > In this case, we might need to set a flag in vmx_pre_ctx_switch_pi() instead
> > of calling vcpu_unblock() directly, then when it returns to 
> > context_switch(),
> > we can check the flag and don't really block the vCPU.
> >
> Yeah, and that would still be rather hard to understand and maintain,
> IMO.
> 
> > But I don't have a clear
> > picture about how to archive this, here are some questions from me:
> > - When we are in context_switch(), we have done the following changes to
> > vcpu's state:
> > * sd->curr is set to next
> > * vCPU's running state (both prev and next ) is changed by
> >   vcpu_runstate_change()
> > * next->is_running is set to 1
> > * periodic timer for prev is stopped
> > * periodic timer for next is setup
> > ..
> >
> > So what point should we perform the action to _unblock_ the vCPU? We
> > Need to roll back the formal changes to the vCPU's state, right?
> >
> Mmm... not really. Not blocking prev does not mean that prev would be
> kept running on the pCPU, and that's true for your current solution as
> well! As you say yourself, you're already in the process of switching
> between prev and next, at a point where it's already a thing that next
> will be the vCPU that will run. Not blocking means that prev is
> reinserted to the runqueue, and a new invocation to the scheduler is
> (potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
> __runq_tickle()), but it's only when such new scheduling happens that
> prev will (potentially) be selected to run again.
> 
> So, no, unless I'm fully missing your point, there wouldn't be no
> rollback required. However, I still would like the other solution (doing
> stuff in vcpu_block()) better (see below).

Thanks for the detailed clarification. Yes, maybe my description above
is a little vague. Yes, the non-blocking vCPU should be put into the
runqueue. I shouldn't use the term "roll back". :)

> 
> > >  - with another hook, perhaps in vcpu_block() ?
> >
> > We could check this in vcpu_block(), however, the logic here is that before
> > vCPU is blocked, we need to change the posted-interrupt descriptor,
> > and during changing it, if 'ON' bit is set, which means VT-d hardware
> > issues a notification event because interrupts from the assigned devices
> > is coming, we don't need to block the vCPU and hence no need to update
> > the PI descriptor in this case.
> >
> Yep, I saw that. But could it be possible to do *everything* related to
> blocking, including the update of the descriptor, in vcpu_block(), if no
> interrupt have been raised yet at that time? I mean, would you, if
> updating the descriptor in there, still get the event that allows you to
> call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
> the blocking, no matter whether that resulted in an actual context
> switch already or not?
> 
> I appreciate that this narrows the window for such an event to happen by
> quite a bit, making the logic itself a little less useful (it makes
> things more similar to "regular" blocking vs. event delivery, though,
> AFAICT), but if it's correct, ad if it allows us to save the ugly
> invocation of vcpu_unblock from context switch context, I'd give it a
> try.
> 
> After all, this PI thing requires actions to be taken when a vCPU is
> scheduled or descheduled because of blocking, unblocking and
> preemptions, and it would seem natural to me to:
>  - deal with blockings in vcpu_block()
>  - deal with unblockings in vcpu_wake()
>  - deal with preemptions in context_switch()
> 
> This does not mean being able to consolidate some of the cases (like
> blockings and preemptions, in the current version of the code) were not
> a nice thing... But we don't want it at all costs . :-)

Yes, doing this in vcpu_block() is indeed an alternative, In fact, I also 
thought
about it before. I need to

Re: [Xen-devel] [PATCH v1 5/5] xsplice: Use ld-embedded build-ids

On 17/09/15 07:41, Martin Pohlack wrote:
> On 17.09.2015 00:31, Andrew Cooper wrote:
>> On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote:
>>> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper 
>>>  wrote:
 On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
> From: Martin Pohlack 
>
> The mechanism to get this is via the XSPLICE_OP and
> we add a new subsequent hypercall to retrieve the
> binary build-id. The hypercall allows an arbirarty
> size (the buffer is provided to the hypervisor) - however
> by default the toolstack will allocate it up to 128
> bytes.
>
> We also add two places for the build-id to be printed:
>  - xsplice keyhandler. We cannot use 'hh' in the hypervisor
>snprintf handler (as it is not implemented) so instead
>we use an simpler way to print it.
>  - In the 'xen-xsplice' tool add an extra parameter - build-id
>to print this as an human readable value.
>
> Note that one can also retrieve the value by 'readelf -h xen-syms'.
>
> Signed-off-by: Martin Pohlack 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  tools/libxc/include/xenctrl.h |  1 +
>  tools/libxc/xc_misc.c | 26 +
>  tools/misc/xen-xsplice.c  | 39 
>  xen/arch/x86/Makefile |  4 +-
>  xen/arch/x86/xen.lds.S|  5 +++
>  xen/common/xsplice.c  | 86
 +++
>  xen/include/public/sysctl.h   | 18 +
>  xen/include/xen/version.h |  1 +
>  8 files changed, 178 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h
 b/tools/libxc/include/xenctrl.h
> index 2cd982d..946ddc0 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2860,6 +2860,7 @@ int xc_xsplice_apply(xc_interface *xch, char
 *id);
>  int xc_xsplice_revert(xc_interface *xch, char *id);
>  int xc_xsplice_unload(xc_interface *xch, char *id);
>  int xc_xsplice_check(xc_interface *xch, char *id);
> +int xc_xsplice_build_id(xc_interface *xch, char *build_id, unsigned
 int max);

 The build id of the current running hypervisor should belong in the
 xeninfo hypercall.  It is not specific to xsplice.
>>> However in the previous reviews it was pointed out that it should only be 
>>> accessible to dom0.
>>>
>>> Or to any domains as long as the XSM allows (and is turned on) - so not the 
>>> default dummy one.
>>>
>>> That is a bit of 'if' extra complexity which I am not sure is worth it?
>> DomU can already read the build information such as changeset, compile
>> time, etc.  Build-id is no more special or revealing.
> I would take this as an argument *against* giving DomU access to those
> pieces of information in details and not as an argument for
> *additionally* giving it access to build-id.
>
> With build-id we have the chance to shape a not-yet-established API and
> I would like to follow the Principle of least privilege wherever it
> makes sense.
>
> To reach a similar security level with the existing API, it might make
> sense to limit DomU access to compile date, compile time, compiled by,
> compiled domain, compiler version and command line details, xen extra
> version, and xen changeset.  Basically anything that might help DomUs to
> uniquely identify a Xen build.
>
> The approach can not directly drop access to those fields, as that would
> break an existing API, but it could restrict the detail level handed out
> to DomU.

These are all valid arguments to be made, but please lets fix the issue
properly rather than hacking build-id on the side of an unrelated component.

>From my point of view, the correct course of action is this:

* Split the current XSM model to contain separate attributes for general
and privileged information.
** For current compatibility, all existing XENVER_* subops fall into general
* Apply an XSM check at the very start of the hypercall.
* Extend do_xen_version() to take 3 parameters.  (It is curious that it
didn't take a length parameter before)
** This is still ABI compatible, as existing subops simply ignore the
parameter.
* Introduce new XENVER_build_id subop which is documented to require the
3-parameter version of the hypercall.
** This subop falls into straight into privileged information.

This will introduce build-id in its correct location, with appropriate
restrictions.

Moving forwards, we really should have an audit of the existing XENVER_*
subops.  Some are clearly safe/required for domU to read, but some such
as XENVER_commandline have no business being accessible.  A separate
argument, from the repeatable build point of view, says that compilation
information isn't useful at all.

Depending on how we wish to fix the issue, we could either do a blanket
move of the subops

[Xen-devel] [PATCH v4 3/4] tools: add tools support for Intel CDP

This is the xl/xc changes to support Intel Code/Data Prioritization.
CAT xl commands to set/get CBMs are extended to support CDP.

Signed-off-by: He Chen 
---
 tools/libxc/include/xenctrl.h |  7 +--
 tools/libxc/xc_psr.c  | 17 ++-
 tools/libxl/libxl.h   |  7 +++
 tools/libxl/libxl_psr.c   |  5 -
 tools/libxl/libxl_types.idl   |  3 +++
 tools/libxl/xl_cmdimpl.c  | 49 ++-
 tools/libxl/xl_cmdtable.c |  3 +++
 7 files changed, 77 insertions(+), 14 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index de3c0ad..54f2069 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2798,7 +2798,9 @@ enum xc_psr_cmt_type {
 typedef enum xc_psr_cmt_type xc_psr_cmt_type;
 
 enum xc_psr_cat_type {
-XC_PSR_CAT_L3_CBM = 1,
+XC_PSR_CAT_L3_CBM  = 1,
+XC_PSR_CAT_L3_CODE = 2,
+XC_PSR_CAT_L3_DATA = 3,
 };
 typedef enum xc_psr_cat_type xc_psr_cat_type;
 
@@ -2824,7 +2826,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
uint32_t domid,
xc_psr_cat_type type, uint32_t target,
uint64_t *data);
 int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
-   uint32_t *cos_max, uint32_t *cbm_len);
+   uint32_t *cos_max, uint32_t *cbm_len,
+   bool *cdp_enabled);
 #endif
 
 #endif /* XENCTRL_H */
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index d8b3a51..5bbe950 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -260,6 +260,12 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t 
domid,
 case XC_PSR_CAT_L3_CBM:
 cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
 break;
+case XC_PSR_CAT_L3_CODE:
+cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
+break;
+case XC_PSR_CAT_L3_DATA:
+cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
+break;
 default:
 errno = EINVAL;
 return -1;
@@ -287,6 +293,12 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t 
domid,
 case XC_PSR_CAT_L3_CBM:
 cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
 break;
+case XC_PSR_CAT_L3_CODE:
+cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
+break;
+case XC_PSR_CAT_L3_DATA:
+cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
+break;
 default:
 errno = EINVAL;
 return -1;
@@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t 
domid,
 }
 
 int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
-   uint32_t *cos_max, uint32_t *cbm_len)
+   uint32_t *cos_max, uint32_t *cbm_len,
+   bool *cdp_enabled)
 {
 int rc;
 DECLARE_SYSCTL;
@@ -320,6 +333,8 @@ int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t 
socket,
 {
 *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
 *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
+*cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
+   XEN_SYSCTL_PSR_CAT_L3_CDP;
 }
 
 return rc;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..611e98d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
libxl_mac *src);
  * If this is defined, the Cache Allocation Technology feature is supported.
  */
 #define LIBXL_HAVE_PSR_CAT 1
+
+/*
+ * LIBXL_HAVE_PSR_CDP
+ *
+ * If this is defined, the Code and Data Prioritization feature is supported.
+ */
+#define LIBXL_HAVE_PSR_CDP 1
 #endif
 
 /*
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3378239..62963cf 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -87,6 +87,9 @@ static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
 case EEXIST:
 msg = "The same CBM is already set to this domain";
 break;
+case EINVAL:
+msg = "Unable to set code or data CBM when CDP is disabled";
+break;
 
 default:
 libxl__psr_log_err_msg(gc, err);
@@ -352,7 +355,7 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, 
libxl_psr_cat_info **info,
 
 for (i = 0; i < nr_sockets; i++) {
 if (xc_psr_cat_get_l3_info(ctx->xch, i, [i].cos_max,
-[i].cbm_len)) {
+   [i].cbm_len, [i].cdp_enabled)) {
 libxl__psr_cat_log_err_msg(gc, errno);
 rc = ERROR_FAIL;
 free(ptr);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef346e7..fa017ad 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -787,9 +787,12 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
 libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
 (0,

[Xen-devel] [PATCH v4 1/4] x86: Support enable CDP by boot parameter and add get CDP status

Add boot parameter `psr=cdp` to enable CDP at boot time.
Intel Code/Data Prioritization(CDP) feature is based on CAT. Note that
cos_max would be half when CDP is on. struct psr_cat_cbm is extended to
support CDP operation. Extend psr_get_cat_l3_info sysctl to get CDP
status.

Signed-off-by: He Chen 
---
 docs/misc/xen-command-line.markdown | 11 -
 xen/arch/x86/psr.c  | 84 ++---
 xen/arch/x86/sysctl.c   |  5 ++-
 xen/include/asm-x86/msr-index.h |  3 ++
 xen/include/asm-x86/psr.h   | 11 -
 xen/include/public/sysctl.h |  4 +-
 6 files changed, 98 insertions(+), 20 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index a2e427c..d92e323 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1165,9 +1165,9 @@ This option can be specified more than once (up to 8 
times at present).
 > `= `
 
 ### psr (Intel)
-> `= List of ( cmt: | rmid_max: | cat: | 
cos_max: )`
+> `= List of ( cmt: | rmid_max: | cat: | 
cos_max: | cdp: )`
 
-> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255`
+> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255,cdp:0`
 
 Platform Shared Resource(PSR) Services.  Intel Haswell and later server
 platforms offer information about the sharing of resources.
@@ -1197,6 +1197,13 @@ The following resources are available:
   the cache allocation.
   * `cat` instructs Xen to enable/disable Cache Allocation Technology.
   * `cos_max` indicates the max value for COS ID.
+* Code and Data Prioritization Technology (Broadwell and later). Information
+  regarding the code cache and the data cache allocation. CDP is based on CAT.
+  * `cdp` instructs Xen to enable/disable Code and Data Prioritization. Note
+that `cos_max` of CDP is a little different from `cos_max` of CAT. With
+CDP, one COS will corespond two CBMs other than one with CAT, due to the
+sum of CBMs is fixed, that means actual `cos_max` in use will automatically
+reduce to half when CDP is enabled.
 
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | 
 > [c]old]`
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c0daa2e..e44ed8b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -21,9 +21,16 @@
 
 #define PSR_CMT(1<<0)
 #define PSR_CAT(1<<1)
+#define PSR_CDP(1<<2)
 
 struct psr_cat_cbm {
-uint64_t cbm;
+union {
+uint64_t cbm;
+struct {
+uint64_t code;
+uint64_t data;
+};
+} u;
 unsigned int ref;
 };
 
@@ -43,6 +50,7 @@ struct psr_cmt *__read_mostly psr_cmt;
 
 static unsigned long *__read_mostly cat_socket_enable;
 static struct psr_cat_socket_info *__read_mostly cat_socket_info;
+static unsigned long *__read_mostly cdp_socket_enable;
 
 static unsigned int __initdata opt_psr;
 static unsigned int __initdata opt_rmid_max = 255;
@@ -94,6 +102,7 @@ static void __init parse_psr_param(char *s)
 
 parse_psr_bool(s, val_str, "cmt", PSR_CMT);
 parse_psr_bool(s, val_str, "cat", PSR_CAT);
+parse_psr_bool(s, val_str, "cdp", PSR_CDP);
 
 if ( val_str && !strcmp(s, "rmid_max") )
 opt_rmid_max = simple_strtoul(val_str, NULL, 0);
@@ -261,8 +270,14 @@ static struct psr_cat_socket_info 
*get_cat_socket_info(unsigned int socket)
 return cat_socket_info + socket;
 }
 
+static inline bool_t cdp_is_enabled(unsigned int socket,
+unsigned long *cdp_socket_enable)
+{
+return (cdp_socket_enable && test_bit(socket, cdp_socket_enable));
+}
+
 int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
-uint32_t *cos_max)
+uint32_t *cos_max, uint32_t *flags)
 {
 struct psr_cat_socket_info *info = get_cat_socket_info(socket);
 
@@ -272,6 +287,10 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t 
*cbm_len,
 *cbm_len = info->cbm_len;
 *cos_max = info->cos_max;
 
+*flags = 0;
+if ( cdp_is_enabled(socket, cdp_socket_enable) )
+*flags |= PSR_CAT_FLAG_L3_CDP;
+
 return 0;
 }
 
@@ -282,7 +301,7 @@ int psr_get_l3_cbm(struct domain *d, unsigned int socket, 
uint64_t *cbm)
 if ( IS_ERR(info) )
 return PTR_ERR(info);
 
-*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].cbm;
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
 
 return 0;
 }
@@ -313,19 +332,34 @@ static bool_t psr_check_cbm(unsigned int cbm_len, 
uint64_t cbm)
 struct cos_cbm_info
 {
 unsigned int cos;
-uint64_t cbm;
+uint64_t cbm_code;
+uint64_t cbm_data;
+bool_t cdp;
 };
 
 static void do_write_l3_cbm(void *data)
 {
 struct cos_cbm_info *info = data;
 
-wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm);
+if ( info->cdp )
+{
+wrmsrl(MSR_IA32_PSR_L3_MASK_CODE(info->cos), info->cbm_code);
+

[Xen-devel] [PATCH v4 0/4] detect and initialize CDP (Code/Data Prioritization) feature

Changes in v4:
- x86:
  * remove union member name in struct `psr_cat_cbm` (suggested by Jan)
  * fix log info of CAT & CDP (suggested by Chao & Jan)
  * add a helper `cdp_is_enabled` to tell the status of CDP and CDP initialize
failed is considered (Jan's comment)
  * XEN_SYSCTL_INTERFACE_VERSION 0x000C -> 0x000D (suggested by Jan)
  * refine CBM type check logic in get/set CBM function (suggested by Jan)
  * loop optimization in function `find_cos` (suggested by Jan)
- tools: Address Chao's comments.
- docs: Address Chao's comments.
- code style

Changes in v3:
- x86: remove redundant CDP field in cat_socket_enable (suggested by Chao)
- tools: simplify CBM setting function in tools (suggested by Jan)
- docs: Add boot parameter description (suggested by Chao & Ian)
- code style

Changes in v2:
- x86: Enable CDP by boot parameter instead of enabling/disabling CDP at
   runtime (suggested by Andrew)
- tools: remove psr-cat-cdp-enable/disable xl commands
- code style

Code/Data Prioritization(CDP) is offered in Intel Broadwell and later server
platforms, which is an extension of CAT. CDP enables isolation and separate
prioritization of code and data fetches to the L3 cache in a software
configurable manner, which can enable workload prioritization and tuning of
cache capacity to the characteristics of the workload. CDP extends Cache
Allocation Technology (CAT) by providing separate code and data capacity bit
masks(CBM) per Class of Service (COS). CDP is used on VM basis in the Xen
implementation.

More information about CDP, please refer to Intel SDM, Volumn 3, section 17.16
http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf

This patch series enables CDP feature in Xen based on CAT code, and extends
CBM operation functions to support CDP. For all the changes, please see in
each patch.

This v4 patchset has been tested on Intel Broadwell server platform.

To make this patchset better, any comment or suggestion is welcomed, I would
really appreciate it.

Thanks

He Chen (4):
  x86: Support enable CDP by boot parameter and add get CDP status
  x86: add domctl cmd to set/get CDP code/data CBM
  tools: add tools support for Intel CDP
  docs: add document to introduce CDP command

 docs/man/xl.pod.1   |  14 +++
 docs/misc/xen-command-line.markdown |  11 +-
 docs/misc/xl-psr.markdown   |  44 ++-
 tools/libxc/include/xenctrl.h   |   7 +-
 tools/libxc/xc_psr.c|  17 ++-
 tools/libxl/libxl.h |   7 ++
 tools/libxl/libxl_psr.c |   5 +-
 tools/libxl/libxl_types.idl |   3 +
 tools/libxl/xl_cmdimpl.c|  49 ++--
 tools/libxl/xl_cmdtable.c   |   3 +
 xen/arch/x86/domctl.c   |  32 -
 xen/arch/x86/psr.c  | 239 +---
 xen/arch/x86/sysctl.c   |   5 +-
 xen/include/asm-x86/msr-index.h |   3 +
 xen/include/asm-x86/psr.h   |  23 +++-
 xen/include/public/domctl.h |   4 +
 xen/include/public/sysctl.h |   4 +-
 17 files changed, 395 insertions(+), 75 deletions(-)

-- 
1.9.1


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

[Xen-devel] [PATCH v4 4/4] docs: add document to introduce CDP command

Add new CDP options with CAT commands in xl interface man page.
Add description of CDP in xl-psr.markdown.

Signed-off-by: He Chen 
---
 docs/man/xl.pod.1 | 14 ++
 docs/misc/xl-psr.markdown | 44 +++-
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f22c3f3..3c7107d 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1530,6 +1530,12 @@ applications. In the Xen implementation, CAT is used to 
control cache allocation
 on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
 (CBM) for the domain.
 
+Intel Broadwell and later server platforms also offer Code/Data Prioritization
+(CDP) for cache allocation, which support specifying code or data cache for
+applications. CDP is used on VM basis in the Xen implementation. To specify
+code or data CBM for the domain, CDP feature must be enabled and CBM type
+options need to be specified when setting CBM.
+
 =over 4
 
 =item B [I] I I
@@ -1545,6 +1551,14 @@ B
 
 Specify the socket to process, otherwise all sockets are processed.
 
+=item B<-c>, B<--code>
+
+Set code CBM when CDP is enabled.
+
+=item B<-d>, B<--data>
+
+Set data CBM when CDP is enabled.
+
 =back
 
 =item B [I]
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 3545912..5eb97cc 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according to the 
RMID and reports
 monitored data via a counter register.
 
 For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"17.15 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
 
 In Xen's implementation, each domain in the system can be assigned a RMID
 independently, while RMID=0 is reserved for monitoring domains that don't
@@ -91,17 +91,42 @@ For example, assuming a system with 8 portions and 3 
domains:
first domain exclusive access to half the cache, and the other two exclusive
access to one quarter each.
 
-For more detailed information please refer to Intel SDM chapter
-"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
-
 In Xen's implementation, CBM can be configured with libxl/xl interfaces but
 COS is maintained in hypervisor only. The cache partition granularity is per
 domain, each domain has COS=0 assigned by default, the corresponding CBM is
 all-ones, which means all the cache resource can be used by default.
 
+Code/Data Prioritization (CDP) Technology is an extension of CAT, which is
+available on Intel Broadwell and later server platforms. CDP enables isolation
+and separate prioritization of code and data fetches to the L3 cache in a
+software configurable manner, which can enable workload prioritization and
+tuning of cache capacity to the characteristics of the workload. CDP extends
+Cache Allocation Technology (CAT) by providing separate code and data masks
+per Class of Service (COS).
+
+CDP can be enabled by adding `psr=cdp` to Xen bootline.
+
+When CDP is enabled,
+
+ * the CAT masks are re-mapped into interleaved pairs of masks for data or code
+   fetches.
+
+ * the range of COS for CAT is re-indexed, with the lower-half of the COS
+   range available for CDP.
+
+CDP allows OS or Hypervisor to partition cache allocation more fine-grained,
+code cache and data cache can be specified respectively. With CDP enabled,
+one COS corresponds to two CBMs (code CBM & data CBM), since the sum of CBMs
+is fixed, that means the number of available COSes will reduce to half when
+CDP is on.
+
+For more detailed information please refer to Intel SDM chapter
+"17.16 - Platform Shared Resource Control: Cache Allocation Technology".
+
 ### xl interfaces
 
-System CAT information such as maximum COS and CBM length can be obtained by:
+System CAT information such as maximum COS, CBM length and CDP status can be
+obtained by:
 
 `xl psr-hwinfo --cat`
 
@@ -116,6 +141,15 @@ A cbm is valid only when:
obtained with `xl psr-hwinfo --cat`.
  * All the set bits are contiguous.
 
+When CDP is enabled, `-c` or `--code` option is available to set code CBM for
+the domain.
+
+When CDP is enabled, `-d` or `--data` option is available to set data CBM for
+the domain.
+
+If neither `-c` nor `-d` option is specified when CDP is on, the same code CBM
+and data CBM will be set for the domain.
+
 In a multi-socket system, the same cbm will be set on each socket by default.
 Per socket cbm can be specified with the `--socket SOCKET` option.
 
-- 
1.9.1


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

[Xen-devel] [PATCH v4 2/4] x86: add domctl cmd to set/get CDP code/data CBM

CDP extends CAT and provides the capacity to control L3 code & data
cache. With CDP, one COS corresponds to two CMBs(code & data). cbm_type
is added to distinguish different CBM operations. Besides, new domctl
cmds are introdunced to support set/get CDP CBM. Some CAT functions to
operation CBMs are extended to support CDP.

Signed-off-by: He Chen 
---
 xen/arch/x86/domctl.c   |  32 -
 xen/arch/x86/psr.c  | 165 ++--
 xen/include/asm-x86/psr.h   |  12 +++-
 xen/include/public/domctl.h |   4 ++
 4 files changed, 172 insertions(+), 41 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index bf62a88..734fddb 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1167,12 +1167,40 @@ long arch_do_domctl(
 {
 case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
 ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
- domctl->u.psr_cat_op.data);
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3);
+break;
+
+case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
+ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_CODE);
+break;
+
+case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
+ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_DATA);
 break;
 
 case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
 ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
- >u.psr_cat_op.data);
+ >u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3);
+copyback = 1;
+break;
+
+case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
+ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+ >u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_CODE);
+copyback = 1;
+break;
+
+case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
+ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+ >u.psr_cat_op.data,
+ PSR_CBM_TYPE_L3_DATA);
 copyback = 1;
 break;
 
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index e44ed8b..c5519b7 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -294,14 +294,40 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t 
*cbm_len,
 return 0;
 }
 
-int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
+int psr_get_l3_cbm(struct domain *d, unsigned int socket,
+   uint64_t *cbm, enum cbm_type type)
 {
 struct psr_cat_socket_info *info = get_cat_socket_info(socket);
+bool_t cdp_enabled = cdp_is_enabled(socket, cdp_socket_enable);
 
 if ( IS_ERR(info) )
 return PTR_ERR(info);
 
-*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+switch ( type )
+{
+case PSR_CBM_TYPE_L3:
+if ( type == PSR_CBM_TYPE_L3 && cdp_enabled )
+return -EXDEV;
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+break;
+
+case PSR_CBM_TYPE_L3_CODE:
+if ( !cdp_enabled )
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+else
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.code;
+break;
+
+case PSR_CBM_TYPE_L3_DATA:
+if ( !cdp_enabled )
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
+else
+*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.data;
+break;
+
+default:
+ASSERT_UNREACHABLE();
+}
 
 return 0;
 }
@@ -375,10 +401,48 @@ static int write_l3_cbm(unsigned int socket, unsigned int 
cos,
 return 0;
 }
 
-int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
+static int find_cos(struct psr_cat_cbm *map, int cos_max,
+uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
 {
-unsigned int old_cos, cos;
-struct psr_cat_cbm *map, *found = NULL;
+unsigned int cos;
+
+for ( cos = 0; cos <= cos_max; cos++ )
+{
+if( map[cos].ref &&
+((!cdp_enabled && map[cos].u.cbm == cbm_code) ||
+(cdp_enabled && map[cos].u.code == cbm_code &&
+map[cos].u.data == cbm_data)))
+return cos;
+}
+
+return -ENOENT;
+}
+
+static int pick_avail_cos(struct psr_cat_cbm *map, unsigned int cos_max,
+  unsigned int old_cos)
+{
+unsigned int cos;
+
+/* If old cos is referred only by the domain, then use it. */
+if ( map[old_cos].ref == 1 )

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

On 09/17/2015 09:19 AM, Chun Yan Liu wrote:
> 
> 
 On 9/9/2015 at 12:52 AM, in message <55ef1244@citrix.com>, George 
 Dunlap
>  wrote: 
>> On 09/08/2015 03:17 PM, Ian Campbell wrote: 
>>> On Mon, 2015-08-10 at 18:35 +0800, Chunyan Liu wrote: 
>>>  
>>> Sorry for the delay, between 4.6 freeze crunch, conference and vacation 
>>> I've been a bit swamped. 
>>>  
>>> I'm just going to comment on the APIs (mainly public libxl.h and .idl) in 
>>> this pass. 
>>>  
 diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
 index 5f9047c..05b6331 100644 
 --- a/tools/libxl/libxl.h 
 +++ b/tools/libxl/libxl.h 
 @@ -123,6 +123,23 @@ 
  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 
   
  /* 
 + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of 
>>>  
>>> And cold-plug, no? 
>>  
>> So you should probably say something like "indicates functions for 
>> plugging in USB devices through pvusb -- both hotplug and at domain 
>> creation time." 
>>  
 +libxl_usbctrl_type = Enumeration("usbctrl_type", [ 
 +(0, "AUTO"), 
>>>  
>>> What are the proposed semantics of using LIBXL_USBCTRL_TYPE_AUTO? 
>>  
>> Generally "DTRT".  Meaning: 
>> 1. If your domain has no devicemodel, use PV. 
>> 2. If your device has a devicemodel, and no PV drivers have peen 
>> detected, use the devicemodel. 
>> 3. If your device has a devicemodel, but PV drivers have been detected, 
>> use PV. 
>>  
>> At the moment we don't have a way to check for PV drivers, so this just 
>> collapses down to "PV for domains without a DM and DM for domains with a 
>> DM." 
>>  
>>>  
 +(1, "PV"), 
 +(2, "QEMU"), 
>>>  
>>> Is "QEMU" what we want here, as opposed to, say, "EMU" (similar to NICs)? 
>>  
>> I had this as "DEVICEMODEL", since what we mean is that we want the 
>> device model to provide access (and in theory in the future we may use a 
>> different device model).  But "EMU" works for me too. 
>>  
>>> I think we probably don't want to go as fine grained as "XHCI" and "EHCI" 
>>> etc, do we? I see we have a version field below, is it intended that there 
>>> be some way to select between e.g. UHCI and OHCI (which IIRC are different 
>>> USB 1.0 controllers). 
>>>  
>>> Maybe these questions should all be left aside for when QMEU support is 
>>> actually added (AFAICT this field is just a placeholder)? In fact I glanced 
>>> at the code and was surprised to find nothing checking for 
>>> LIBXL_USBCTRL_TYPE at all, did I miss something? 
>>>  
>>> I think the two choices are: 
>>>  
>>> We can decide quickly and easily what the option(s) other than PV should be 
>>> here and you include it in the IDL, but you would then need to check 
>>> usbctrl->type == PV at various points, not silently treat all options as 
>>> PV. 
>>>  
>>> Or this becomes a long conversation in which case I think your best bet 
>>> would be to leave the enum with just the PV (and maybe AUTO) entries and 
>>> leave the decision on the name for the emulated option to the series which 
>>> implements that. 
>>  
>> I think the idea was to simply offer 1, 2, and 3 as options, and for the 
>> devicemodel version, choose a suitable controller (or set of 
>> controllers) for each option; similar to what usbversion= does now.
> 
>  
> Hi, George,
> 
> I'm still confused about the expected look concerning PV/EMU type handling in
> this patch series.
> 
> In earlier version, we tried to extract common things in libxl_usb.c and put
> pvusb specific thing in libxl_pvusb.c, prefixed with pvusb_xxx. As you
> suggested, we can leave that when EMU USB patch series added.
> 
> Now, about how to handle PV/EMU type in this patch series, I can think of 3 
> ways:
> 
> 1. We define the enumeration (contains PV/AUTO only, user interface only 
> allows
> 'pv' or 'not specified', so we handle everything in 'pv' way without further 
> check. Leave check and other adjusting things when EMU USB patch series added.
> 
> 2. We check domain type and set proper type if not specified (i.e. 'pv' for 
> PV 
> guest, 'emu' for HVM guest). In add/remove function, check if type='emu', 
> report  
> 'not supported' directly; otherwise, continue do following things. When EMU 
> USB
> patch serires added, need to extract common things and adjust the check place.
> 
> 3. Same as 2, but extract common things, only in PV/EMU USB specific part, 
> check
> type, if type='emu', report 'not supported'; otherwise, do pvusb work. When 
> adding EMU USB patch series, only need to add EMU USB specific things in the
> type='emu' branch.
> 
> Which one is expected? Or none?

So there are two questions here, first WRT the code, the second WRT the
interface.

WRT the code, *normally* the first person to submit the code gets to
have it easy, and the second person has to do all the work of
refactoring.  So you would be completely within your rights to simply
submit "libxl_usb.c", and make me refactor that into libxl_pvusb.c

Re: [Xen-devel] [OSSTest Nested v12 00/21] Introduction of netsted HVM test job

Hu, Robert writes ("RE: [OSSTest Nested v12 00/21] Introduction of netsted HVM 
test job"):
> [Ian Jackson:]
> > Sorry for the delay in reviewing the last few patches in this series.
> > I've finished with it now.  I hope my previous review comments
> > etc. have been helpful.
>
> Though just took a quick glance at a few, I believe they are helpful.

OK, good, thanks.

> > Do you intend to resend a v13 soon ?  If you are finding my comments
> > and suggestions difficult to implement, perhaps it would be better for
> > you to give me a git branch for me to work on.
>
> Yes.

Great.

> That would be a great favor, thank you very very much!  Would you
> give me a branch I can push my code to? for I'm afraid that company
> policy doesn't allow me to set up git repo for external access.

How irritating.  We will set you up with an account on xenbits.
(Lars, I trust this is OK.)

Robert, could you please send me your ssh public key ?

> > I would be happy to rework the bits of your series which need the most
> > reorganisation, and then pass it back on to you for testing.  Would
> > that be a good workflow for you ?
>
> Sure, I'd love to.

Great.

Thanks,
Ian.

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

Re: [Xen-devel] Notes from Xen BoF at Debconf15

On Tue, Sep 8, 2015 at 3:49 PM, Doug Goldstein  wrote:
>> Stubdomains
>> ===
>>
>> Hard to do in a packaging environment (is really its own partial
>> architecture). Rump kernels are no different in this regard.
>>
>> No clever ideas were put forward.
>
> Honestly what about moving these more out of tree? Now with mini-os
> being out of tree and the stubdoms needing mini-os its an absolute mess
> to build from a distro standpoint since mini-os is git fetched. To make
> it work upstream using raisin would be a great improvement here.

The real question with stubdomains is about how to build them at all
so that they're available from within a Debian/Linux distribution,
given that what you want is a binary that consists of code from dozens
of Debian packages (e.g., qemu + all its dependencies) re-compiled for
a different environment (minios or rump kernels).  See the "fork" of
the thread we had on this subject.

(And of course the same if you s/Debian/$SOME_OTHER_DISTRO/;)

 -George

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

[Xen-devel] [PATCH] MAINTAINERS: Split out maintainers for the hypervisor

This is a copy of the `THE REST' entry but with my own entry removed,
as I do not normally review hypervisor patches.

I have chosen to be conservative and make a minimal change, rather
than trying to describe a desirable state, or reflect reality.

The obvious questions to me, that I have not answered, are:
 - Does Ian Campbell want to be in this list ?
 - Should some of the arch-specific Xen maintainers be in this list ?
 - Are there others who should be listed ?

I have tried to include in the CC list the active people listed in
MAINTAINERS for any of the principal components under xen/.

Signed-off-by: Ian Jackson 
CC: Ian Campbell 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
CC: Julien Grall 
CC: Suravee Suthikulpanit 
CC: Aravind Gopalakrishnan 
CC: Boris Ostrovsky 
CC: Suravee Suthikulpanit 
CC: Aravind Gopalakrishnan 
CC: Stefano Stabellini 
CC: Juergen Gross 
CC: David Vrabel 
CC: Gang Wei 
CC: Shane Wang 
CC: Yang Zhang 
CC: Kevin Tian 
CC: Jun Nakajima 
CC: Liu Jinsong 
CC: George Dunlap 
CC: Konrad Rzeszutek Wilk 
CC: Razvan Cojocaru 
CC: Tamas K Lengyel 
CC: Daniel De Graaf 
---
 MAINTAINERS |9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a6bece6..45fe426 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -404,6 +404,15 @@ F:  xen/include/xsm/
 F:  xen/xsm/
 F:  docs/misc/xsm-flask.txt
 
+REST OF THE HYPERVISOR
+M: Jan Beulich 
+M: Tim Deegan 
+M: Keir Fraser 
+M: Ian Campbell 
+L: xen-devel@lists.xen.org
+S: Supported
+F: xen/
+
 THE REST
 M: Ian Campbell 
 M: Ian Jackson 
-- 
1.7.10.4


___
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

Julien Grall writes ("Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous 
Device-TLB Flush for ATS Device"):
> On 16/09/2015 14:47, Ian Jackson wrote:
> > I don't consider myself qualified to review that.  I think the
> > MAINTAINERS file should have an entry for xen/ but it doesn't seem to.
> 
> I think a such patch should come from you.

You're probably right.  Just sent.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops



On 17/09/2015 11:31, Borislav Petkov wrote:
> 
>> > Crashing the bootup on an unknown MSR is bad. Many MSR reads and writes 
>> > are 
>> > non-critical and returning the 'safe' result is much better than crashing 
>> > or 
>> > hanging the bootup.
> ... and prepending all MSR accesses with feature/CPUID checks is probably 
> almost
> impossible.

That's not a big deal, that's what *_safe is for.  The problem is that
there are definitely some cases where the *_safe version is not being used.

I agree with Ingo that we should start with a WARN.  For example:

- give the read_msr and write_msr hooks the same prototype as the safe
variants

- make the virt platforms always return "no error" for the unsafe
variants (I understand if your first reaction is "ouch", but this
effectively is already the current behavior)

- change rdmsr/wrmsr/rdmsrl/wrmsrl to WARN if the read_msr and write_msr
hooks return an error

Paolo

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

[Xen-devel] [PATCH] xen/x86: Record xsave features in c->x86_capabilities

Convert existing cpu_has_x??? to being functions of boot_cpu_data (matching
the prevailing style), and mask out unsupported features.

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

This is another patch from my feature levelling series which is being posted
early because of interactions with other code.

Shuai: This patch will interact with your xsaves series, although I hope it
will make your series easier.

Jan: I have opted for adding leaf 8 rather than reusing leaf 2, due to the
uncertainty with how this information is exposed in libxl.  This patch
introduces no change with how the information is represented in userspace.
---
 xen/arch/x86/cpu/common.c|2 +-
 xen/arch/x86/traps.c |5 +
 xen/arch/x86/xstate.c|   31 +++
 xen/include/asm-x86/cpufeature.h |   13 -
 xen/include/asm-x86/xstate.h |8 ++--
 5 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 35ef21b..0be0656 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -311,7 +311,7 @@ void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
clear_bit(X86_FEATURE_XSAVE, boot_cpu_data.x86_capability);
 
if ( cpu_has_xsave )
-   xstate_init(c == _cpu_data);
+   xstate_init(c);
 
/*
 * The vendor-specific functions might have changed features.  Now
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 9f5a6c6..07feb6d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -935,10 +935,7 @@ void pv_cpuid(struct cpu_user_regs *regs)
 goto unsupported;
 if ( regs->_ecx == 1 )
 {
-a &= XSTATE_FEATURE_XSAVEOPT |
- XSTATE_FEATURE_XSAVEC |
- (cpu_has_xgetbv1 ? XSTATE_FEATURE_XGETBV1 : 0) |
- (cpu_has_xsaves ? XSTATE_FEATURE_XSAVES : 0);
+a &= boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32];
 if ( !cpu_has_xsaves )
 b = c = d = 0;
 }
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index d5f5e3b..3e107a2 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -14,11 +14,6 @@
 #include 
 #include 
 
-static bool_t __read_mostly cpu_has_xsaveopt;
-static bool_t __read_mostly cpu_has_xsavec;
-bool_t __read_mostly cpu_has_xgetbv1;
-bool_t __read_mostly cpu_has_xsaves;
-
 /*
  * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all
  * the supported and enabled features on the processor, including the
@@ -281,8 +276,9 @@ unsigned int xstate_ctxt_size(u64 xcr0)
 }
 
 /* Collect the information of processor's extended state */
-void xstate_init(bool_t bsp)
+void xstate_init(struct cpuinfo_x86 *c)
 {
+bool_t bsp = c == _cpu_data;
 u32 eax, ebx, ecx, edx;
 u64 feature_mask;
 
@@ -325,20 +321,15 @@ void xstate_init(bool_t bsp)
 
 /* Check extended XSAVE features. */
 cpuid_count(XSTATE_CPUID, 1, , , , );
-if ( bsp )
-{
-cpu_has_xsaveopt = !!(eax & XSTATE_FEATURE_XSAVEOPT);
-cpu_has_xsavec = !!(eax & XSTATE_FEATURE_XSAVEC);
-/* XXX cpu_has_xgetbv1 = !!(eax & XSTATE_FEATURE_XGETBV1); */
-/* XXX cpu_has_xsaves = !!(eax & XSTATE_FEATURE_XSAVES); */
-}
-else
-{
-BUG_ON(!cpu_has_xsaveopt != !(eax & XSTATE_FEATURE_XSAVEOPT));
-BUG_ON(!cpu_has_xsavec != !(eax & XSTATE_FEATURE_XSAVEC));
-/* XXX BUG_ON(!cpu_has_xgetbv1 != !(eax & XSTATE_FEATURE_XGETBV1)); */
-/* XXX BUG_ON(!cpu_has_xsaves != !(eax & XSTATE_FEATURE_XSAVES)); */
-}
+
+/* Mask out features not currently understood by Xen. */
+eax &= (cpufeat_mask(X86_FEATURE_XSAVEOPT) |
+cpufeat_mask(X86_FEATURE_XSAVEC));
+
+c->x86_capability[X86_FEATURE_XSAVEOPT / 32] = eax;
+
+if ( !bsp )
+BUG_ON(eax != boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32]);
 }
 
 static bool_t valid_xcr0(u64 xcr0)
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 9a01563..aa228db 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -9,7 +9,7 @@
 #define __ASM_I386_CPUFEATURE_H
 #endif
 
-#define NCAPINTS   8   /* N 32-bit words worth of info */
+#define NCAPINTS   9   /* N 32-bit words worth of info */
 
 /* Intel-defined CPU features, CPUID level 0x0001 (edx), word 0 */
 #define X86_FEATURE_FPU(0*32+ 0) /* Onboard FPU */
@@ -154,6 +154,12 @@
 #define X86_FEATURE_ADX(7*32+19) /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP   (7*32+20) /* Supervisor Mode Access Prevention 
*/
 
+/* Intel-defined CPU features, CPUID level 0x000D:1 (eax), word 8 */
+#define X86_FEATURE_XSAVEOPT   (8*32+ 0)
+#define X86_FEATURE_XSAVEC (8*32+ 1)
+#define

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops



On 17/09/2015 10:58, Peter Zijlstra wrote:
> But the far greater problem I have with the whole virt thing is that
> you cannot use rdmsr_safe() to probe if an MSR exists at all because, as
> you told me, these virt thingies return 0 for all 'unknown' MSRs instead
> of faulting.

At least for KVM, that behavior is opt-in (the ignore_msrs parameter)
and no distro that I know enables it by default.

Paolo

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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On 09/17/2015 10:38 AM, George Dunlap wrote:
> Is it the case that the interrupt is not actually delivered to the
> processor, but that the pending bit will be set in the pi field, so that
> the interrupt will be delivered the next time the hypervisor returns
> into the guest?
> 
> (I am assuming that is the case, because if the hypervisor *does* get an
> interrupt, then it can just unblock it there.)

Actually, it looks like you *do* in fact get a
pi_notification_interrupt() in this case.  Could we to check to see if
the current vcpu is blocked and unblock it?

I haven't yet decided whether I prefer my original suggestion of
switching the interrupt and putting things on the wake-up list in
vcpu_block(), or of deferring adding things to the wake-up list until
the actual context switch.

 -George

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

Re: [Xen-devel] Newbie

Cc Lars

On Thu, Sep 17, 2015 at 02:33:43PM +0530, Lasya Venneti wrote:
> Hi everyone,
> 
> I'm Lasya a student studying in IIIT-H, Hyderabad, India.
> 
> I wish to participate in round 11 of Outreachy this time. I would be
> grateful if someone would direct me as to how I am supposed to start
> contributing to Xen Project for this Outreachy round.
> 
> Sincerely,
> Lasya V

> ___
> 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 for-4.6] libxl: handle read-only drives with qemu-xen

On Thu, 2015-09-17 at 12:35 +0100, Wei Liu wrote:
> On Thu, Sep 17, 2015 at 12:13:20PM +0100, Ian Campbell wrote:
> > On Wed, 2015-09-16 at 14:54 +0100, Ian Jackson wrote:
> > > M A Young writes ("Re: [PATCH v2 for-4.6] libxl: handle read-only
> > > drives
> > > with qemu-xen"):
> > > > On Tue, 15 Sep 2015, Stefano Stabellini wrote:
> > > > Is ERROR_INVAL the right error to return? I get
> > > > 
> > > > libxl_dm.c: In function 'libxl__build_device_model_args_new':
> > > > libxl_dm.c:807:28: error: return makes pointer from integer without
> > > > a
> > > > cast 
> > > > [-Werror=int-conversion]
> > > >  return ERROR_INVAL;
> > > > ^
> > > > cc1: all warnings being treated as errors
> > > 
> > > Yikes.
> > > 
> > > > when I try to build xen with the proposed patch.  NULL is returned
> > > > when
> > > > there is a problem in other places in this function.
> > > 
> > > Clearly not.
> > > 
> > > Stefano, do you want to respin ?
> > 
> > > From the other subthread it seems this is down to:
> > 
> > commit de214e9f93de57fb5239e958372f314d29d3f7a9
> > Author: Olaf Hering 
> > Date:   Mon Apr 20 13:40:31 2015 +
> > 
> > libxl: pass environment to device model
> > 
> > Prepare device-model setup functions to pass also environment
> > variables
> > to the spawned process. This is required for upcoming changes
> > which will
> > set DISPLAY and XAUTHORITY for SDL.
> > 
> > Signed-off-by: Olaf Hering 
> > Cc: Ian Jackson 
> > Cc: Stefano Stabellini  >  >
> > Cc: Ian Campbell 
> > Cc: Wei Liu 
> > Acked-by: Ian Campbell 
> > 
> > which is 4.6 but not in 4.5 (where Michael was applying). So I think
> > this
> > is just an issue for backport (should return NULL instead) and not for
> > applying now.
> > 
> > So shall we go ahead with this for 4.6 or is there more
> > testing/discussion/whatever needed?
> > 
> 
> Yes, of course.

"Yes, of course, go ahead" or "Yes, of course, more
testing/discusion/whatever is needed" ?

Yes is not a helpful answer to a question which offers a choice ;-)

Ian.

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

Re: [Xen-devel] [PULL 0/19] xen-2015-09-08-tag

2015-09-17 Thread Stefano Stabellini

On Wed, 16 Sep 2015, Chen, Tiejun wrote:
> On 9/15/2015 7:00 PM, Paolo Bonzini wrote:
> > 
> > 
> > On 15/09/2015 11:55, Stefano Stabellini wrote:
> > > On Mon, 14 Sep 2015, Paolo Bonzini wrote:
> > > > > On 10/09/2015 12:29, Stefano Stabellini wrote:
> > > > > > > +if (lseek(config_fd, pos, SEEK_SET) != pos) {
> > > > > > > +return -errno;
> > > > > > > +}
> > > > > > >  do {
> > > > > > > -rc = pread(config_fd, (uint8_t *), len, pos);
> > > > > > > +rc = read(config_fd, (uint8_t *), len);
> > > > > > >  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> > > > >
> > > > > This leaks config_fd.
> > > I don't follow, it leaks config_fd where?
> > 
> > Where lseek returns -errno (and IIRC in other places in the same function).
> 
> Do you mean we need this change?

Yes, please send out a separate patch. Add my Acked-by.


> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1fb71c8..7d44228 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -775,15 +775,18 @@ static int host_pci_config_read(int pos, int len,
> uint32_t val)
>  }
> 
>  if (lseek(config_fd, pos, SEEK_SET) != pos) {
> +close(config_fd);
>  return -errno;
>  }
>  do {
>  rc = read(config_fd, (uint8_t *), len);
>  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>  if (rc != len) {
> +close(config_fd);
>  return -errno;
>  }
> 
> +close(config_fd);
>  return 0;
>  }
> 
> 
> Thanks
> Tiejun
> 

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

[Xen-devel] [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

This is a copy of the `THE REST' entry but with my own entry removed
(as I do not normally review hypervisor patches) and Andrew Cooper's
added (which seems appropriate given his status as x86 hypervisor
maintainer).

The effect is that patches touching xen/common/ will no longer be CC'd
to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
patches touching xen/common/, not just ones which match one of their
existing more specific entries.

Signed-off-by: Ian Jackson 
CC: Ian Campbell 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
CC: Julien Grall 
CC: David Vrabel 
CC: Andrew Cooper 

---
v2: A completely different approach following discussion with Ian C.
---
 MAINTAINERS |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a6bece6..e35c1e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -404,6 +404,16 @@ F:  xen/include/xsm/
 F:  xen/xsm/
 F:  docs/misc/xsm-flask.txt
 
+HYPERVISOR CORE
+M: Jan Beulich 
+M: Tim Deegan 
+M: Keir Fraser 
+M: Ian Campbell 
+M: Andrew Cooper 
+L: xen-devel@lists.xen.org
+S: Supported
+F: xen/common/
+
 THE REST
 M: Ian Campbell 
 M: Ian Jackson 
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

On 17/09/15 12:53, Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed
> (as I do not normally review hypervisor patches) and Andrew Cooper's
> added (which seems appropriate given his status as x86 hypervisor
> maintainer).
>
> The effect is that patches touching xen/common/ will no longer be CC'd
> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
> patches touching xen/common/, not just ones which match one of their
> existing more specific entries.
>
> Signed-off-by: Ian Jackson 
> CC: Ian Campbell 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Julien Grall 
> CC: David Vrabel 
> CC: Andrew Cooper 
>
> ---
> v2: A completely different approach following discussion with Ian C.
> ---
>  MAINTAINERS |   10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..e35c1e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +HYPERVISOR CORE
> +M:   Jan Beulich 
> +M:   Tim Deegan 
> +M:   Keir Fraser 
> +M:   Ian Campbell 
> +M:   Andrew Cooper 
> +L:   xen-devel@lists.xen.org
> +S:   Supported
> +F:   xen/common/

I am happy with the change in principle, but it should cover xen/ rather
than xen/common/ to also include otherwise unqualified items in
xen/drivers/.

~Andrew

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

Re: [Xen-devel] [PATCH] x86/sysctl: Don't clobber memory if NCAPINTS > ARRAY_SIZE(pi->hw_cap)

On Wed, 2015-09-16 at 16:01 +0100, Wei Liu wrote:
> On Wed, Sep 16, 2015 at 10:01:45AM +0100, Andrew Cooper wrote:
> > There is no current problem, as both NCAPINTS and pi->hw_cap are 8
> > entries,
> > but the limit should be calculated appropriately so as to avoid
> > hypervisor
> > stack corruption if the two do get out of sync.
> > 
> > Signed-off-by: Andrew Cooper 
> > ---
> > CC: Jan Beulich 
> > CC: Wei Liu 
> > 
> > I came across this during my cpuid levelling work.  As I know I am not
> > the
> > only person playing with NCAPINTS at the moment, I am posting this
> > ahead of
> > the rest of the work.
> > 
> > Wei: Concerning 4.6, it might we worth taking this, as it will likely
> > bite
> > downstream distributers who backport a 4.7 feature.
> > 
> 
> Release-acked-by: Wei Liu 

Andy tells me that Jan is away so I have cherry-picked this
(c373b912e74659f0e0898ae93e89513694cfd94e) to staging-4.6 at his request.

Ian.

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

Re: [Xen-devel] [PATCH] pci-attach: fix assertation

On Wed, 2015-09-16 at 15:08 +0100, Wei Liu wrote:
> On Wed, Sep 16, 2015 at 09:25:25AM +0100, Ian Campbell wrote:
> > On Wed, 2015-09-16 at 14:16 +0800, Chunyan Liu wrote:
> > 
> > For the subject I prefer to avoid "fix " style messages. In this
> > case something like "libxl: ensure xs transaction is initialised in
> > libxl__device_pci_add_xenstore" would be better.
> > 
> > > Run "xl pci-attach  ", the 2nd time fails:
> > > xl: libxl_xshelp.c:209: libxl__xs_transaction_start: Assertion `!*t'
> > > failed.
> > > Aborted
> > > 
> > > To fix that, initialize xs_transaction to avoid
> > > libxl__xs_transaction_start assertation error.
> > 
> > "assertion".
> > 
> > > Signed-off-by: Chunyan Liu 
> > 
> > With an updated commit message (which I can do as I commit):
> > Acked-by: Ian Campbell 
> > 
> > Wei, I think we want this for 4.6.
> > 
> 
> Release-acked-by: Wei Liu 

Applied to staging and staging-4.6.

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

Re: [Xen-devel] Newbie - OutreachY round 11 (Dec 7 to March 7)

2015-09-17 Thread Lars Kurth

Hi all,

we do have two OutreachY slots and I just received information related
about the timeline for OutreachY yesterday. See below

September 28 organizations' landing pages need to be ready with project
ideas
September 29 application process opens
November 2 application deadline
November 17 accepted applicants announced
December 7 - March 7 internship dates

For now, we can use the information on
http://wiki.xenproject.org/wiki/Outreachy/Round10 and then update the
project lists (under "Community Reviewed Project List")

@Lasya, is your interest in Hypervisor work? Or higher level work such as
Mirage OS? 
For the former starting information can be found at
http://wiki.xenproject.org/wiki/Category:Developers - but we should also
possibly haver some projects in areas such as Xen - OpenStack integration,
Xen - Libvirt integration, Raisin
(https://blog.xenproject.org/2015/06/28/project-raisin-raise-xen/), and
many other areas that are not reflected in the project list

Depending on your interest, the existing project list or some areas you
may be interested in, we can start updating "Community Reviewed Project
List"

@Wei, @Stefano, @Konrad and everyone else: do we have any smaller work
items / bug fixes / etc. that need doing and would be suitable for Lasya?
Remember, the purpose of these items, is to get familiar with set-up and
the community processes. Maybe some stuff coverity has thrown up to start
with and then maybe something more complex a little later. We then also
need to find mentors and update projects.

Best Regards
Lars

On 17/09/2015 12:49, "Wei Liu"  wrote:

>Cc Lars
>
>On Thu, Sep 17, 2015 at 02:33:43PM +0530, Lasya Venneti wrote:
>> Hi everyone,
>> 
>> I'm Lasya a student studying in IIIT-H, Hyderabad, India.
>> 
>> I wish to participate in round 11 of Outreachy this time. I would be
>> grateful if someone would direct me as to how I am supposed to start
>> contributing to Xen Project for this Outreachy round.
>> 
>> Sincerely,
>> Lasya V
>
>> ___
>> 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] MAINTAINERS: Document maintainers for xen/common/

2015-09-17 Thread Julien Grall

On 17/09/15 12:59, Andrew Cooper wrote:
> On 17/09/15 12:53, Ian Jackson wrote:
>> This is a copy of the `THE REST' entry but with my own entry removed
>> (as I do not normally review hypervisor patches) and Andrew Cooper's
>> added (which seems appropriate given his status as x86 hypervisor
>> maintainer).
>>
>> The effect is that patches touching xen/common/ will no longer be CC'd
>> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
>> patches touching xen/common/, not just ones which match one of their
>> existing more specific entries.
>>
>> Signed-off-by: Ian Jackson 
>> CC: Ian Campbell 
>> CC: Jan Beulich 
>> CC: Keir Fraser 
>> CC: Tim Deegan 
>> CC: Julien Grall 
>> CC: David Vrabel 
>> CC: Andrew Cooper 
>>
>> ---
>> v2: A completely different approach following discussion with Ian C.
>> ---
>>  MAINTAINERS |   10 ++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6bece6..e35c1e9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>>  F:  xen/xsm/
>>  F:  docs/misc/xsm-flask.txt
>>  
>> +HYPERVISOR CORE
>> +M:  Jan Beulich 
>> +M:  Tim Deegan 
>> +M:  Keir Fraser 
>> +M:  Ian Campbell 
>> +M:  Andrew Cooper 
>> +L:  xen-devel@lists.xen.org
>> +S:  Supported
>> +F:  xen/common/
> 
> I am happy with the change in principle, but it should cover xen/ rather
> than xen/common/ to also include otherwise unqualified items in
> xen/drivers/.

The first version of this patch was covering xen/. But the downside was
to CC all the "REST OF THE HYPERVISOR" group to any changes under xen/.
I.e you would be CCed on ARM changes for instance.

See http://lists.xen.org/archives/html/xen-devel/2015-09/msg02171.html

I know some maintainers which will complaining very quickly on the ML
and ask the contributors to use correctly scripts/get_maintainers.pl.

But the scrips doesn't do correctly his job for Xen...

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v2 for-4.6] libxl: handle read-only drives with qemu-xen

On Thu, Sep 17, 2015 at 12:49:41PM +0100, Ian Campbell wrote:
[...]
> > > So shall we go ahead with this for 4.6 or is there more
> > > testing/discussion/whatever needed?
> > > 
> > 
> > Yes, of course.
> 
> "Yes, of course, go ahead" or "Yes, of course, more
> testing/discusion/whatever is needed" ?
> 
> Yes is not a helpful answer to a question which offers a choice ;-)
> 

I think this patch should be applied to 4.6 and staging. The patch
itself is correct.

Wei.

> Ian.

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

Re: [Xen-devel] [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters

2015-09-17 Thread Shannon Zhao

Hi,

>From the comments on this patch, IIUC, we don't object to the change
brought by this patch. What we didn't reach an agreement is how to
support runtime service for Dom0. Right? If so, I think this patch
doesn't conflict with adding support for runtime service in the future.
So could we move this patch forward? Then I could continue working on
adding ARM ACPI support on Xen.

Any comments?

Thanks,
-- 
Shannon

On 2015/9/10 16:41, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> These EFI stub parameters are used to internal communication between EFI
> stub and Linux kernel and EFI stub creates these parameters. But for Xen
> on ARM when booting with UEFI, Xen will create a minimal DT providing
> these parameters for Dom0 and Dom0 is not only Linux kernel, but also
> other OS (such as FreeBSD) which will be used in the future. So here
> we plan to standardize the names by dropping the prefix "linux," and
> make them common to other OS. Also this will not break the compatibility
> since these parameters are used to internal communication between EFI
> stub and kernel.
> 
> Signed-off-by: Shannon Zhao 
> ---
> Look at [1] for the discussion about this in Xen ML. The purpose of this
> patch is to standardize the names to make Linux ARM kernel work on Xen
> with UEFI. Also it hopes other OS(e.g. FreeBSD), which will be used as
> Dom0 on Xen, could support this mechanism as well.
> 
> [1]http://lists.xenproject.org/archives/html/xen-devel/2015-08/msg02250.html
> 
>  Documentation/arm/uefi.txt | 10 +-
>  drivers/firmware/efi/efi.c | 10 +-
>  drivers/firmware/efi/libstub/fdt.c | 10 +-
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/arm/uefi.txt b/Documentation/arm/uefi.txt
> index d60030a..8c83243 100644
> --- a/Documentation/arm/uefi.txt
> +++ b/Documentation/arm/uefi.txt
> @@ -45,18 +45,18 @@ following parameters:
>  
> 
>  Name  | Size   | Description
>  
> 
> -linux,uefi-system-table   | 64-bit | Physical address of the UEFI System 
> Table.
> +uefi-system-table | 64-bit | Physical address of the UEFI System 
> Table.
>  
> 
> -linux,uefi-mmap-start | 64-bit | Physical address of the UEFI memory map,
> +uefi-mmap-start   | 64-bit | Physical address of the UEFI memory map,
>|| populated by the UEFI GetMemoryMap() 
> call.
>  
> 
> -linux,uefi-mmap-size  | 32-bit | Size in bytes of the UEFI memory map
> +uefi-mmap-size| 32-bit | Size in bytes of the UEFI memory map
>|| pointed to in previous entry.
>  
> 
> -linux,uefi-mmap-desc-size | 32-bit | Size in bytes of each entry in the UEFI
> +uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
>|| memory map.
>  
> 
> -linux,uefi-mmap-desc-ver  | 32-bit | Version of the mmap descriptor format.
> +uefi-mmap-desc-ver| 32-bit | Version of the mmap descriptor format.
>  
> 
>  linux,uefi-stub-kern-ver  | string | Copy of linux_banner from build.
>  
> 
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d6144e3..3878715 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -481,11 +481,11 @@ static __initdata struct {
>   int offset;
>   int size;
>  } dt_params[] = {
> - UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> - UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> - UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> - UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> - UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> + UEFI_PARAM("System Table", "uefi-system-table", system_table),
> + UEFI_PARAM("MemMap Address", "uefi-mmap-start", mmap),
> + UEFI_PARAM("MemMap Size", "uefi-mmap-size", mmap_size),
> + UEFI_PARAM("MemMap Desc. Size", "uefi-mmap-desc-size", desc_size),
> + UEFI_PARAM("MemMap Desc. Version", "uefi-mmap-desc-ver", desc_ver)
>  };
>  
>  struct param_info {
> diff --git a/drivers/firmware/efi/libstub/fdt.c 
> b/drivers/firmware/efi/libstub/fdt.c
> index ef5d764..e94589a 100644
> ---

Re: [Xen-devel] [PATCH v4 1/4] x86: Support enable CDP by boot parameter and add get CDP status

On 17/09/15 10:35, He Chen wrote:
> Add boot parameter `psr=cdp` to enable CDP at boot time.
> Intel Code/Data Prioritization(CDP) feature is based on CAT. Note that
> cos_max would be half when CDP is on. struct psr_cat_cbm is extended to
> support CDP operation. Extend psr_get_cat_l3_info sysctl to get CDP
> status.
>
> Signed-off-by: He Chen 
> ---
>  docs/misc/xen-command-line.markdown | 11 -
>  xen/arch/x86/psr.c  | 84 
> ++---
>  xen/arch/x86/sysctl.c   |  5 ++-
>  xen/include/asm-x86/msr-index.h |  3 ++
>  xen/include/asm-x86/psr.h   | 11 -
>  xen/include/public/sysctl.h |  4 +-
>  6 files changed, 98 insertions(+), 20 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index a2e427c..d92e323 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1165,9 +1165,9 @@ This option can be specified more than once (up to 8 
> times at present).
>  > `= `
>  
>  ### psr (Intel)
> -> `= List of ( cmt: | rmid_max: | cat: | 
> cos_max: )`
> +> `= List of ( cmt: | rmid_max: | cat: | 
> cos_max: | cdp: )`
>  
> -> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255`
> +> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255,cdp:0`
>  
>  Platform Shared Resource(PSR) Services.  Intel Haswell and later server
>  platforms offer information about the sharing of resources.
> @@ -1197,6 +1197,13 @@ The following resources are available:
>the cache allocation.
>* `cat` instructs Xen to enable/disable Cache Allocation Technology.
>* `cos_max` indicates the max value for COS ID.
> +* Code and Data Prioritization Technology (Broadwell and later). Information
> +  regarding the code cache and the data cache allocation. CDP is based on 
> CAT.
> +  * `cdp` instructs Xen to enable/disable Code and Data Prioritization. Note
> +that `cos_max` of CDP is a little different from `cos_max` of CAT. With
> +CDP, one COS will corespond two CBMs other than one with CAT, due to the
> +sum of CBMs is fixed, that means actual `cos_max` in use will 
> automatically
> +reduce to half when CDP is enabled.
>  
>  ### reboot
>  > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | 
> [c]old]`
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index c0daa2e..e44ed8b 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -21,9 +21,16 @@
>  
>  #define PSR_CMT(1<<0)
>  #define PSR_CAT(1<<1)
> +#define PSR_CDP(1<<2)
>  
>  struct psr_cat_cbm {
> -uint64_t cbm;
> +union {
> +uint64_t cbm;
> +struct {
> +uint64_t code;
> +uint64_t data;
> +};
> +} u;

You can also drop this u which will further reduce the diff later in the
patch.

With this change, Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH] MAINTAINERS: Split out maintainers for the hypervisor

2015-09-17 Thread Julien Grall

Hi Ian,

On 17/09/15 11:16, Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed,
> as I do not normally review hypervisor patches.
> 
> I have chosen to be conservative and make a minimal change, rather
> than trying to describe a desirable state, or reflect reality.
> 
> The obvious questions to me, that I have not answered, are:
>  - Does Ian Campbell want to be in this list ?
>  - Should some of the arch-specific Xen maintainers be in this list ?
>  - Are there others who should be listed ?
> 
> I have tried to include in the CC list the active people listed in
> MAINTAINERS for any of the principal components under xen/.

There is a slight problem with this change. The group "REST OF THE
HYPERVISOR" is now CCed to any file under xen.

Before:

42sh> scripts/get_maintainers.pl -f xen/arch/arm/gic.c
Ian Campbell 
Stefano Stabellini 
xen-devel@lists.xen.org

After:

42sh> scripts/get_maintainers.pl -f xen/arch/arm/gic.c
Ian Campbell 
Stefano Stabellini 
Jan Beulich 
Tim Deegan 
Keir Fraser 
xen-devel@lists.xen.org


I suspect this is a bug in get_maintainers.pl because we had to tweak it
for "THE REST" group. It may be necessary to do the same for "REST OF
THE HYPERVISOR"?

Regards,

> 
> Signed-off-by: Ian Jackson 
> CC: Ian Campbell 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Julien Grall 
> CC: Suravee Suthikulpanit 
> CC: Aravind Gopalakrishnan 
> CC: Boris Ostrovsky 
> CC: Suravee Suthikulpanit 
> CC: Aravind Gopalakrishnan 
> CC: Stefano Stabellini 
> CC: Juergen Gross 
> CC: David Vrabel 
> CC: Gang Wei 
> CC: Shane Wang 
> CC: Yang Zhang 
> CC: Kevin Tian 
> CC: Jun Nakajima 
> CC: Liu Jinsong 
> CC: George Dunlap 
> CC: Konrad Rzeszutek Wilk 
> CC: Razvan Cojocaru 
> CC: Tamas K Lengyel 
> CC: Daniel De Graaf 
> ---
>  MAINTAINERS |9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..45fe426 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,15 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +REST OF THE HYPERVISOR
> +M:   Jan Beulich 
> +M:   Tim Deegan 
> +M:   Keir Fraser 
> +M:   Ian Campbell 
> +L:   xen-devel@lists.xen.org
> +S:   Supported
> +F:   xen/
> +
>  THE REST
>  M:   Ian Campbell 
>  M:   Ian Jackson 
> 


-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v4 2/4] x86: add domctl cmd to set/get CDP code/data CBM

On 17/09/15 10:35, He Chen wrote:
> @@ -375,10 +401,48 @@ static int write_l3_cbm(unsigned int socket, unsigned 
> int cos,
>  return 0;
>  }
>  
> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
> +uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
>  {
> -unsigned int old_cos, cos;
> -struct psr_cat_cbm *map, *found = NULL;
> +unsigned int cos;
> +
> +for ( cos = 0; cos <= cos_max; cos++ )
> +{
> +if( map[cos].ref &&
> +((!cdp_enabled && map[cos].u.cbm == cbm_code) ||
> +(cdp_enabled && map[cos].u.code == cbm_code &&

Correct alignment here would have one extra space, as the ( should match
the inner ( of the line above.

> @@ -387,53 +451,80 @@ int psr_set_l3_cbm(struct domain *d, unsigned int 
> socket, uint64_t cbm)
>  if ( !psr_check_cbm(info->cbm_len, cbm) )
>  return -EINVAL;
>  
> +if ( !cdp_enabled && (type == PSR_CBM_TYPE_L3_CODE ||
> +  type == PSR_CBM_TYPE_L3_DATA) )
> +return -EINVAL;
> +
> +cos_max = info->cos_max;
>  old_cos = d->arch.psr_cos_ids[socket];
>  map = info->cos_to_cbm;
>  
> -spin_lock(>cbm_lock);
> -
> -for ( cos = 0; cos <= info->cos_max; cos++ )
> +switch( type )
>  {
> -/* If still not found, then keep unused one. */
> -if ( !found && cos != 0 && map[cos].ref == 0 )
> -found = map + cos;
> -else if ( map[cos].u.cbm == cbm )
> -{
> -if ( unlikely(cos == old_cos) )
> -{
> -ASSERT(cos == 0 || map[cos].ref != 0);
> -spin_unlock(>cbm_lock);
> -return 0;
> -}
> -found = map + cos;
> -break;
> -}
> -}
> +case PSR_CBM_TYPE_L3:
> +cbm_code = cbm;
> +cbm_data = cbm;
> +break;
>  
> -/* If old cos is referred only by the domain, then use it. */
> -if ( !found && map[old_cos].ref == 1 )
> -found = map + old_cos;
> +case PSR_CBM_TYPE_L3_CODE:
> +cbm_code = cbm;
> +cbm_data = map[old_cos].u.data;
> +break;
>  
> -if ( !found )
> -{
> -spin_unlock(>cbm_lock);
> -return -EOVERFLOW;
> +case PSR_CBM_TYPE_L3_DATA:
> +cbm_code = map[old_cos].u.code;
> +cbm_data = cbm;
> +break;
> +
> +default:
> +ASSERT_UNREACHABLE();

Alignment here as well.

With these two alignment issues fixed, and the knock-on effect of
dropping .u from the previous patch, Reviewed-by: Andrew Cooper


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

Re: [Xen-devel] [PATCH XEN] Config: Switch to unified qemu trees.

Ian Campbell writes ("[PATCH XEN] Config: Switch to unified qemu trees."):
> Upstream qemu is not in qemu-xen.git and the trad fork is in
> qemu-xen-traditional.h
> 
> QEMU_UPSTREAM_REVISION is currently a tag and
> QEMU_TRADITIONAL_REVISION is a specific revision, so no changes are
> required to those.

When this is backported, something will have to be done about that, of
course.  Otherwise stable branches might pick up qemu-*#master

> Signed-off-by: Ian Campbell 
> (cherry picked from commit dff5c395c1d23c21238ce17ddcd6f7abe2efd08d)
> 
> Conflicts:
>   Config.mk

Did you intend that to appear here ?

Ian.

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

Re: [Xen-devel] [PATCH] MAINTAINERS: Split out maintainers for the hypervisor

Julien Grall writes ("Re: [PATCH] MAINTAINERS: Split out maintainers for the 
hypervisor"):
> There is a slight problem with this change. The group "REST OF THE
> HYPERVISOR" is now CCed to any file under xen.

Eeek.  (Dropping many of the CCs.)

> I suspect this is a bug in get_maintainers.pl because we had to tweak it
> for "THE REST" group. It may be necessary to do the same for "REST OF
> THE HYPERVISOR"?

I had missed us adding a special case for `THE REST'.

I will come back with a proper fix to this.

Ian.

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

Re: [Xen-devel] [PATCH v4 3/4] tools: add tools support for Intel CDP

On 17/09/15 10:35, He Chen wrote:
> @@ -2798,7 +2798,9 @@ enum xc_psr_cmt_type {
>  typedef enum xc_psr_cmt_type xc_psr_cmt_type;
>  
>  enum xc_psr_cat_type {
> -XC_PSR_CAT_L3_CBM = 1,
> +XC_PSR_CAT_L3_CBM  = 1,
> +XC_PSR_CAT_L3_CODE = 2,
> +XC_PSR_CAT_L3_DATA = 3,
>  };

No need for the explicit assignments here.  The exact values are not
interesting and guaranteed to be as currently assigned.

>  typedef enum xc_psr_cat_type xc_psr_cat_type;
>
> @@ -306,7 +318,8 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
> uint32_t domid,
>  }
>  
>  int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
> -   uint32_t *cos_max, uint32_t *cbm_len)
> +   uint32_t *cos_max, uint32_t *cbm_len,
> +   bool *cdp_enabled)
>  {
>  int rc;
>  DECLARE_SYSCTL;
> @@ -320,6 +333,8 @@ int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t 
> socket,
>  {
>  *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
>  *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
> +*cdp_enabled = sysctl.u.psr_cat_op.u.l3_info.flags &
> +   XEN_SYSCTL_PSR_CAT_L3_CDP;

!!(sysctl.u.psr_cat_op.u.l3_info.flags & XEN_SYSCTL_PSR_CAT_L3_CDP);

To turn it into a proper boolean, rather than a just a non-zero integer.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ebbb9a5..8128f54 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8390,6 +8390,10 @@ static int psr_cat_hwinfo(void)
>  }
>  printf("%-16s: %u\n", "Socket ID", socketid);
>  printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> +if (info->cdp_enabled)
> +printf("%-16s: Enabled\n", "CDP Status");
> +else
> +printf("%-16s: Disabled\n", "CDP Status");

printf("%-16s: %sabled\n", "CDP Status", info->cdp_enabled ? "En" : "Dis");

is rather shorter, if you prefer.

>  printf("%-16s: %u\n", "Maximum COS", info->cos_max);
>  printf("%-16s: %u\n", "CBM length", info->cbm_len);
>  printf("%-16s: %#llx\n", "Default CBM",
> @@ -8401,7 +8405,8 @@ out:
>  return rc;
>  }
>  
> -static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid)
> +static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
> + bool cdp_enabled)
>  {
>  char *domain_name;
>  uint64_t cbm;
> @@ -8410,20 +8415,29 @@ static void psr_cat_print_one_domain_cbm(uint32_t 
> domid, uint32_t socketid)
>  printf("%5d%25s", domid, domain_name);
>  free(domain_name);
>  
> -if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
> -   socketid, ))
> - printf("%#16"PRIx64, cbm);
> -
> +if (!cdp_enabled) {
> +if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
> +   socketid, ))
> +printf("%#16"PRIx64, cbm);

Mixing # and an explicit width of the integer will clip two nibbles of
the integer.

In this situation, the correct formatting is "0x%016"PRIx64, or
"%#018"PRIx64

> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 0071f12..13e7aa1 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -543,6 +543,8 @@ struct cmd_spec cmd_table[] = {
>"Set cache capacity bitmasks(CBM) for a domain",
>"[options]  ",
>"-sSpecify the socket to process, otherwise all 
> sockets are processed\n"
> +  "-cSet code CBM if CDP is supported\n"
> +  "-dSet data CBM if CDP is supported\n"
>  },
>  { "psr-cat-show",
>_psr_cat_show, 0, 1,
> @@ -551,6 +553,7 @@ struct cmd_spec cmd_table[] = {
>  },
>  
>  #endif
> +

Spurious whitespace change.

~Andrew

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

Re: [Xen-devel] [PATCH OSSTEST] Switch to merged qemu-xen{, -traditional}.git trees

On Thu, 2015-09-17 at 11:43 +0100, Ian Campbell wrote:
> > > +qemu-upstream-*-testing)
> > ...
> > > + # For now also push to the old split trees for historical
> > > + # branches only (qemu-upstream started with xen-4.2-testing
> > > + # and the split trees end at xen-4.6-testing)
> > > + case "$xenbranch" in
> > > + xen-4.[23456]-testing)
> > > + tree=$XENBITS:/home/xen/git/qemu-upstream
> > > -${xenbranch#xen-}.git
> > > + git push $tree $revision:refs/heads/master
> > 
> > I think this would be clearer if not done in terms of xenbranch.  I
> > would use case $branch and then $branchcore.  What do you think ?
> 
> Good idea.

$branch is a bit wordy at this point, but $branchcore is usable for both
cases incrementally:

diff --git a/ap-push b/ap-push
index c3bc814..41acb39 100755
--- a/ap-push
+++ b/ap-push
@@ -77,9 +77,9 @@ qemu-upstream-*-testing)
# For now also push to the old split trees for historical
# branches only (qemu-upstream started with xen-4.2-testing
# and the split trees end at xen-4.6-testing)
-   case "$xenbranch" in
-   xen-4.[23456]-testing)
-   tree=$XENBITS:/home/xen/git/qemu-upstream-${xenbranch#xen-}.git
+   case "$branchcore" in
+   4.[23456])
+   
tree=$XENBITS:/home/xen/git/qemu-upstream-${branchcore}-testing.git
git push $tree $revision:refs/heads/master
;;
esac

Look ok?


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

Re: [Xen-devel] [PATCH v2 for-4.6] libxl: handle read-only drives with qemu-xen

On Wed, 2015-09-16 at 14:54 +0100, Ian Jackson wrote:
> M A Young writes ("Re: [PATCH v2 for-4.6] libxl: handle read-only drives
> with qemu-xen"):
> > On Tue, 15 Sep 2015, Stefano Stabellini wrote:
> > Is ERROR_INVAL the right error to return? I get
> > 
> > libxl_dm.c: In function 'libxl__build_device_model_args_new':
> > libxl_dm.c:807:28: error: return makes pointer from integer without a
> > cast 
> > [-Werror=int-conversion]
> >  return ERROR_INVAL;
> > ^
> > cc1: all warnings being treated as errors
> 
> Yikes.
> 
> > when I try to build xen with the proposed patch.  NULL is returned when
> > there is a problem in other places in this function.
> 
> Clearly not.
> 
> Stefano, do you want to respin ?

>From the other subthread it seems this is down to:

commit de214e9f93de57fb5239e958372f314d29d3f7a9
Author: Olaf Hering 
Date:   Mon Apr 20 13:40:31 2015 +

libxl: pass environment to device model

Prepare device-model setup functions to pass also environment variables
to the spawned process. This is required for upcoming changes which will
set DISPLAY and XAUTHORITY for SDL.

Signed-off-by: Olaf Hering 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Ian Campbell 
Cc: Wei Liu 
Acked-by: Ian Campbell 

which is 4.6 but not in 4.5 (where Michael was applying). So I think this
is just an issue for backport (should return NULL instead) and not for
applying now.

So shall we go ahead with this for 4.6 or is there more
testing/discussion/whatever needed?

Ian.


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

Re: [Xen-devel] [PATCH] MAINTAINERS: Split out maintainers for the hypervisor

(cutting Cc to just those listed as REST plus Julien who suggested this
change)

On Thu, 2015-09-17 at 11:16 +0100, Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed,
> as I do not normally review hypervisor patches.
> 
> I have chosen to be conservative and make a minimal change, rather
> than trying to describe a desirable state, or reflect reality.
> 
> The obvious questions to me, that I have not answered, are:
>  - Does Ian Campbell want to be in this list ?
>  - Should some of the arch-specific Xen maintainers be in this list ?

I think under the second (for arm) I would want the first regardless of
other considerations.


>  - Are there others who should be listed ?
> 
> I have tried to include in the CC list the active people listed in
> MAINTAINERS for any of the principal components under xen/.
> 
> Signed-off-by: Ian Jackson 
> CC: Ian Campbell 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Julien Grall 
> CC: Suravee Suthikulpanit 
> CC: Aravind Gopalakrishnan 
> CC: Boris Ostrovsky 
> CC: Suravee Suthikulpanit 
> CC: Aravind Gopalakrishnan 
> CC: Stefano Stabellini 
> CC: Juergen Gross 
> CC: David Vrabel 
> CC: Gang Wei 
> CC: Shane Wang 
> CC: Yang Zhang 
> CC: Kevin Tian 
> CC: Jun Nakajima 
> CC: Liu Jinsong 
> CC: George Dunlap 
> CC: Konrad Rzeszutek Wilk 
> CC: Razvan Cojocaru 
> CC: Tamas K Lengyel 
> CC: Daniel De Graaf 
> ---
>  MAINTAINERS |9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..45fe426 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,15 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +REST OF THE HYPERVISOR
> +M:   Jan Beulich 
> +M:   Tim Deegan 
> +M:   Keir Fraser 
> +M:   Ian Campbell 
> +L:   xen-devel@lists.xen.org
> +S:   Supported
> +F:   xen/
> +
>  THE REST
>  M:   Ian Campbell 
>  M:   Ian Jackson 

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

Re: [Xen-devel] [PATCH v2 for-4.6] libxl: handle read-only drives with qemu-xen

2015-09-17 Thread M A Young

On Thu, 17 Sep 2015, Ian Campbell wrote:

> On Wed, 2015-09-16 at 14:54 +0100, Ian Jackson wrote:
> > M A Young writes ("Re: [PATCH v2 for-4.6] libxl: handle read-only drives
> > with qemu-xen"):
> > > On Tue, 15 Sep 2015, Stefano Stabellini wrote:
> > > Is ERROR_INVAL the right error to return? I get
> > > 
> > > libxl_dm.c: In function 'libxl__build_device_model_args_new':
> > > libxl_dm.c:807:28: error: return makes pointer from integer without a
> > > cast 
> > > [-Werror=int-conversion]
> > >  return ERROR_INVAL;
> > > ^
> > > cc1: all warnings being treated as errors
> > 
> > Yikes.
> > 
> > > when I try to build xen with the proposed patch.  NULL is returned when
> > > there is a problem in other places in this function.
> > 
> > Clearly not.
> > 
> > Stefano, do you want to respin ?
> 
> From the other subthread it seems this is down to:
> 
> commit de214e9f93de57fb5239e958372f314d29d3f7a9
> Author: Olaf Hering 
> Date:   Mon Apr 20 13:40:31 2015 +
> 
> libxl: pass environment to device model
> 
> Prepare device-model setup functions to pass also environment 
> variables
> to the spawned process. This is required for upcoming changes which 
> will
> set DISPLAY and XAUTHORITY for SDL.
> 
> Signed-off-by: Olaf Hering 
> Cc: Ian Jackson 
> Cc: Stefano Stabellini 
> Cc: Ian Campbell 
> Cc: Wei Liu 
> Acked-by: Ian Campbell 
> 
> which is 4.6 but not in 4.5 (where Michael was applying). So I think this
> is just an issue for backport (should return NULL instead) and not for
> applying now.
> 
> So shall we go ahead with this for 4.6 or is there more
> testing/discussion/whatever needed?

There is an update in the bugzilla report (testing 4.5 with NULL returned)
at https://bugzilla.redhat.com/show_bug.cgi?id=1257893#c9
querying whether xvd? read-only drives should work. I think the answer is 
still no, but I should at least mention the question here.

Michael Young

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

Re: [Xen-devel] [PATCH OSSTEST+XEN 0/1+1] Switching to one qemu tree per qemu version (trad vs upstream)

Ian Campbell writes ("[PATCH OSSTEST+XEN 0/1+1] Switching to one qemu tree per 
qemu version (trad vs upstream)"):
> As discussed[0] I think we should move away from having a pair of qemu
> repositories for each Xen branch and instead have a single pair (one for
> qemu-xen and one for qemu-xen-traditional).
> 
> I've prepared a pair of repositories:
> 
> 
> http://xenbits.xen.org/gitweb/?p=people/ianc/single-qemu-repo/qemu-xen-traditional.git;a=summary
> http://xenbits.xen.org/gitweb/?p=people/ianc/single-qemu-repo/qemu-xen.git;a=summary
> 
> These will eventually become, respectively:
> git://xenbits.xen.org/qemu-xen-traditional.git
> git://xenbits.xen.org/qemu-xen.git
> 
> Note that qemu-xen-traditional does not have an osstest gate, it is gated
> by changes to Config.mk in xen.git pulling in the specific revision.
> 
> In addition to the scheme described in [0] the qemu-mainline test output
> gate changes from:
> git://xenbits.xen.org/osstest/qemu.git#mainline/xen-tested-master
> to
> git://xenbits.xen.org/qemu-xen.git#upstream-tested
> 
> Thereby sharing a tree with our upstream branches, which I think makes
> sense. The input remains git://git.qemu.org/qemu.git#master, of course.
> 
> I will post two patches in reply to this, the first is for xen.git to make
> the obvious change to Config.mk.
> 
> The second is for osstest to change it to fetch from these new trees and
> push to them, and in addition push to the old trees for existing stable
> branches only (including 4.6).
> 
> I believe the plan for deployment should be:
> 
>  * Today we can already just remove the old staging/qemu-xen-* trees, they
>are unused (apart from being manually pushed to along with the staging
>trees, I think).

Yes.  (it needs some consequential changes to my ad-hoc scripts but
that's fine.)

>  * Push the osstest patch (possibly consider a force push? An osstest
>flight doesn't actually exercise this and it would make coordination
>with the next step easier...)

A force push would seem fine to me.

>  * ASAP after the osstest patch reaches production push the patch to
>xen.git#staging. This will cause the xen-unstable builds to use the new
>output gate. Until this is done unstable builds will continue using the
>old master push gate, which is not updated after the osstest update
>(only stable branches are), this isn't a big deal but obviously a
>smaller window would be better.

Right.

>  * At this point we could remove the old staging/qemu-upstream-* trees,
>they are not referenced by anything.

Promptly removing things that are unreferenced and not updated would
be a good idea :-).

>  * At our leisure backport the xen.git change to stable branches, probably
>back as far as 4.2 (when qemu-xen was introduced).
>  * Do stable releases of each of the above.

This will take quite some time.

>  * Drop (one by one or all at once) the push to the legacy stable branches
>from osstest as stable releases are made referencing the new trees.

I think it would be sensible to do this all at once and to expect to
do it perhaps 12 months from when we start.

>  * Consider hiding (or removing) the old output trees from xenbits as well.

Hiding them would be a good idea.  I wonder if that's possible.

Ian.

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

Re: [Xen-devel] [PATCH OSSTEST] Switch to merged qemu-xen{, -traditional}.git trees

Ian Campbell writes ("[PATCH OSSTEST] Switch to merged 
qemu-xen{,-traditional}.git trees"):
> The qemu-mainline flights now push to the upstream-tested branch of
> qemu-xen.git (while still pulling from upstream).

This mostly looks fine.

How are we going to test this ?  I think the most practical approach
is to add a step to your deployment plan (after the new trees are in
place, but before the osstest force push) where you explicitly ap-push
and ap-fetch the relevant trees.  You could do the ap-push as a user
without the appropriate permissions to get a dry run.

Then if this patch needs updating it can be fixed up quite easily.

> --- a/ap-push
> +++ b/ap-push
...
> +qemu-upstream-*-testing)
...
> + # For now also push to the old split trees for historical
> + # branches only (qemu-upstream started with xen-4.2-testing
> + # and the split trees end at xen-4.6-testing)
> + case "$xenbranch" in
> + xen-4.[23456]-testing)
> + tree=$XENBITS:/home/xen/git/qemu-upstream-${xenbranch#xen-}.git
> + git push $tree $revision:refs/heads/master

I think this would be clearer if not done in terms of xenbranch.  I
would use case $branch and then $branchcore.  What do you think ?

Ian.

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

Re: [Xen-devel] [PATCH OSSTEST+XEN 0/1+1] Switching to one qemu tree per qemu version (trad vs upstream)

On Thu, 2015-09-17 at 11:23 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST+XEN 0/1+1] Switching to one qemu tree 
> per qemu version (trad vs upstream)"):
> > I believe the plan for deployment should be:
> > 
> >  * Today we can already just remove the old staging/qemu-xen-* trees, they
> >are unused (apart from being manually pushed to along with the staging
> >trees, I think).
> 
> Yes.  (it needs some consequential changes to my ad-hoc scripts but
> that's fine.)

Which reminds me: there will no doubt be some knock on effects on the
release check list both for 4.7 and for existing stable branches which
pickup the Config.mk change.

> 
> >  * Push the osstest patch (possibly consider a force push? An osstest
> >flight doesn't actually exercise this and it would make coordination
> >with the next step easier...)
> 
> A force push would seem fine to me.
> 
> >  * ASAP after the osstest patch reaches production push the patch to
> >xen.git#staging. This will cause the xen-unstable builds to use the
> > new
> >output gate. Until this is done unstable builds will continue using
> > the
> >old master push gate, which is not updated after the osstest update
> >(only stable branches are), this isn't a big deal but obviously a
> >smaller window would be better.
> 
> Right.
> 
> >  * At this point we could remove the old staging/qemu-upstream-* trees,
> >they are not referenced by anything.
> 
> Promptly removing things that are unreferenced and not updated would
> be a good idea :-).

Right. My concern would be people with trees cloned from it, I suppose they
will get the git equivalent of a 404 and can update.

> 
> >  * At our leisure backport the xen.git change to stable branches,
> > probably
> >back as far as 4.2 (when qemu-xen was introduced).
> >  * Do stable releases of each of the above.
> 
> This will take quite some time.

Yes, I don't envisage this stage happening quickly. More "in the natural
course of things".

> 
> >  * Drop (one by one or all at once) the push to the legacy stable
> > branches
> >from osstest as stable releases are made referencing the new trees.
> 
> I think it would be sensible to do this all at once and to expect to
> do it perhaps 12 months from when we start.

Ack.

> >  * Consider hiding (or removing) the old output trees from xenbits as
> > well.
> 
> Hiding them would be a good idea.  I wonder if that's possible.

Sadly not in the mode which we use gitweb in which is to effectively do
"find $root" and export everything, with no blacklist support.

It is possible to switch to a mode which is default deny with an explicit
whitelist, which would involve some faff for each tree we create. I'm not
sure if it is worth it.

NB in that mode it should be possible to have repos which are present (i.e.
can be cloned) but not listed on the gitweb index page, and which still
have their own subpage if you know where to look.

I've deployed gitolite on my own VPS not so long ago, perhaps we should
look at this, as a medium term goal, it would also make it easier to give
people trees without ssh accounts and to do access control on various
shared trees etc.

Ian.

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

Re: [Xen-devel] [PATCH XEN] Config: Switch to unified qemu trees.

On Thu, 2015-09-17 at 11:32 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH XEN] Config: Switch to unified qemu
> trees."):
> > Upstream qemu is not in qemu-xen.git and the trad fork is in
> > qemu-xen-traditional.h
> > 
> > QEMU_UPSTREAM_REVISION is currently a tag and
> > QEMU_TRADITIONAL_REVISION is a specific revision, so no changes are
> > required to those.
> 
> When this is backported, something will have to be done about that, of
> course.  Otherwise stable branches might pick up qemu-*#master

Stable branches already have this switched out for a specific versioned tag
as part of the branching process, I believe.

> > Signed-off-by: Ian Campbell 
> > (cherry picked from commit dff5c395c1d23c21238ce17ddcd6f7abe2efd08d)
> > 
> > Conflicts:
> > Config.mk
> 
> Did you intend that to appear here ?

No. In fact I ran git send-email with my staging-4.5 branch checked out
where I had been checking how noisey the cherry pick was etc. Sorry.

The proper commit for staging is below.

FWIW the cherry-pick conflicts because the branch has the different split
tree, just accepting the version from the cherry-picked patch is correct.

Ian.

>From dff5c395c1d23c21238ce17ddcd6f7abe2efd08d Mon Sep 17 00:00:00 2001
From: Ian Campbell 
Date: Thu, 10 Sep 2015 14:31:34 +0100
Subject: [PATCH] Config: Switch to unified qemu trees.

Upstream qemu is now in qemu-xen.git and the trad fork is in
qemu-xen-traditional.h

QEMU_UPSTREAM_REVISION is currently a tag and
QEMU_TRADITIONAL_REVISION is a specific revision, so no changes are
required to those.

Signed-off-by: Ian Campbell 
---
 Config.mk | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Config.mk b/Config.mk
index 59607b4..a371cd9 100644
--- a/Config.mk
+++ b/Config.mk
@@ -242,14 +242,14 @@ endif
 
 ifeq ($(GIT_HTTP),y)
 OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
-QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-upstream-unstable.git
-QEMU_TRADITIONAL_URL ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
+QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-xen.git
+QEMU_TRADITIONAL_URL ?= 
http://xenbits.xen.org/git-http/qemu-xen-traditional.git
 SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
 MINIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/mini-os.git
 else
 OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
-QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-upstream-unstable.git
-QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-unstable.git
+QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-xen.git
+QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-traditional.git
 SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
 MINIOS_UPSTREAM_URL ?= git://xenbits.xen.org/mini-os.git
 endif
-- 
2.1.4


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

Re: [Xen-devel] [PATCH OSSTEST] Switch to merged qemu-xen{, -traditional}.git trees

On Thu, 2015-09-17 at 11:31 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] Switch to merged qemu-xen{,
> -traditional}.git trees"):
> > The qemu-mainline flights now push to the upstream-tested branch of
> > qemu-xen.git (while still pulling from upstream).
> 
> This mostly looks fine.
> 
> How are we going to test this ?  I think the most practical approach
> is to add a step to your deployment plan (after the new trees are in
> place, but before the osstest force push) where you explicitly ap-push
> and ap-fetch the relevant trees.

This sounds workable.

>   You could do the ap-push as a user
> without the appropriate permissions to get a dry run.

Since this would be pre-deployment it wouldn't be too hard to just let it
do it and then force a return to whatever baseline manually.
> 
> Then if this patch needs updating it can be fixed up quite easily.

> > --- a/ap-push
> > +++ b/ap-push
> ...
> > +qemu-upstream-*-testing)
> ...
> > +   # For now also push to the old split trees for historical
> > +   # branches only (qemu-upstream started with xen-4.2-testing
> > +   # and the split trees end at xen-4.6-testing)
> > +   case "$xenbranch" in
> > +   xen-4.[23456]-testing)
> > +   tree=$XENBITS:/home/xen/git/qemu-upstream
> > -${xenbranch#xen-}.git
> > +   git push $tree $revision:refs/heads/master
> 
> I think this would be clearer if not done in terms of xenbranch.  I
> would use case $branch and then $branchcore.  What do you think ?

Good idea.

Ian.


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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread H. Peter Anvin

However, the difference between one CONFIG and another is quite frankly crazy.  
We should explicitly use the safe versions where this is appropriate, and then 
yes, we should do this.

Yet another reason the paravirt code is batshit crazy.

On September 17, 2015 2:31:34 AM PDT, Borislav Petkov  wrote:
>On Thu, Sep 17, 2015 at 09:19:20AM +0200, Ingo Molnar wrote:
>> Most big distro kernels on bare metal have CONFIG_PARAVIRT=y (I
>checked Ubuntu and 
>> Fedora), so we are potentially exposing a lot of users to problems.
>
>+ SUSE.
>
>> Crashing the bootup on an unknown MSR is bad. Many MSR reads and
>writes are 
>> non-critical and returning the 'safe' result is much better than
>crashing or 
>> hanging the bootup.
>
>... and prepending all MSR accesses with feature/CPUID checks is
>probably almost
>impossible.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

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

2015-09-17 Thread Chun Yan Liu



>>> On 9/14/2015 at 10:03 PM, in message
, George
Dunlap  wrote: 
> On Mon, Sep 14, 2015 at 12:12 PM, Ian Jackson   
> wrote: 
> > Juergen Gross writes ("Re: [Xen-devel] [PATCH V6 3/7] libxl: add pvusb  
> API"): 
> >> On 09/14/2015 12:36 PM, George Dunlap wrote: 
> >> > Anyone want to look into the Linux source code to find out how big it 
> >> > will allow busnum / devnum to grow? 
> >> 
> >> drivers/usb/core/hcd.c is using a bitmap to find the next bus number 
> >> currently not in use. It's size is USB_MAXBUS which in turn has the 
> >> value 64. 
> >> 
> >> choose_devnum() in drivers/usb/core/hub.c is doing a similar job for 
> >> device numbers. Here the highest number supported is 127. 
> > 
> > We are defining an API, which shouldn't involve this kind of 
> > implementation-grobbling. 
> > 
> > At an API level, it seems that this Linux busnum is not documented to 
> > have any particular number or behaviour or range or anything.  We 
> > should use the biggest type we can use conveniently 
> > 
> > Do we need to worry that some bus might have 2^24 unplugs/plugs 
> > (perhaps in some kind of software emulation) and that we need to use a 
> > type which can hold a uint32_t or maybe even a uint64_t ? 
>  
> libusb is already a published API that supports uint8, or up to 255. 
> Following their lead seems like a reasonable thing to do.  If ever 
> that number goes above 255, basically every Linux program that touches 
> a USB device will need to be recompiled with a new version of libusb. 
>  
> Is there any reason for Linux to go above 255?  Things I can think of: 
>  
> 1. Users have more than 255 devices plugged into the same bus. 
>  
> 2. A security / confusion issue due to devnum reuse when users plug 
> and unplug devices hundreds of times. 
>  
> Both of these seem pretty unlikely. 
>  
> I would personally go with uint8, but int16 or int32 certainly won't hurt. 

So can we agree to use uint8 for hostbus and hostaddr as libusb does?

>  
>  -George 
>  
> ___ 
> 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 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Peter Zijlstra

On Wed, Sep 16, 2015 at 04:33:11PM -0700, Andy Lutomirski wrote:
> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.
> 
> This is IMO awful: it papers over bugs.  In particular, KVM gueests
> might be unwittingly depending on this behavior because
> CONFIG_KVM_GUEST currently depends on CONFIG_PARAVIRT.  I'm not
> aware of any such problems, but applying this series would be a good
> way to shake them out.
> 
> Fix it so that the MSR operations work the same on CONFIG_PARAVIRT=n
> and CONFIG_PARAVIRT=y as long as Xen isn't being used.  The Xen
> maintainers are welcome to make a similar change on top of this.
> 
> Since there's plenty of time before the next merge window, I think
> we should apply and fix anything that breaks.
> 
> Doing this is probably a prerequisite to sanely decoupling
> CONFIG_KVM_GUEST and CONFIG_PARAVIRT, which would probably make
> Arjan and the rest of the Clear Containers people happy :)

So I actually like this, although by Ingo's argument, its a tad risky.

But the far greater problem I have with the whole virt thing is that
you cannot use rdmsr_safe() to probe if an MSR exists at all because, as
you told me, these virt thingies return 0 for all 'unknown' MSRs instead
of faulting.

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

[Xen-devel] Newbie

2015-09-17 Thread Lasya Venneti

Hi everyone,

I'm Lasya a student studying in IIIT-H, Hyderabad, India.

I wish to participate in round 11 of Outreachy this time. I would be
grateful if someone would direct me as to how I am supposed to start
contributing to Xen Project for this Outreachy round.

Sincerely,
Lasya V
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Borislav Petkov

On Thu, Sep 17, 2015 at 09:19:20AM +0200, Ingo Molnar wrote:
> Most big distro kernels on bare metal have CONFIG_PARAVIRT=y (I checked 
> Ubuntu and 
> Fedora), so we are potentially exposing a lot of users to problems.

+ SUSE.

> Crashing the bootup on an unknown MSR is bad. Many MSR reads and writes are 
> non-critical and returning the 'safe' result is much better than crashing or 
> hanging the bootup.

... and prepending all MSR accesses with feature/CPUID checks is probably almost
impossible.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On 09/17/2015 10:38 AM, George Dunlap wrote:
> On 09/17/2015 09:48 AM, Dario Faggioli wrote:
>> On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:
>>
 -Original Message-
 From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
>>
 So, I guess, first of all, can you confirm whether or not it's exploding
 in debug builds?
>>>
>>> Does the following information in Config.mk mean it is a debug build?
>>>
>>> # A debug build of Xen and tools?
>>> debug ?= y
>>> debug_symbols ?= $(debug)
>>>
>> I think so. But as I said in my other email, I was wrong, and this is
>> probably not an issue.
>>
 And in either case (just tossing out ideas) would it be
 possible to deal with the "interrupt already raised when blocking" case:
>>>
>>> Thanks for the suggestions below!
>>>
>> :-)
>>
  - later in the context switching function ?
>>> In this case, we might need to set a flag in vmx_pre_ctx_switch_pi() instead
>>> of calling vcpu_unblock() directly, then when it returns to 
>>> context_switch(),
>>> we can check the flag and don't really block the vCPU. 
>>>
>> Yeah, and that would still be rather hard to understand and maintain,
>> IMO.
>>
>>> But I don't have a clear
>>> picture about how to archive this, here are some questions from me:
>>> - When we are in context_switch(), we have done the following changes to
>>> vcpu's state:
>>> * sd->curr is set to next
>>> * vCPU's running state (both prev and next ) is changed by
>>>   vcpu_runstate_change()
>>> * next->is_running is set to 1
>>> * periodic timer for prev is stopped
>>> * periodic timer for next is setup
>>> ..
>>>
>>> So what point should we perform the action to _unblock_ the vCPU? We
>>> Need to roll back the formal changes to the vCPU's state, right?
>>>
>> Mmm... not really. Not blocking prev does not mean that prev would be
>> kept running on the pCPU, and that's true for your current solution as
>> well! As you say yourself, you're already in the process of switching
>> between prev and next, at a point where it's already a thing that next
>> will be the vCPU that will run. Not blocking means that prev is
>> reinserted to the runqueue, and a new invocation to the scheduler is
>> (potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
>> __runq_tickle()), but it's only when such new scheduling happens that
>> prev will (potentially) be selected to run again.
>>
>> So, no, unless I'm fully missing your point, there wouldn't be no
>> rollback required. However, I still would like the other solution (doing
>> stuff in vcpu_block()) better (see below).
>>
  - with another hook, perhaps in vcpu_block() ?
>>>
>>> We could check this in vcpu_block(), however, the logic here is that before
>>> vCPU is blocked, we need to change the posted-interrupt descriptor,
>>> and during changing it, if 'ON' bit is set, which means VT-d hardware
>>> issues a notification event because interrupts from the assigned devices
>>> is coming, we don't need to block the vCPU and hence no need to update
>>> the PI descriptor in this case. 
>>>
>> Yep, I saw that. But could it be possible to do *everything* related to
>> blocking, including the update of the descriptor, in vcpu_block(), if no
>> interrupt have been raised yet at that time? I mean, would you, if
>> updating the descriptor in there, still get the event that allows you to
>> call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
>> the blocking, no matter whether that resulted in an actual context
>> switch already or not?
>>
>> I appreciate that this narrows the window for such an event to happen by
>> quite a bit, making the logic itself a little less useful (it makes
>> things more similar to "regular" blocking vs. event delivery, though,
>> AFAICT), but if it's correct, ad if it allows us to save the ugly
>> invocation of vcpu_unblock from context switch context, I'd give it a
>> try.
>>
>> After all, this PI thing requires actions to be taken when a vCPU is
>> scheduled or descheduled because of blocking, unblocking and
>> preemptions, and it would seem natural to me to:
>>  - deal with blockings in vcpu_block()
>>  - deal with unblockings in vcpu_wake()
>>  - deal with preemptions in context_switch()
>>
>> This does not mean being able to consolidate some of the cases (like
>> blockings and preemptions, in the current version of the code) were not
>> a nice thing... But we don't want it at all costs . :-)
> 
> So just to clarify the situation...

Er, and to clarify something else -- Technically I'm responding to Dario
here, but my mail is actually addressed to Wu Feng.  This was just a
good point to "put my oar in" to the conversation. :-)

 -George


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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:

> > -Original Message-
> > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]

> > So, I guess, first of all, can you confirm whether or not it's exploding
> > in debug builds?
> 
> Does the following information in Config.mk mean it is a debug build?
> 
> # A debug build of Xen and tools?
> debug ?= y
> debug_symbols ?= $(debug)
> 
I think so. But as I said in my other email, I was wrong, and this is
probably not an issue.

> > And in either case (just tossing out ideas) would it be
> > possible to deal with the "interrupt already raised when blocking" case:
> 
> Thanks for the suggestions below!
> 
:-)

> >  - later in the context switching function ?
> In this case, we might need to set a flag in vmx_pre_ctx_switch_pi() instead
> of calling vcpu_unblock() directly, then when it returns to context_switch(),
> we can check the flag and don't really block the vCPU. 
>
Yeah, and that would still be rather hard to understand and maintain,
IMO.

> But I don't have a clear
> picture about how to archive this, here are some questions from me:
> - When we are in context_switch(), we have done the following changes to
> vcpu's state:
>   * sd->curr is set to next
>   * vCPU's running state (both prev and next ) is changed by
> vcpu_runstate_change()
>   * next->is_running is set to 1
>   * periodic timer for prev is stopped
>   * periodic timer for next is setup
>   ..
> 
> So what point should we perform the action to _unblock_ the vCPU? We
> Need to roll back the formal changes to the vCPU's state, right?
> 
Mmm... not really. Not blocking prev does not mean that prev would be
kept running on the pCPU, and that's true for your current solution as
well! As you say yourself, you're already in the process of switching
between prev and next, at a point where it's already a thing that next
will be the vCPU that will run. Not blocking means that prev is
reinserted to the runqueue, and a new invocation to the scheduler is
(potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
__runq_tickle()), but it's only when such new scheduling happens that
prev will (potentially) be selected to run again.

So, no, unless I'm fully missing your point, there wouldn't be no
rollback required. However, I still would like the other solution (doing
stuff in vcpu_block()) better (see below).

> >  - with another hook, perhaps in vcpu_block() ?
> 
> We could check this in vcpu_block(), however, the logic here is that before
> vCPU is blocked, we need to change the posted-interrupt descriptor,
> and during changing it, if 'ON' bit is set, which means VT-d hardware
> issues a notification event because interrupts from the assigned devices
> is coming, we don't need to block the vCPU and hence no need to update
> the PI descriptor in this case. 
>
Yep, I saw that. But could it be possible to do *everything* related to
blocking, including the update of the descriptor, in vcpu_block(), if no
interrupt have been raised yet at that time? I mean, would you, if
updating the descriptor in there, still get the event that allows you to
call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
the blocking, no matter whether that resulted in an actual context
switch already or not?

I appreciate that this narrows the window for such an event to happen by
quite a bit, making the logic itself a little less useful (it makes
things more similar to "regular" blocking vs. event delivery, though,
AFAICT), but if it's correct, ad if it allows us to save the ugly
invocation of vcpu_unblock from context switch context, I'd give it a
try.

After all, this PI thing requires actions to be taken when a vCPU is
scheduled or descheduled because of blocking, unblocking and
preemptions, and it would seem natural to me to:
 - deal with blockings in vcpu_block()
 - deal with unblockings in vcpu_wake()
 - deal with preemptions in context_switch()

This does not mean being able to consolidate some of the cases (like
blockings and preemptions, in the current version of the code) were not
a nice thing... But we don't want it at all costs . :-)

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

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

2015-09-17 Thread Julien Grall




On 16/09/2015 14:47, Ian Jackson wrote:

Julien Grall writes ("Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB 
Flush for ATS Device"):

On 16/09/15 11:46, Ian Jackson wrote:

JOOI why did you CC me ?  I did a quick scan of these patches and they
don't seem to have any tools impact.  I would prefer not to be CC'd
unless there is a reason why my attention would be valueable.


The common directory is maintained by "THE REST" group. From the
MAINTAINERS file you are part of it.


Ah.  Hmmm.  You mean xen/common/ ?


Right.



I don't consider myself qualified to review that.  I think the
MAINTAINERS file should have an entry for xen/ but it doesn't seem to.


I think a such patch should come from you.

Regards,

--
Julien Grall

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

[Xen-devel] [PATCH OSSTEST] Switch to merged qemu-xen{, -traditional}.git trees

The qemu-mainline flights now push to the upstream-tested branch of
qemu-xen.git (while still pulling from upstream).

The qemu-upstream-unstable flights pull from staging and push to
master in qemu-xen.git

All of the qemu-upstream-X.Y-testing pull from staging-X.Y and push to
stable-X.Y branch in qemu-xen.git.

For now we also continue pushing to the old trees for qemu-upstream
4.2 through to 4.6-testing. Once those branches have updated their
Config.mk and done a point release we can consider removing these.

Abolish $LOCALREV_QEMU_MAINLINE in favour of $LOCALREV_QEMU_UPSTREAM,
it was used inconsistently.

While changing things ensure all pushes are done to refs/heads/$thing
to avoid issues when output branches to not exist.

Signed-off-by: Ian Campbell 
---
 ap-common| 10 +++---
 ap-fetch-version | 10 --
 ap-fetch-version-old | 14 +-
 ap-push  | 25 -
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/ap-common b/ap-common
index dfeab9e..2cba7f8 100644
--- a/ap-common
+++ b/ap-common
@@ -26,9 +26,7 @@
 : ${TREE_XEN:=git://xenbits.xen.org/xen.git}
 : ${PUSH_TREE_XEN:=$XENBITS:/home/xen/git/xen.git}
 
-#: ${TREE_QEMU:=git://mariner.uk.xensource.com/qemu-$xenbranch.git}
-: ${TREE_QEMU:=git://xenbits.xen.org/staging/qemu-$xenbranch.git}
-
+: ${TREE_QEMU:=git://xenbits.xen.org/qemu-xen-traditional.git}
 
 : ${GIT_KERNEL_ORG:=git://git.kernel.org}
 : ${KERNEL_SCM:=${GIT_KERNEL_ORG}/pub/scm/linux/kernel/git}
@@ -87,13 +85,11 @@ fi
 
 : ${TREEBASE_LINUX_XCP:=http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27}
 
-: 
${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/staging/qemu-upstream-${xenbranch#xen-}.git}
+: ${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/qemu-xen.git}
+: ${PUSH_TREE_QEMU_UPSTREAM=$XENBITS:/home/xen/git/qemu-xen.git
 : ${LOCALREV_QEMU_UPSTREAM:=daily-cron.$branch}
 
 : ${TREE_QEMU_MAINLINE:=git://git.qemu.org/qemu.git}
-: ${BASE_TREE_QEMU_MAINLINE:=git://xenbits.xen.org/osstest/qemu.git}
-: ${PUSH_TREE_QEMU_MAINLINE:=$XENBITS:/home/xen/git/osstest/qemu.git}
-: ${LOCALREV_QEMU_MAINLINE:=daily-cron.$branch}
 
 info_linux_tree () {
case $1 in
diff --git a/ap-fetch-version b/ap-fetch-version
index 8b41acf..754a398 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -53,9 +53,15 @@ qemu-mainline)
repo_tree_rev_fetch_git $branch \
$TREE_QEMU_MAINLINE master $LOCALREV_QEMU_UPSTREAM
;;
-qemu-upstream-*)
+qemu-upstream-unstable)
 repo_tree_rev_fetch_git $branch \
-   $TREE_QEMU_UPSTREAM master $LOCALREV_QEMU_UPSTREAM
+   $TREE_QEMU_UPSTREAM staging $LOCALREV_QEMU_UPSTREAM
+;;
+qemu-upstream-*-testing)
+   branchcore=${branch#qemu-upstream-}
+   branchcore=${branchcore%-testing}
+repo_tree_rev_fetch_git $branch \
+   $TREE_QEMU_UPSTREAM staging-$branchcore $LOCALREV_QEMU_UPSTREAM
 ;;
 linux)
repo_tree_rev_fetch_git linux \
diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 0b4739b..464d1ab 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -32,8 +32,6 @@ select_xenbranch
 : ${BASE_LOCALREV_SEABIOS:=daily-cron.$branch.old}
 : ${BASE_LOCALREV_OVMF:=daily-cron.$branch.old}
 
-: ${BASE_TREE_QEMU_UPSTREAM:=${TREE_QEMU_UPSTREAM/\/staging\//\/}}
-
 if info_linux_tree "$branch"; then
repo_tree_rev_fetch_git linux \
$BASE_TREE_LINUX_THIS $BASE_TAG_LINUX_THIS $BASE_LOCALREV_LINUX
@@ -59,11 +57,17 @@ xen-4.*-testing)
;;
 qemu-mainline)
 repo_tree_rev_fetch_git $branch \
-   $BASE_TREE_QEMU_MAINLINE mainline/xen-tested-master 
$LOCALREV_QEMU_MAINLINE
+   $TREE_QEMU_UPSTREAM upstream-tested $LOCALREV_QEMU_UPSTREAM
+;;
+qemu-upstream-unstable)
+repo_tree_rev_fetch_git $branch \
+   $TREE_QEMU_UPSTREAM master $LOCALREV_QEMU_UPSTREAM
 ;;
-qemu-upstream-*)
+qemu-upstream-*-testing)
+   branchcore=${branch#qemu-upstream-}
+   branchcore=${branchcore%-testing}
 repo_tree_rev_fetch_git $branch \
-   $BASE_TREE_QEMU_UPSTREAM master $LOCALREV_QEMU_UPSTREAM
+   $TREE_QEMU_UPSTREAM stable-$branchcore $LOCALREV_QEMU_UPSTREAM
 ;;
 linux)
repo_tree_rev_fetch_git linux \
diff --git a/ap-push b/ap-push
index 5967b42..c3bc814 100755
--- a/ap-push
+++ b/ap-push
@@ -30,8 +30,7 @@ select_xenbranch
 . ap-common
 
 TREE_LINUX=$PUSH_TREE_LINUX
-TREE_QEMU_MAINLINE=$PUSH_TREE_QEMU_MAINLINE
-TREE_QEMU_UPSTREAM=$XENBITS:/home/xen/git/qemu-upstream-${xenbranch#xen-}.git
+TREE_QEMU_UPSTREAM=$PUSH_TREE_QEMU_UPSTREAM
 TREE_XEN=$PUSH_TREE_XEN
 TREE_LIBVIRT=$PUSH_TREE_LIBVIRT
 TREE_RUMPUSERXEN=$PUSH_TREE_RUMPUSERXEN
@@ -63,11 +62,27 @@ xen-*-testing)
;;
 qemu-mainline)
cd $repos/qemu-mainline
-   git push $TREE_QEMU_MAINLINE 
$revision:refs/heads/mainline/xen-tested-master
+   git push $TREE_QEMU_UPSTREAM

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On 09/17/2015 09:48 AM, Dario Faggioli wrote:
> On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:
> 
>>> -Original Message-
>>> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> 
>>> So, I guess, first of all, can you confirm whether or not it's exploding
>>> in debug builds?
>>
>> Does the following information in Config.mk mean it is a debug build?
>>
>> # A debug build of Xen and tools?
>> debug ?= y
>> debug_symbols ?= $(debug)
>>
> I think so. But as I said in my other email, I was wrong, and this is
> probably not an issue.
> 
>>> And in either case (just tossing out ideas) would it be
>>> possible to deal with the "interrupt already raised when blocking" case:
>>
>> Thanks for the suggestions below!
>>
> :-)
> 
>>>  - later in the context switching function ?
>> In this case, we might need to set a flag in vmx_pre_ctx_switch_pi() instead
>> of calling vcpu_unblock() directly, then when it returns to context_switch(),
>> we can check the flag and don't really block the vCPU. 
>>
> Yeah, and that would still be rather hard to understand and maintain,
> IMO.
> 
>> But I don't have a clear
>> picture about how to archive this, here are some questions from me:
>> - When we are in context_switch(), we have done the following changes to
>> vcpu's state:
>>  * sd->curr is set to next
>>  * vCPU's running state (both prev and next ) is changed by
>>vcpu_runstate_change()
>>  * next->is_running is set to 1
>>  * periodic timer for prev is stopped
>>  * periodic timer for next is setup
>>  ..
>>
>> So what point should we perform the action to _unblock_ the vCPU? We
>> Need to roll back the formal changes to the vCPU's state, right?
>>
> Mmm... not really. Not blocking prev does not mean that prev would be
> kept running on the pCPU, and that's true for your current solution as
> well! As you say yourself, you're already in the process of switching
> between prev and next, at a point where it's already a thing that next
> will be the vCPU that will run. Not blocking means that prev is
> reinserted to the runqueue, and a new invocation to the scheduler is
> (potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
> __runq_tickle()), but it's only when such new scheduling happens that
> prev will (potentially) be selected to run again.
> 
> So, no, unless I'm fully missing your point, there wouldn't be no
> rollback required. However, I still would like the other solution (doing
> stuff in vcpu_block()) better (see below).
> 
>>>  - with another hook, perhaps in vcpu_block() ?
>>
>> We could check this in vcpu_block(), however, the logic here is that before
>> vCPU is blocked, we need to change the posted-interrupt descriptor,
>> and during changing it, if 'ON' bit is set, which means VT-d hardware
>> issues a notification event because interrupts from the assigned devices
>> is coming, we don't need to block the vCPU and hence no need to update
>> the PI descriptor in this case. 
>>
> Yep, I saw that. But could it be possible to do *everything* related to
> blocking, including the update of the descriptor, in vcpu_block(), if no
> interrupt have been raised yet at that time? I mean, would you, if
> updating the descriptor in there, still get the event that allows you to
> call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
> the blocking, no matter whether that resulted in an actual context
> switch already or not?
> 
> I appreciate that this narrows the window for such an event to happen by
> quite a bit, making the logic itself a little less useful (it makes
> things more similar to "regular" blocking vs. event delivery, though,
> AFAICT), but if it's correct, ad if it allows us to save the ugly
> invocation of vcpu_unblock from context switch context, I'd give it a
> try.
> 
> After all, this PI thing requires actions to be taken when a vCPU is
> scheduled or descheduled because of blocking, unblocking and
> preemptions, and it would seem natural to me to:
>  - deal with blockings in vcpu_block()
>  - deal with unblockings in vcpu_wake()
>  - deal with preemptions in context_switch()
> 
> This does not mean being able to consolidate some of the cases (like
> blockings and preemptions, in the current version of the code) were not
> a nice thing... But we don't want it at all costs . :-)

So just to clarify the situation...

If a vcpu configured for the "running" state (i.e., NV set to
"posted_intr_vector", notifications enabled), and an interrupt happens
in the hypervisor -- what happens?

Is it the case that the interrupt is not actually delivered to the
processor, but that the pending bit will be set in the pi field, so that
the interrupt will be delivered the next time the hypervisor returns
into the guest?

(I am assuming that is the case, because if the hypervisor *does* get an
interrupt, then it can just unblock it there.)

This sort of race condition -- where we get an interrupt to wake up a
vcpu as we're blocking -- is

[Xen-devel] [PATCH XEN] Config: Switch to unified qemu trees.

Upstream qemu is not in qemu-xen.git and the trad fork is in
qemu-xen-traditional.h

QEMU_UPSTREAM_REVISION is currently a tag and
QEMU_TRADITIONAL_REVISION is a specific revision, so no changes are
required to those.

Signed-off-by: Ian Campbell 
(cherry picked from commit dff5c395c1d23c21238ce17ddcd6f7abe2efd08d)

Conflicts:
Config.mk
---
 Config.mk | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Config.mk b/Config.mk
index 9936090..1b559b0 100644
--- a/Config.mk
+++ b/Config.mk
@@ -242,13 +242,13 @@ endif
 
 ifeq ($(GIT_HTTP),y)
 OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
-QEMU_UPSTREAM_URL ?= 
http://xenbits.xen.org/git-http/qemu-upstream-4.5-testing.git
-QEMU_TRADITIONAL_URL ?= 
http://xenbits.xen.org/git-http/qemu-xen-4.5-testing.git
+QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-xen.git
+QEMU_TRADITIONAL_URL ?= 
http://xenbits.xen.org/git-http/qemu-xen-traditional.git
 SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
 else
 OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
-QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-upstream-4.5-testing.git
-QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-4.5-testing.git
+QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-xen.git
+QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-traditional.git
 SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
 endif
 OVMF_UPSTREAM_REVISION ?= cb9a7ebabcd6b8a49dc0854b2f9592d732b5afbd
-- 
2.1.4


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

[Xen-devel] [PATCH OSSTEST+XEN 0/1+1] Switching to one qemu tree per qemu version (trad vs upstream)

As discussed[0] I think we should move away from having a pair of qemu
repositories for each Xen branch and instead have a single pair (one for
qemu-xen and one for qemu-xen-traditional).

I've prepared a pair of repositories:

http://xenbits.xen.org/gitweb/?p=people/ianc/single-qemu-repo/qemu-xen-traditional.git;a=summary
http://xenbits.xen.org/gitweb/?p=people/ianc/single-qemu-repo/qemu-xen.git;a=summary

These will eventually become, respectively:
git://xenbits.xen.org/qemu-xen-traditional.git
git://xenbits.xen.org/qemu-xen.git

Note that qemu-xen-traditional does not have an osstest gate, it is gated
by changes to Config.mk in xen.git pulling in the specific revision.

In addition to the scheme described in [0] the qemu-mainline test output
gate changes from:
git://xenbits.xen.org/osstest/qemu.git#mainline/xen-tested-master
to
git://xenbits.xen.org/qemu-xen.git#upstream-tested

Thereby sharing a tree with our upstream branches, which I think makes
sense. The input remains git://git.qemu.org/qemu.git#master, of course.

I will post two patches in reply to this, the first is for xen.git to make
the obvious change to Config.mk.

The second is for osstest to change it to fetch from these new trees and
push to them, and in addition push to the old trees for existing stable
branches only (including 4.6).

I believe the plan for deployment should be:

* Today we can already just remove the old staging/qemu-xen-* trees, they
are unused (apart from being manually pushed to along with the staging
trees, I think).
* Move the two new trees out of people/ianc into the correct places.
Ensure git-http etc all works and that Stefano + IanJ have appropriate
permissions.
* Push the osstest patch (possibly consider a force push? An osstest
flight doesn't actually exercise this and it would make coordination
with the next step easier...)
* Once that change is in osstest.git#production Stefano and Ian would
switch to pushing to the appropriate staging* branches new trees'.
osstest will ignore the old staging trees and osstest will update both
the new master/stable branches and the old stable trees (but not the old
qemu-upstream-unstable#master branch).
* ASAP after the osstest patch reaches production push the patch to
xen.git#staging. This will cause the xen-unstable builds to use the new
output gate. Until this is done unstable builds will continue using the
old master push gate, which is not updated after the osstest update
(only stable branches are), this isn't a big deal but obviously a
smaller window would be better.
* At this point we could remove the old staging/qemu-upstream-* trees,
they are not referenced by anything.
* At our leisure backport the xen.git change to stable branches, probably
back as far as 4.2 (when qemu-xen was introduced).
* Do stable releases of each of the above.
* Drop (one by one or all at once) the push to the legacy stable branches
from osstest as stable releases are made referencing the new trees.
* Consider hiding (or removing) the old output trees from xenbits as well.

I don't think this has any impact on the 4.6 release, apart from the point
where Stefano+Ian start pushing to the new trees, so we could consider
starting whenever we like.

I don't propose that we try and do the backport of the Config.mk change to
4.6 for 4.6.0, it can wait for 4.6.1.

Ian.

[0] http://mid.gmane.org/<1438266156.11600.347.ca...@citrix.com>

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

[Xen-devel] [xen-4.5-testing test] 62022: regressions - FAIL

2015-09-17 Thread osstest service owner

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-vhd9 debian-di-install fail REGR. vs. 61513

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-vhd 14 guest-saverestore.2 fail in 61844 pass in 62022
 test-armhf-armhf-xl-arndale   9 debian-install fail in 61844 pass in 62022
 test-amd64-i386-xl-qemuu-winxpsp3 13 guest-localmigrate fail in 61844 pass in 
62022
 test-amd64-i386-rumpuserxen-i386 10 guest-start fail pass in 61844
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-localmigrate  fail pass in 61844
 test-amd64-amd64-xl-qemuu-winxpsp3 15 guest-localmigrate.2  fail pass in 61844

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-libvirt-vhd  9 debian-di-install fail REGR. vs. 61513
 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail blocked in 61513
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 61844 
like 61513
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 61844 
like 61513
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail in 61844 like 61513
 test-amd64-i386-libvirt-raw   9 debian-di-installfail   like 61513
 test-amd64-amd64-libvirt-raw  9 debian-di-installfail   like 61513
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail like 61513
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 61513

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-migrupgrade  1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-amd64-amd64-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 build-amd64-prev  5 xen-buildfail   never pass
 build-i386-prev   5 xen-buildfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   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-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 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-i386-libvirt-vhd  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-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  ffb4e6387f489b6b5ce287f51db43cb37ebae064
baseline version:
 xen  ef89dc8c00087c8c1819e60bdad5527db70425e1

Last test of basis61513  2015-09-07 11:42:18 Z9 days
Failing since 61751  2015-09-10 14:07:54 Z6 days3 attempts
Testing same since61844  2015-09-12 15:58:20 Z4 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anshul Makkar 
  Aravind Gopalakrishnan 
  Ian Campbell 
  Ian Jackson

Re: [Xen-devel] [RFC PATCH 0/5] Load BIOS via toolstack instead of been embedded in hvmloader.

2015-09-17 Thread Anthony PERARD

On Wed, Sep 16, 2015 at 07:56:44PM +0100, Andrew Cooper wrote:
> On 16/09/2015 18:19, Anthony PERARD wrote:
> > Hi all,
> >
> > I've start to look at loading the BIOS and the ACPI tables via the
> > toolstack instead of having them embedded in the hvmloader binary. This is
> > done by using the same mechanics as the one used to load extra ACPI tables
> > or SMBIOS.
> >
> > - libxl load the blob into it's memory and add it to
> >   struct xc_hvm_build_args.bios_module
> > - libxc load the blob into the guest memory and give back the guest address
> >   of the blob.
> > - libxl store the location of the blob (and it's lenght) into xenstore, in
> >   /local/domain/$domid/hvmloader/$blob_name/{address,length} ($blob_name
> >   would be "bios" or "acpi_table"; there is already "acpi" and "smbios"
> >   that exist)
> > - hvmloader read the addresses from xenstore and put the blob at the right
> >   place.
> >
> > How this is looking?
> >
> > Right now, this patch series would only work for SeaBIOS.
> 
> I highly recommend that you build on top of Rogers DMlite series, which
> already offers a multiboot-style way of adding extra modules to HVM
> guests.  (That was the way I was planning to do this in some copious
> free time).

Thanks. I will look into this.

> In particular, storing the address/length in xenstore is conceptually
> incorrect as the information turns stale as soon as hvmloader starts
> running.
> 
> * Modify hvmloader to have a DMLite entry, which gives it a command line
> and module list
> * Modify libxc to be able to take an arbitrary quantity of modules,
> rather than the currently limit of 1
> 
> Once the above work times are in place, the actual splitting-up of
> hvmloader can occur.
> 
> I recommend a command line way of telling hvmloader which module is
> what, such as a legacy bios image, (other firmware image?), supplemental
> acpi table, supplemental smbios table, nvram blob?  (This last one will
> require a bit of extra work so the toolstack can pull the blob back out
> of the guest on destruction)

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

2015-09-17 Thread Tim Deegan

At 12:53 +0100 on 17 Sep (1442494439), Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed
> (as I do not normally review hypervisor patches) and Andrew Cooper's
> added (which seems appropriate given his status as x86 hypervisor
> maintainer).
> 
> The effect is that patches touching xen/common/ will no longer be CC'd
> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
> patches touching xen/common/, not just ones which match one of their
> existing more specific entries.

This has the same problem as the previous patch, on a smaller
scale.  There's no need for us all to be CC'd on code that already has
a maintainer (e.g. scheduler, xentrace, device-tree, tmem...)

I would rather your v1 plus an appropriate change to get_maintainers.

Cheers,

Tim.

> Signed-off-by: Ian Jackson 
> CC: Ian Campbell 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Julien Grall 
> CC: David Vrabel 
> CC: Andrew Cooper 
> 
> ---
> v2: A completely different approach following discussion with Ian C.
> ---
>  MAINTAINERS |   10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..e35c1e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +HYPERVISOR CORE
> +M:   Jan Beulich 
> +M:   Tim Deegan 
> +M:   Keir Fraser 
> +M:   Ian Campbell 
> +M:   Andrew Cooper 
> +L:   xen-devel@lists.xen.org
> +S:   Supported
> +F:   xen/common/
> +
>  THE REST
>  M:   Ian Campbell 
>  M:   Ian Jackson 
> -- 
> 1.7.10.4
> 

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Peter Zijlstra

On Thu, Sep 17, 2015 at 01:40:30PM +0200, Paolo Bonzini wrote:
> 
> 
> On 17/09/2015 10:58, Peter Zijlstra wrote:
> > But the far greater problem I have with the whole virt thing is that
> > you cannot use rdmsr_safe() to probe if an MSR exists at all because, as
> > you told me, these virt thingies return 0 for all 'unknown' MSRs instead
> > of faulting.
> 
> At least for KVM, that behavior is opt-in (the ignore_msrs parameter)
> and no distro that I know enables it by default.

Ah, that would be good news. Andy earlier argued I could not rely on
rdmsr_safe() faulting on unknown MSRs. If practically we can there's
some code I can simplify :-)

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

[Xen-devel] [xen-unstable baseline-only test] 37938: tolerable FAIL

2015-09-17 Thread Platform Team regression test user

This run is configured for baseline tests only.

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail REGR. vs. 37590

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  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-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail 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-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
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-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-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-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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  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-raw  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail never 
pass
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-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-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail 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:
 xen  a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
baseline version:
 xen  e13013dbf1d5997915548a3b5f1c39594d8c1d7b

Last test of basis37590  2015-05-26 15:29:41 Z  113 days
Testing same since37938  2015-09-15 16:29:30 Z1 days1 attempts


People who touched revisions under test:
  Alan Robinson 
  Andres Lagar-Cavilla 
  Andrew Cooper 
  Andrew Cooper  for the x86 bits.
  Anshul Makkar 
  Anthony PERARD 
  Aravind Gopalakrishnan 
  Ard Biesheuvel 
  Ben Catterall 
  Boris Ostrovsky 
  Chao Peng 
  Charles Arnold 
  Chen Baozi 
  Chris (Christopher) Brand 
  Chris Brand 
  Daniel De Graaf

Re: [Xen-devel] [xen-4.6-testing test] 62015: regressions - FAIL

On Thu, Sep 17, 2015 at 06:44:05AM +, osstest service owner wrote:
> flight 62015 xen-4.6-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/62015/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install 
> fail REGR. vs. 61745
> 

I think this test case is unreliable.  For example, it passed in 61745
(xen-4.6-staging) but failed in 62012 (linux-3.14) while both had the
same version of Xen.

The history of this test case shows more failures than successes. It's
failing in all branches.

Nonetheless, to justify a force push, adequate analysis is required.

The list of commits between stable-4.6 and staging-4.6:

d892e956 x86/MSI: fail if no hardware support
70d63e48 x86/p2m: fix mismatched unlock
11d942df vtd/iommu: permit group devices to passthrough in relaxed mode

Irrelevant to failure.

da24ee8f xl: handle empty vnuma configuration
ed9926ec xl/libxl: disallow saving a guest with vNUMA configured
13f44615 libxc: introduce xc_domain_getvnuma
607dfbb1 libxl: format fd flags with 0x since they are hex.

Irrelevant to failure.

93a6ff88 x86/hvm: fix saved pmtimer and hpet values

This commit touches hpet_save function, which is only used to save HVM
guest. This is not related to failure because the failing test step is
debian-hvm-install which doesn't involve saving.

And the code it touches it not specific to stubdom. Other test cases
which involve saving hvm guest confirmed this change is either correct
or irrelevant (because the code path is not taken).

45761470 libxl: clear O_NONBLOCK|O_NDELAY on migration fd and reinstate 
afterwards

Tested in save / restore test case. Irrelevant to failure.

0b6d907c QEMU_TAG update
b0c5ff5e libxl: set ret to non-zero value in failure path
a4cebfd1 configure: don't silently disable systemd support
cb5599f7 Config.mk: Non-debug build by default.
33ad644b x86/VPMU: Set VPMU context pointer to NULL when freeing it
e7e711e4 efi: introduce efi_arch_flush_dcache_area
5af5c1ad Branch for 4.6: Document stable tree MAINTAINERS
ddd9d77e MAINTAINERS: stable backports should be requested on xen-devel
8d7fee3a Branch for 4.6: Update QEMU references

Irrelevant to failure.

I think a force push is justified.

Wei.

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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On Thu, 2015-09-17 at 12:44 +0100, George Dunlap wrote:
> On 09/17/2015 10:38 AM, George Dunlap wrote:
> > Is it the case that the interrupt is not actually delivered to the
> > processor, but that the pending bit will be set in the pi field, so that
> > the interrupt will be delivered the next time the hypervisor returns
> > into the guest?
> > 
> > (I am assuming that is the case, because if the hypervisor *does* get an
> > interrupt, then it can just unblock it there.)
> 
> Actually, it looks like you *do* in fact get a
> pi_notification_interrupt() in this case.  Could we to check to see if
> the current vcpu is blocked and unblock it?
> 
> I haven't yet decided whether I prefer my original suggestion of
> switching the interrupt and putting things on the wake-up list in
> vcpu_block(), or of deferring adding things to the wake-up list until
> the actual context switch.
> 
Sorry but I don't get what you mean with the latter.

I particular, I don't think I understand what you mean with and how it
would work to "defer[ring] adding things to the wake-up list until
actual context switch"... In what case would you defer stuff to context
switch?

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

Re: [Xen-devel] [xen-4.6-testing test] 62015: regressions - FAIL

On Thu, 2015-09-17 at 13:40 +0100, Wei Liu wrote:
> On Thu, Sep 17, 2015 at 06:44:05AM +, osstest service owner wrote:
> > flight 62015 xen-4.6-testing real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/62015/
> > 
> > Regressions :-(
> > 
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> >  test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm
> > -install fail REGR. vs. 61745
> > 
> 
> I think this test case is unreliable.  For example, it passed in 61745
> (xen-4.6-staging) but failed in 62012 (linux-3.14) while both had the
> same version of Xen.

This has failed twice in a row on xen-4.6-testing, in 61839 and here. The
bisector already had a go at the initial failure, but concluded
unreproducible as shown by the yellow box in

http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-4.6-testing/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm.debian-hvm-install.html

> The history of this test case shows more failures than successes. It's
> failing in all branches.

Looking at each branch individually under http://logs.test-lab.xenproject.o
rg/osstest/results/history/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64
-xsm/ it looks to me like it might be specific to a class of machines or
something like that.

> I think a force push is justified.

I think I agree.

Ian.

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Peter Zijlstra

On Thu, Sep 17, 2015 at 08:17:18AM -0700, Andy Lutomirski wrote:

> > Ah, that would be good news. Andy earlier argued I could not rely on
> > rdmsr_safe() faulting on unknown MSRs. If practically we can there's
> > some code I can simplify :-)
> 
> I was taking about QEMU TCG, not KVM.

Just for my education, TCG is the thing without _any_ hardware assist?
The thing you fear to use because it cannot boot a kernel this side of
tomorrow etc.. ?

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Andy Lutomirski

On Thu, Sep 17, 2015 at 8:17 AM, Peter Zijlstra  wrote:
> On Thu, Sep 17, 2015 at 08:17:18AM -0700, Andy Lutomirski wrote:
>
>> > Ah, that would be good news. Andy earlier argued I could not rely on
>> > rdmsr_safe() faulting on unknown MSRs. If practically we can there's
>> > some code I can simplify :-)
>>
>> I was taking about QEMU TCG, not KVM.
>
> Just for my education, TCG is the thing without _any_ hardware assist?

Yes.

> The thing you fear to use because it cannot boot a kernel this side of
> tomorrow etc.. ?

I actually use it on a semi-regular basis.  It appears to boot a
normal kernel correctly and surprisingly quickly.  It's important for
a silly reason.  Asking KVM on an Intel host to emulate an AMD CPU or
vice versa results in a chimera: I get an Opteron that's
"GenuineIntel", which causes Linux to treat it as Intel, which means
it gets the Intel quirks (SYSENTER in long mode, no
X86_BUG_SYSRET_SS_ATTRS, etc) instead of the AMD quirks (SYSCALL in
compat mode, etc).  So, if I want to test compat SYSCALL, I have to
use TCG, which will actually do a decent job of emulating an AMD CPU
for me.

Maybe Paolo can fix QEMU to fail bad MSR accesses for real...

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

Re: [Xen-devel] [PATCH v5 2/9] x86/intel_pstate: APERF/MPERF feature detect

On 14/09/15 03:32, Wei Wang wrote:
> Add support to detect the APERF/MPERF feature. Also, remove the identical
> code in cpufreq.c and powernow.c. This patch is independent of the
> earlier patches.
>
> Signed-off-by: Wei Wang 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH OSSTEST 3/4] ms-queuedaemon: Add report-projection

No semantic change. Eventually more tasks will be added which are
wanted in report-projection but not report-plan.

Signed-off-by: Ian Campbell 
---
 ms-queuedaemon | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ms-queuedaemon b/ms-queuedaemon
index f3f85bd..9c7645d 100755
--- a/ms-queuedaemon
+++ b/ms-queuedaemon
@@ -187,12 +187,12 @@ proc runneeded-perhaps-start {} {
 proc queuerun-finished/plan {} {
 runneeded-ensure-will 0
 report-plan plan plan
-report-plan plan projection
+report-projection plan
 }
 
 proc queuerun-finished/projection {} {
 runneeded-ensure-will 0
-report-plan projection projection
+report-projection projection
 }
 
 proc runneeded-ensure-polling {} {
@@ -309,6 +309,10 @@ proc report-plan {w wo} {
 }
 }
 
+proc report-projection {w} {
+report-plan $w projection
+}
+
 proc we-are-thinking {chan} {
 set ws {}
 foreach-walker w {
-- 
2.5.1


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

Re: [Xen-devel] [PATCH 1/4] x86/EPT: always return proper order value from ept_get_entry()

On 15/09/15 08:30, Jan Beulich wrote:
> This is so that callers can determine what range of address space would
> get altered by a corresponding "set".
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH OSSTEST 4/4] ms-queuedaemon: Call ms-flights-summary upon completed plan/projection

Signed-off-by: Ian Campbell 
---
 ms-queuedaemon | 13 +
 1 file changed, 13 insertions(+)

diff --git a/ms-queuedaemon b/ms-queuedaemon
index 9c7645d..6ae9677 100755
--- a/ms-queuedaemon
+++ b/ms-queuedaemon
@@ -310,7 +310,20 @@ proc report-plan {w wo} {
 }
 
 proc report-projection {w} {
+global c
+
 report-plan $w projection
+
+# report-plan will have ensured data-projection.final.pl is up to
+# date
+
+catching-internally "producing summary" {
+   set proj "data-projection.final.pl"
+   set outputfile "$c(WebspaceFile)/summary.html"
+   exec ./ms-flights-summary $proj > $outputfile
+} {
+   log "$w report-projection OK"
+}
 }
 
 proc we-are-thinking {chan} {
-- 
2.5.1


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

Re: [Xen-devel] [PATCH OSSTEST v4] ms-flights-summary: Produce an HTML report of all active flights

On Wed, 2015-09-16 at 14:07 +0100, Ian Campbell wrote:
> TODO: Hook up to ms-queuedaemon to run when a new projection is
> complete.

I will followup to this with 4 patches (including 2 from you) which do
this.

They are currently live in the Cambridge daemons-testing.git and
successfully producing http://osstest.xs.citrite.net/~osstest/summary.html 

Ian.

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

[Xen-devel] [PATCH OSSTEST 2/4] ms-queuedaemon: Break out catching-internally

From: Ian Jackson 

No functional change.

Signed-off-by: Ian Jackson 
---
 ms-queuedaemon | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/ms-queuedaemon b/ms-queuedaemon
index 1a31284..f3f85bd 100755
--- a/ms-queuedaemon
+++ b/ms-queuedaemon
@@ -39,6 +39,16 @@ proc foreach-walker {walkervar body} {
 }
 }
 
+proc catching-internally {what try {ifok {}}} {
+if {[catch {
+   uplevel 1 $try
+} emsg]} {
+log "INTERNAL ERROR $what: $emsg"
+} else {
+   uplevel 1 $ifok
+}
+}
+
 proc chan-destroy-stuff {chan} {
 dequeue-chan $chan destroy
 upvar #0 chan-info/$chan info
@@ -288,12 +298,10 @@ proc queuerun-perhaps-step {w} {
 
 proc report-plan {w wo} {
 global c
-if {[catch {
+catching-internally "showing $w html" {
set outputfile "$c(WebspaceFile)/resource-$wo.html"
exec ./ms-planner -w$w show-html > $outputfile
-} emsg]} {
-log "INTERNAL ERROR showing $w html: $emsg"
-} else {
+} {
set out data-$wo.final.pl
file copy -force data-$w.pl $out.new
file rename -force $out.new $out
@@ -486,10 +494,8 @@ proc restarter-restart-now {} {
log-event "restarter-restart-now projection-running"
 }
 
-if {[catch {
+catching-internally "setting unprocessed" {
chans-note-unprocessed plan [set plan/queue_running]
-} emsg]} {
-   log "INTERNAL ERROR setting unprocessed: $emsg"
 }
 report-plan plan plan
 
-- 
2.5.1


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

[Xen-devel] [PATCH for-4.6] HACK: Use my local trees

---
 .config | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.config b/.config
index 8258faa..9b808a2 100644
--- a/.config
+++ b/.config
@@ -1,5 +1,5 @@
-MINIOS_UPSTREAM_URL := git://xenbits.xen.org/people/ianc/mini-os.git
-MINIOS_UPSTREAM_REVISION := 134bad3e7d1ab2c93019813f607cc703ccef4661
+MINIOS_UPSTREAM_URL := /home/ianc/devel/libxenctrl-split/mini-os.git
+MINIOS_UPSTREAM_REVISION := master
 
-QEMU_TRADITIONAL_URL := git://xenbits.xen.org/people/ianc/qemu-xen-unstable.git
-QEMU_TRADITIONAL_REVISION := e06f1ca27b11fe122edb025919d4e69800eca6a9
+QEMU_TRADITIONAL_URL := 
/home/ianc/devel/libxenctrl-split/qemu-xen-traditional.git
+QEMU_UPSTREAM_URL := /home/ianc/devel/libxenctrl-split/qemu-xen.git
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

On Thu, 2015-09-17 at 15:30 +0100, George Dunlap wrote:
> On 09/17/2015 01:40 PM, Dario Faggioli wrote:

> >> I haven't yet decided whether I prefer my original suggestion of
> >> switching the interrupt and putting things on the wake-up list in
> >> vcpu_block(), or of deferring adding things to the wake-up list until
> >> the actual context switch.
> >>
> > Sorry but I don't get what you mean with the latter.
> > 
> > I particular, I don't think I understand what you mean with and how it
> > would work to "defer[ring] adding things to the wake-up list until
> > actual context switch"... In what case would you defer stuff to context
> > switch?
> 
> So one option is to do the "blocking" stuff in an arch-specific call
> from vcpu_block():
> 
> vcpu_block()
>  set(_VPF_blocked)
>  v->arch.block()
>   - Add v to pcpu.pi_blocked_vcpu
>   - NV => pi_wakeup_vector
>  local_events_need_delivery()
>hvm_vcpu_has_pending_irq()
> 
>  ...
>  context_switch(): nothing
> 
> The other is to do the "blocking" stuff in the context switch (similar
> to what's done now):
> 
> vcpu_block()
>   set(_VPF_blocked)
>   local_events_need_delivery()
> hvm_vcpu_has_pending_irq()
>   ...
> context_switch
>v->arch.block()
> - Add v to pcpu.pi_blocked_vcpu
> - NV => pi_wakeup_vector
> 
Ok, thanks for elaborating.

> If we do it the first way, and an interrupt comes in before the context
> switch is finished, it will call pi_wakeup_vector, which will DTRT --
> take v off the pi_blocked_vcpu list and call vcpu_unblock() (which in
> turn will set NV back to posted_intr_vector).
> 
> If we do it the second way, and an interrupt comes in before the context
> switch is finished, it will call posted_intr_vector.  We can, at that
> point, check to see if the current vcpu is marked as blocked.  If it is,
> we can call vcpu_unblock() without having to modify NV or worry about
> adding / removing the vcpu from the pi_blocked_vcpu list.
> 
Right.

> The thing I like about the first one is that it makes PI blocking the
> same as normal blocking -- everything happens in the same place; so the
> code is cleaner and easier to understand.
> 
Indeed.

> The thing I like about the second one is that it cleverly avoids having
> to do all the work of adding the vcpu to the list and then searching to
> remove it if the vcpu in question gets woken up on the way to being
> blocked.  So the code may end up being faster for workloads where that
> happens frequently.
> 
Maybe some instrumentation to figure out how frequent this is, at least
in a couple of (thought to be) common workloads can be added, and a few
test performed? Feng?

One thing that may be worth giving some thinking at is whether
interrupts are enabled or not. I mean, during vcpu_block()
SCHEDULE_SOFTIRQ is raised, for invoking the scheduler. Then (at least)
during schedule(), IRQs are disabled. They're re-enabled near the end of
schedule(), and re-disabled for the actual context switch ( ~= around
__context_switch()).

My point being that IRQs are going to be disabled for a significant
amount of time, between vcpu_block() and the actual context being
switched. And solution 2 requires IRQs to be enabled in order to avoid a
potentially pointless NV update, doesn't it? If yes, that may
(negatively) affect the probability of being able to actually benefit
from the optimization...

Anyway, I'm not sure. I think my gut feelings are in favour of solution
1, but, really, it's hard to tell without actually trying. 

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

Re: [Xen-devel] [PATCH v5 5/9] x86/intel_pstate: changes in cpufreq_del_cpu for CPU offline

On 14/09/15 03:32, Wei Wang wrote:
> We change to NULL the cpufreq_cpu_policy pointer after the call of
> cpufreq_driver->exit, because the pointer is still needed in
> intel_pstate_set_pstate().
>
> Signed-off-by: Wei Wang 
> ---
>  xen/drivers/cpufreq/cpufreq.c  | 6 +++---
>  xen/include/acpi/cpufreq/cpufreq.h | 7 +++
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
>  changes in v5:
>  1) put this patch prior to the "main body of intel pstate driver", which is 
> one of the acceptable options suggested by the Jan.
>
> diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
> index 0c437d4..5485944 100644
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -334,12 +334,11 @@ int cpufreq_del_cpu(unsigned int cpu)
>  
>  /* for HW_ALL, stop gov for each core of the _PSD domain */
>  /* for SW_ALL & SW_ANY, stop gov for the 1st core of the _PSD domain */
> -if (hw_all || (cpumask_weight(cpufreq_dom->map) ==
> -   perf->domain_info.num_processors))
> +if (!policy->internal_gov && (hw_all || 
> (cpumask_weight(cpufreq_dom->map) ==
> +   perf->domain_info.num_processors)))

This will be easier to read if you change the location of the linebreak
as such:

perf->domain_info.num_processors)))


if (!policy->internal_gov &&
(hw_all || (cpumask_weight(cpufreq_dom->map) ==
   perf->domain_info.num_processors)))

>  __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
>  
>  cpufreq_statistic_exit(cpu);
> -per_cpu(cpufreq_cpu_policy, cpu) = NULL;
>  cpumask_clear_cpu(cpu, policy->cpus);
>  cpumask_clear_cpu(cpu, cpufreq_dom->map);
>  
> @@ -348,6 +347,7 @@ int cpufreq_del_cpu(unsigned int cpu)
>  free_cpumask_var(policy->cpus);
>  xfree(policy);
>  }
> +per_cpu(cpufreq_cpu_policy, cpu) = NULL;
>  
>  /* for the last cpu of the domain, clean room */
>  /* It's safe here to free freq_table, drv_data and policy */
> diff --git a/xen/include/acpi/cpufreq/cpufreq.h 
> b/xen/include/acpi/cpufreq/cpufreq.h
> index c6976d0..48bd94d 100644
> --- a/xen/include/acpi/cpufreq/cpufreq.h
> +++ b/xen/include/acpi/cpufreq/cpufreq.h
> @@ -53,6 +53,12 @@ struct perf_limits {
>  uint32_t min_policy_pct;
>  };
>  
> +struct internal_governor {
> +char *avail_gov;
> +uint32_t gov_num;
> +uint32_t cur_gov;
> +};
> +

Adding this structure needs a mention in the commit message.

~Andrew

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops



On 17/09/2015 17:26, Andy Lutomirski wrote:
> Maybe Paolo can fix QEMU to fail bad MSR accesses for real...

I was afraid of someone proposing exactly that. :)

I can do it since the list of MSRs can be lifted from KVM.  Let's first
see the direction these patches go...

Paolo

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Arjan van de Ven




( We should double check that rdmsr()/wrmsr() results are never left
   uninitialized, but are set to zero or so, for cases where the return code is 
not
   checked. )


It sure looks like native_read_msr_safe doesn't clear the output if
the rdmsr fails.


I'd suggest to return some poison not just 0...
less likely to get interesting surprises that are insane hard to debug/diagnose




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

Re: [Xen-devel] [PATCH for-4.6] INSTALL: Mention MINIOS_UPSTREAM_URL

On Thu, Sep 17, 2015 at 05:30:50PM +0100, Ian Campbell wrote:
> All the other ones seem to be there.
> 
> Signed-off-by: Ian Campbell 
> ---
> For 4.6: trivially

Documentation update is safe to go in.

Release-acked-by: Wei Liu 

> ---
>  INSTALL | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/INSTALL b/INSTALL
> index 12615ab..56e2950 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -248,6 +248,7 @@ OVMF_UPSTREAM_URL=
>  QEMU_UPSTREAM_URL=
>  QEMU_TRADITIONAL_URL=
>  SEABIOS_UPSTREAM_URL=
> +MINIOS_UPSTREAM_URL=
>  
>  Using additional CFLAGS to build tools which will run in dom0 is
>  required when building distro packages. These variables can be used to
> -- 
> 2.1.4

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

Re: [Xen-devel] [PATCH OSSTEST 3/4] ms-queuedaemon: Add report-projection

Ian Campbell writes ("[PATCH OSSTEST 3/4] ms-queuedaemon: Add 
report-projection"):
> No semantic change. Eventually more tasks will be added which are
> wanted in report-projection but not report-plan.

Acked-by: Ian Jackson 

Though you might want to avoid the word `task'.  How about `Eventually
more work will be done ...' ?

Ian.

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

Re: [Xen-devel] [PATCH v5 6/9] x86/intel_pstate: the main boby of the intel_pstate driver

On 14/09/15 03:32, Wei Wang wrote:
> We simply grab the fundamental logic of the intel_pstate driver
> from Linux kernel, and customize it to Xen style.

How much customisation?  Is it just style, or other additions as well? 
(Deletions are less interesting)

For files we import directly from Linux, we keep Linux style to simplify
taking new patches in the future.  Whether it is sensible to do so here
depends on how modified it is from Linux.

> diff --git a/xen/include/asm-x86/cpufreq.h b/xen/include/asm-x86/cpufreq.h
> new file mode 100644
> index 000..94410f8
> --- /dev/null
> +++ b/xen/include/asm-x86/cpufreq.h
> @@ -0,0 +1,34 @@
> +#ifndef _ASM_X86_CPUFREQ_H
> +#define _ASM_X86_CPUFREQ_H
> +
> +/*
> + *  Copyright (C) 2015 Wei Wang 
> + *
> + * ~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Per c/s 443701e "Replace FSF street address with canonical URL", please
correct this GPL header.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.6] HACK: Use my local trees

On Thu, 2015-09-17 at 17:28 +0100, Ian Campbell wrote:

Sorry, wrong patch, please ignore

> ---
>  .config | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/.config b/.config
> index 8258faa..9b808a2 100644
> --- a/.config
> +++ b/.config
> @@ -1,5 +1,5 @@
> -MINIOS_UPSTREAM_URL := git://xenbits.xen.org/people/ianc/mini-os.git
> -MINIOS_UPSTREAM_REVISION := 134bad3e7d1ab2c93019813f607cc703ccef4661
> +MINIOS_UPSTREAM_URL := /home/ianc/devel/libxenctrl-split/mini-os.git
> +MINIOS_UPSTREAM_REVISION := master
>  
> -QEMU_TRADITIONAL_URL := git://xenbits.xen.org/people/ianc/qemu-xen
> -unstable.git
> -QEMU_TRADITIONAL_REVISION := e06f1ca27b11fe122edb025919d4e69800eca6a9
> +QEMU_TRADITIONAL_URL := /home/ianc/devel/libxenctrl-split/qemu-xen
> -traditional.git
> +QEMU_UPSTREAM_URL := /home/ianc/devel/libxenctrl-split/qemu-xen.git

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Andy Lutomirski

On Thu, Sep 17, 2015 at 12:19 AM, Ingo Molnar  wrote:
>
> * Andy Lutomirski  wrote:
>
>> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
>> turns all rdmsr and wrmsr operations into the safe variants without
>> any checks that the operations actually succeed.
>>
>> This is IMO awful: it papers over bugs.  In particular, KVM gueests
>> might be unwittingly depending on this behavior because
>> CONFIG_KVM_GUEST currently depends on CONFIG_PARAVIRT.  I'm not
>> aware of any such problems, but applying this series would be a good
>> way to shake them out.
>>
>> Fix it so that the MSR operations work the same on CONFIG_PARAVIRT=n
>> and CONFIG_PARAVIRT=y as long as Xen isn't being used.  The Xen
>> maintainers are welcome to make a similar change on top of this.
>>
>> Since there's plenty of time before the next merge window, I think
>> we should apply and fix anything that breaks.
>
> No, I think we should at most generate a warning instead, and not crash the 
> kernel
> via rdmsr()!
>
> Most big distro kernels on bare metal have CONFIG_PARAVIRT=y (I checked 
> Ubuntu and
> Fedora), so we are potentially exposing a lot of users to problems.
>
> Crashing the bootup on an unknown MSR is bad. Many MSR reads and writes are
> non-critical and returning the 'safe' result is much better than crashing or
> hanging the bootup.
>

Should we do that for CONFIG_PARAVIRT=n, too?

It would be straightforward to rig this up (temporarily?) on top of
these patches.  To keep bloat down, we might want to implement it in
do_general_protection rather than sticking it in native_read_msr.

wrmsr is a different beast, since we can fail due to writing the wrong
value to an otherwise valid MSR.  Given that MSR screwups can very
easily be security holes, I'm not sure that warning and blindly
continuing on an unchecked failed wrmsr is a good idea.

In any event, I think it's nuts that CONFIG_PARAVIRT changes this
behavior.  We should pick something sane and stick with it.
CONFIG_PARAVIRT=n appears to work just fine on bare metal in lots of
deployments, especially pre-KVM.  CONFIG_PARAVIRT=n also appears to
work fine under KVM, and I use it frequently.

> ( We should double check that rdmsr()/wrmsr() results are never left
>   uninitialized, but are set to zero or so, for cases where the return code 
> is not
>   checked. )

It sure looks like native_read_msr_safe doesn't clear the output if
the rdmsr fails.

--Andy

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

On 17/09/15 16:27, Borislav Petkov wrote:
> On Thu, Sep 17, 2015 at 01:39:26PM +0200, Paolo Bonzini wrote:
>> That's not a big deal, that's what *_safe is for.  The problem is that
>> there are definitely some cases where the *_safe version is not being used.
> I mean to do feature checks which assure you that those MSRs are
> there so you don't need the safe variants. And that is not always
> easy/possible.
>

There are plenty of non-architectural MSRs in use which don't have
feature bits.

Xen used to have problems booting when using the masking MSRs when
booting virtualised.  Nowadays it uses a cpu vendor check and _safe()
probe to detect support.

~Andrew

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops



On 17/09/2015 17:31, Arjan van de Ven wrote:
>>
>> What about 0 + WARN?
> 
> why 0?
> 
> 0xdeadbeef or any other pattern (even 0x3636363636) makes more sense (of
> course also WARN... but most folks don't read dmesg for WARNs)
> 
> (it's the same thing we do for list or slab poison stuff)

Sorry, brain fart.  That makes sense for the safe variants.  It would
require some auditing though.

Paolo

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

Re: [Xen-devel] [PATCH v4 1/5] sched: factor out VCPU2ONLINE to common header file

On Sun, 2015-07-12 at 22:13 -1000, Justin T. Weaver wrote:
> Move macro VCPU2ONLINE from schedule.c to sched.h so it can be used by other
> source files.
> 
"No functional changes intended."

> Signed-off-by: Justin T. Weaver 
>
(with the above)

Reviewed-by: Dario Faggioli 
-- 
<> (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 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Borislav Petkov

On Thu, Sep 17, 2015 at 04:32:53PM +0100, Andrew Cooper wrote:
> There are plenty of non-architectural MSRs in use which don't have
> feature bits.

That's exactly what the "possible" adjective was supposed to represent.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

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

Re: [Xen-devel] [PATCH v5 4/9] x86/intel_pstate: relocate the driver register function

On 14/09/15 03:32, Wei Wang wrote:
> Move the driver register function to
> the cpufreq.c.
>
> Signed-off-by: Wei Wang 
> ---
>  xen/drivers/cpufreq/cpufreq.c  | 15 +++
>  xen/include/acpi/cpufreq/cpufreq.h | 27 +--
>  2 files changed, 16 insertions(+), 26 deletions(-)
>
>  changes in v5:
>  1) keep cpufreq_presmp_init() intact.
>
> diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
> index 567e9e9..0c437d4 100644
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -638,3 +638,18 @@ static int __init cpufreq_presmp_init(void)
>  }
>  presmp_initcall(cpufreq_presmp_init);
>  
> +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> +{
> +   if ( !driver_data || !driver_data->init ||
> +!driver_data->verify || !driver_data->exit ||
> +(!driver_data->target == !driver_data->setpolicy) )

This line will incur the wrath of newer GCC's which have warnings
against such logic.

Either bracket the (!driver_data->$X) or alter the logic itself.

Also, the commit message needs to be corrected to state that you are
also introducing a new check to the registration function.

~Andrew

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

Re: [Xen-devel] [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops

2015-09-17 Thread Arjan van de Ven


On 9/17/2015 8:29 AM, Paolo Bonzini wrote:



On 17/09/2015 17:27, Arjan van de Ven wrote:



( We should double check that rdmsr()/wrmsr() results are never left
uninitialized, but are set to zero or so, for cases where the
return code is not
checked. )


It sure looks like native_read_msr_safe doesn't clear the output if
the rdmsr fails.


I'd suggest to return some poison not just 0...


What about 0 + WARN?


why 0?

0xdeadbeef or any other pattern (even 0x3636363636) makes more sense (of course 
also WARN... but most folks don't read dmesg for WARNs)

(it's the same thing we do for list or slab poison stuff)


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

Re: [Xen-devel] [PATCH v5 1/9] x86/intel_pstate: add some calculation related support

On 14/09/15 03:32, Wei Wang wrote:
> The added calculation related functions will be used in the intel_pstate.c.
> They are copied from the Linux kernel(commit 2418f4f2, f3002134, eb18cba7).
>
> Signed-off-by: Wei Wang 
> ---
>  xen/arch/x86/oprofile/op_model_athlon.c |  9 
>  xen/include/asm-x86/div64.h | 79 
> +
>  xen/include/xen/kernel.h| 23 ++
>  3 files changed, 102 insertions(+), 9 deletions(-)
>
>  changes in v5:
>  1) add clamp(), a type checking variant of clamp_t();
>  2) remove the private copy of clamp() in op_model_athlon.c.
>
> diff --git a/xen/arch/x86/oprofile/op_model_athlon.c 
> b/xen/arch/x86/oprofile/op_model_athlon.c
> index c0a81ed..4122eee 100644
> --- a/xen/arch/x86/oprofile/op_model_athlon.c
> +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> @@ -103,15 +103,6 @@ static u64 ibs_op_ctl;
>  #define IBS_FETCH_CODE  13
>  #define IBS_OP_CODE 14
>  
> -#define clamp(val, min, max) ({  \
> - typeof(val) __val = (val);  \
> - typeof(min) __min = (min);  \
> - typeof(max) __max = (max);  \
> - (void) (&__val == &__min);  \
> - (void) (&__val == &__max);  \
> - __val = __val < __min ? __min: __val;   \
> - __val > __max ? __max: __val; })
> -
>  /*
>   * 16-bit Linear Feedback Shift Register (LFSR)
>   */
> diff --git a/xen/include/asm-x86/div64.h b/xen/include/asm-x86/div64.h
> index dd49f64..6ba03cb 100644
> --- a/xen/include/asm-x86/div64.h
> +++ b/xen/include/asm-x86/div64.h
> @@ -11,4 +11,83 @@
>  __rem;  \
>  })
>  

Comment to explain the functionality?

> +static inline uint64_t div_u64_rem(uint64_t dividend, uint32_t divisor,
> +  uint32_t *remainder)

Alignment

> +{
> +*remainder = do_div(dividend, divisor);
> +return dividend;
> +}
> +
> +static inline uint64_t div_u64(uint64_t dividend, uint32_t  divisor)


> +
> +/**
>   * container_of - cast a member of a structure out to the containing 
> structure
>   *
>   * @ptr: the pointer to the member.

Too many spaces before "divisor"

> +{
> +uint32_t remainder;
> +
> +return div_u64_rem(dividend, divisor, );
> +}
> +
> +/*
> + * div64_u64 - unsigned 64bit divide with 64bit divisor
> + * @dividend:64bit dividend
> + * @divisor:64bit divisor

Alignment also looks wonky here.

> + *
> + * This implementation is a modified version of the algorithm proposed
> + * by the book 'Hacker's Delight'.  The original source and full proof
> + * can be found here and is available for use without restriction.
> + *
> + * 'http://www.hackersdelight.org/HDcode/newCode/divDouble.c.txt'
> + */
> +static inline uint64_t div64_u64(uint64_t dividend, uint64_t divisor)
> +{
> +uint32_t high = divisor >> 32;
> +uint64_t quot;
> +
> +if ( high == 0 )
> +quot = div_u64(dividend, divisor);
> +else
> +{
> +int n = 1 + fls(high);
> +
> +quot = div_u64(dividend >> n, divisor >> n);
> +
> +if ( quot != 0 )
> +quot--;
> +if ( (dividend - quot * divisor) >= divisor )
> +quot++;
> +}
> +return quot;
> +}
> +

Comment?

> +static inline int64_t div_s64_rem(int64_t dividend, int32_t divisor,
> + int32_t *remainder)

Alignment.

> +{
> +int64_t quotient;
> +
> +if ( dividend < 0 )
> +{
> +quotient = div_u64_rem(-dividend, ABS(divisor),
> +(uint32_t *)remainder);

Alignment.

> +*remainder = -*remainder;
> +if ( divisor > 0 )
> +quotient = -quotient;
> +}
> +else
> +{
> +quotient = div_u64_rem(dividend, ABS(divisor),
> +(uint32_t *)remainder);

Alignment.

> +if ( divisor < 0 )
> +quotient = -quotient;
> +}
> +return quotient;
> +}
> +
> +/*
> + * div_s64 - signed 64bit divide with 32bit divisor
> + */
> +static inline int64_t div_s64(int64_t dividend, int32_t divisor)
> +{
> +int32_t remainder;
> +
> +return div_s64_rem(dividend, divisor, );
> +}
> +
>  #endif
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64d..9812698 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -43,6 +43,29 @@
>  #define MAX(x,y) ((x) > (y) ? (x) : (y))
>  
>  /**
> + * clamp - return a value clamped to a given range with strict typechecking
> + * @val: current value
> + * @lo: lowest allowable value
> + * @hi: highest allowable value
> + *
> + * This macro does strict typechecking of lo/hi to make sure they are of the
> + * same type as val.  See the unnecessary pointer comparisons.
> + */
> +#define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)

This is a change of behaviour from the clamp() you removed, as this now

Re: [Xen-devel] [PATCH v5 7/9] x86/intel_pstate: add a booting param to select the driver to load